PERFORCE change 124529 for review

Jung-uk Kim jkim at FreeBSD.org
Mon Aug 27 11:16:33 PDT 2007


On Monday 27 August 2007 01:39 pm, Ken Smith wrote:
> On Sun, 2007-08-26 at 17:32 +0300, Kostik Belousov wrote:
> > On Sat, Aug 04, 2007 at 11:46:24AM +0200, Roman Divacky wrote:
> > > > > > > thnx for the review!
> > > > > >
> > > > > > Thank you!
> > > > >
> > > > > so... do you agree that the current revision is fine. I
> > > > > need this to get kib@ to commit this...
> > > >
> > > > Yes.
> > >
> > > in a case you missed that...
> > >
> > > the patch is here inlined or can be reached at:
> > > www.vlakno.cz/~rdivacky/linux_java.patch
> >
> > ...
> >
> > > to commit this:
> > >
> > > 1) apply the patch
> > > 2) commit
> > > the commit message should be something like "Implement fake
> > > sched_getaffinity() syscall to enable java work with 2.6
> > > emulation."
> > >
> > > 3) cd /sys/i386/linux && make sysent
> > > 4) cd /sys/amd64/linux32 && make sysent
> > > 5) commit with "Regen after syscalls.master update"
> > >
> > > thank you!
> >
> > Please, approve the attached patch. Since patch modifies syscall
> > tables, special commit procedure, as outlined above by Roman, is
> > needed.
> >
> > Commit message:
> > Implement fake linux sched_getaffinity() syscall to enable java
> > work with Linux 2.6 emulation. This shall be reimplemented once
> > FreeBSD gets native scheduler affinity syscalls.
> >
> > Submitted by:   rdivacky
> > Reviewed by:    jkim
> > Sponsored by:   Google Summer of Code 2007
> > Approved by:    re (XXX)
> >
> >
> >
> >
> >
> >
> >
> > text/x-diff attachment (linux_java.1.patch)
> >
> > Index: i386/linux/syscalls.master
> > =================================================================
> >== RCS file:
> > /usr/local/arch/ncvs/src/sys/i386/linux/syscalls.master,v
> > retrieving revision 1.87
> > diff -u -r1.87 syscalls.master
> > --- i386/linux/syscalls.master  29 Mar 2007 02:11:46 -0000     
> > 1.87 +++ i386/linux/syscalls.master  26 Aug 2007 14:20:12 -0000
> > @@ -410,7 +410,8 @@
> >  240    AUE_NULL        STD     { int linux_sys_futex(void
> > *uaddr, int op, int val, \ struct l_timespec *timeout, void
> > *uaddr2, int val3); } 241    AUE_NULL        UNIMPL 
> > linux_sched_setaffinity
> > -242    AUE_NULL        UNIMPL  linux_sched_getaffinity
> > +242    AUE_NULL        STD     { int
> > linux_sched_getaffinity(l_pid_t pid, l_uint len, \ +             
> >                          l_ulong *user_mask_ptr); } 243   
> > AUE_NULL        STD     { int linux_set_thread_area(struct
> > l_user_desc *desc); } 244    AUE_NULL        STD     { int
> > linux_get_thread_area(struct l_user_desc *desc); } 245   
> > AUE_NULL        UNIMPL  linux_io_setup
> > Index: amd64/linux32/syscalls.master
> > =================================================================
> >== RCS file:
> > /usr/local/arch/ncvs/src/sys/amd64/linux32/syscalls.master,v
> > retrieving revision 1.28
> > diff -u -r1.28 syscalls.master
> > --- amd64/linux32/syscalls.master       30 Mar 2007 00:06:21
> > -0000      1.28 +++ amd64/linux32/syscalls.master       26 Aug
> > 2007 14:20:12 -0000 @@ -408,7 +408,8 @@
> >  240    AUE_NULL        STD     { int linux_sys_futex(void
> > *uaddr, int op, int val, \ struct l_timespec *timeout, void
> > *uaddr2, int val3); } 241    AUE_NULL        UNIMPL 
> > linux_sched_setaffinity
> > -242    AUE_NULL        UNIMPL  linux_sched_getaffinity
> > +242    AUE_NULL        STD     { int
> > linux_sched_getaffinity(l_pid_t pid, l_uint len, \ +             
> >                          l_ulong *user_mask_ptr); } 243   
> > AUE_NULL        STD     { int linux_set_thread_area(struct
> > l_user_desc *desc); } 244    AUE_NULL        UNIMPL 
> > linux_get_thread_area
> >  245    AUE_NULL        UNIMPL  linux_io_setup
> > Index: compat/linux/linux_misc.c
> > =================================================================
> >== RCS file:
> > /usr/local/arch/ncvs/src/sys/compat/linux/linux_misc.c,v
> > retrieving revision 1.213
> > diff -u -r1.213 linux_misc.c
> > --- compat/linux/linux_misc.c   12 Jun 2007 00:11:57 -0000     
> > 1.213 +++ compat/linux/linux_misc.c   26 Aug 2007 14:20:12 -0000
> > @@ -1713,3 +1713,24 @@
> >
> >         return (error);
> >  }
> > +
> > +/*
> > + * XXX: fake one.. waiting for real implementation of affinity
> > mask. + */
> > +int
> > +linux_sched_getaffinity(struct thread *td,
> > +    struct linux_sched_getaffinity_args *args)
> > +{
> > +       int error;
> > +       cpumask_t i = ~0;
> > +
> > +       if (args->len < sizeof(cpumask_t))
> > +               return (EINVAL);
> > +
> > +       error = copyout(&i, args->user_mask_ptr,
> > sizeof(cpumask_t)); +       if (error)
> > +               return (EFAULT);
> > +
> > +       td->td_retval[0] = sizeof(cpumask_t);
> > +       return (0);
> > +}
>
> Ok, I think I'm missing something here.  I'm in the process of
> doing a buildworld/etc on a scratch machine to test this but ...
>
> Doesn't this:
>
> 	td->td_retval[0] = sizeof(cpumask_t);
>
> mean the calling process would see that as the return value of the
> system call?  I have a RedHat Linux (RHEL-4) machine handy, and it
> says in the manual page for sched_getaffinity(2):
>
> RETURN VALUE
>        On success, sched_setaffinity and sched_getaffinity both
> return 0.  On error, -1 is returned, and errno is set
> appropriately.
>
> I tested it with a quick (ugly) little program:
>
> nickelback 12 % cat foo.c
> #include <stdio.h>
>
> main(int argc, char *argv[])
> {
>         int ret;
>         unsigned long mask;
>
>         ret = sched_getaffinity(0, sizeof(mask), &mask);
>         printf("mask: %ld ret: %d\n", mask, ret);
> }
> nickelback 13 % ./foo
> mask: 15 ret: 0
> nickelback 14 % uname -a
> Linux nickelback.cse.buffalo.edu 2.6.9-42.0.10.ELsmp #1 SMP Fri Feb
> 16 17:13:42 EST 2007 x86_64 x86_64 x86_64 GNU/Linux nickelback 15 %
>
> (Note the sched_getaffinity() manual page also says using 0 for the
> process ID argument results in it using the current process which
> is why I used that in the test program).
>
> If that analysis is correct you should be skipping the assignment
> of td->td_retval[0] and just going straight into the "return (0);".
>  Linux programs would actually get "upset" seeing a non-zero return
> value if the system call was supposed to succeed.
>
> Is that correct or did I miss something?  Thanks.

sched_{get,set}affinity() are very misleading syscalls.  Userland 
(glibc) and kernel have different definitions and Roman's patch 
implemented linux 2.6 kernel behaviour, AFAIK.  Glibc wraps all 
differences between kernel versions.  See the following link:

http://jeff.squyres.com/journal/archives/2005/10/linux_processor.html

Jung-uk Kim


More information about the p4-projects mailing list