[PATCH 1 of 2] fix sched_setaffinity(2) call failure

[Maxim Dounin]

Hello!

On Mon, Jan 16, 2012 at 01:52:09PM +0800, Joshua Zhu wrote:

[...]

> > diff -r e1296af53cc0 src/os/unix/ngx_process_cycle.c
> > --- a/src/os/unix/ngx_process_cycle.c   Mon Dec 26 00:00:00 2011 +0400
> > +++ b/src/os/unix/ngx_process_cycle.c   Fri Jan 13 11:58:01 2012 -0500
> > @@ -914,7 +914,10 @@
> >         ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0,
> >                       "sched_setaffinity(0x%08Xl)", cpu_affinity);
> >
> > -        if (sched_setaffinity(0, 32, (cpu_set_t *) &cpu_affinity) == -1) {
> > +        if (sched_setaffinity(0, sizeof(cpu_affinity),
> > +                              (cpu_set_t *) &cpu_affinity)
> > +            == -1)
> > +        {
> >             ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
> >                           "sched_setaffinity(0x%08Xl) failed",
> > cpu_affinity);
> >         }
> >
> > Could you please test if it works for you?
> >
> 
> Yes, it works on one of my Linux box which has two cores. But I have not
> tested it on more other systems yet.

Ok, I'll commit it then, as this looks like an obvious fix.

[...]

> > While I generally agree that using cpu_set_t and CPU_* macros
> > explicitly would be better, I don't think we want to introduce
> > linux-centric code here in platform-independent code.
> >
> > Instead we probably want to leave this code generic enough to be
> > useable with other scheduler interfaces, and convert the data
> > somewhere near platform-specific whatever_setaffinity() calls if
> > needed.  (Likely unneeded for the Linux with the above patch.)
> >
> >
> We use cpu_set_t and CPU_* macros in the patch not only because it's more
> generic and not depend on the implementation of the kernel (I guess the
> code in Nginx now is inspired by some _early_ kernel implementation, i.e.
> Linux 2.5.x), but also we will have no 32-core-limitation any more
> ('"worker_cpu_affinity" supports up to 32 CPU only').

Are you facing the 32-core limit in practice?

> I agree that the set_cpu_affinity related code in Nginx core should be
> platform-independent. But the code now is _not_ (please look at those
> NGX_HAVE_SCHED_SETAFFINITY macros).

As of now, the only part which isn't portable is 
sched_setaffinity() call itself.  Other parts (i.e. config 
parsing) may be reused for other whatever_setaffinity() calls as 
soon as we add support.

> And I do recommend move the
> implementation of ngx_set_cpu_affinity() to the os/ directory when
> worker_cpu_affinity is supported on other platforms.

Yes, that's basically what I suggest also.  We want some generic 
interface, and platform-specific wrappers which do needed 
conversions/magic.

Maxim Dounin