Re: [PATCH] Cygwin: Fix return value of sched_getaffinity

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Jun 25 01:17, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > Hi Mark,
> > 
> > On Jun 24 22:25, Mark Geisert wrote:
> > > Return what the documentation says, instead of a misreading of it.
> > > ---
> > >   winsup/cygwin/sched.cc | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/winsup/cygwin/sched.cc b/winsup/cygwin/sched.cc
> > > index e7b44d319..8f24bf80d 100644
> > > --- a/winsup/cygwin/sched.cc
> > > +++ b/winsup/cygwin/sched.cc
> > > @@ -608,7 +608,7 @@ done:
> > >     else
> > >       {
> > >         /* Emulate documented Linux kernel behavior on successful return */
> > > -      status = wincap.cpu_count ();
> > > +      status = sizeof (cpu_set_t);
> > 
> > Wait... what docs are you referring to?  The Linux man page in Fedora 29
> > says
> > 
> >   On success, sched_setaffinity() and sched_getaffinity() return  0.   On
> >   error, -1 is returned, and errno is set appropriately.
> 
> I've been using http://man7.org/linux/man-pages/man2/sched_setaffinity.2.html
> which has the text you quoted under the RETURN VALUE heading, but has the
> following further down the page under the heading "C library/kernel
> differences":
> |        This manual page describes the glibc interface for the CPU affinity
> |        calls.  The actual system call interface is slightly different, with
> |        the mask being typed as unsigned long *, reflecting the fact that the
> |        underlying implementation of CPU sets is a simple bit mask.
> |
> |        On success, the raw sched_getaffinity() system call returns the
> |        number of bytes placed copied into the mask buffer; this will be the
> |        minimum of cpusetsize and the size (in bytes) of the cpumask_t data
> |        type that is used internally by the kernel to represent the CPU set
> |        bit mask.
> 
> I see now that that 2nd paragraph has actually been updated since I printed
> it out in April so I'll need to update the patch yet again.
> 
> The taskset(1) utility in util-linux actually depends on the kernel return
> value that glibc doesn't return.  On Cygwin there is only one "syscall"
> interface so I have to have sched_getaffinity() return a nonzero value on
> success like the Linux kernel does.

That's very unfortunate.  Keep in mind that applications usually link
against the C lib, so the expectation of developers is usually that the
API follows the C lib definitions.  The kernel API should only be
considered if there's no equivalent/documented C lib API.  And even
then, the call is typically just a SYSCALL wrapper, something we don't
support at all.

The API of sched_getaffinity should definitely follow glibc.  If you
need the kernel API as well, create another function like, say,
__sched_getaffinity_raw() (leading underscores) and export this as well.
You can simply add a cpp macro in your application to support this.
Make sure to document the differences in the source, lest we forget.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Attachment: signature.asc
Description: PGP signature