[libvirt] [PATCH 2/2] Alternate CPU affinity impl to cope with NR_CPUS > 1024

[Daniel P. Berrange]

On Mon, Nov 16, 2009 at 06:13:05PM +0100, Daniel Veillard wrote:
> On Mon, Nov 16, 2009 at 04:49:30PM +0000, Daniel P. Berrange wrote:
> > The cpu_set_t type can only cope with NR_CPUS <= 1024, beyond this
> > it is neccessary to use alternate CPU_SET maps with a dynamically
> > allocated CPU map
> > 
> > +realloc:
> > +    masklen = CPU_ALLOC_SIZE(numcpus);
> > +    mask = CPU_ALLOC(numcpus);
> > +
> > +    if (!mask) {
> > +        virReportOOMError(NULL);
> > +        return -1;
> > +    }
> > +
> > +    CPU_ZERO_S(masklen, mask);
> > +    for (i = 0 ; i < maxcpu ; i++) {
> > +        if (VIR_CPU_USABLE(map, maplen, 0, i))
> > +            CPU_SET_S(i, masklen, mask);
> > +    }
> > +
> > +    if (sched_setaffinity(pid, masklen, mask) < 0) {
> > +        CPU_FREE(mask);
> > +        if (errno == EINVAL &&
> > +            numcpus < (1024 << 8)) { /* 262144 cpus ought to be enough for anyone */
> > +            numcpus = numcpus << 2;
> 
>                let's just 
>                  numcpus *= 2;
>                or
>                  numcpus *= 4;
> it's not like we want to shave a microsecond, makes code less readable.



> 
> > +            goto realloc;
> > +        }
> > +        virReportSystemError(NULL, errno,
> > +                             _("cannot set CPU affinity on process %d"), pid);
> > +        return -1;
> > +    }
> > +    CPU_FREE(mask);
> > +#else
> > +    /* Legacy method uses a fixed size cpu mask, only allows upto 1024 cpus */
> >      cpu_set_t mask;
> >  
> >      CPU_ZERO(&mask);
> > @@ -51,6 +93,7 @@ int virProcessInfoSetAffinity(pid_t pid,
> >                               _("cannot set CPU affinity on process %d"), pid);
> >          return -1;
> >      }
> > +#endif
> >  
> >      return 0;
> >  }
> > @@ -61,6 +104,46 @@ int virProcessInfoGetAffinity(pid_t pid,
> >                                int maxcpu)
> >  {
> >      int i;
> > +#ifdef CPU_ALLOC
> > +    /* New method dynamically allocates cpu mask, allowing unlimted cpus */
> > +    int numcpus = 1024;
> > +    size_t masklen;
> > +    cpu_set_t *mask;
> > +
> > +    /* Not only may the statically allocated cpu_set_t be too small,
> > +     * but there is no way to ask the kernel what size is large enough.
> > +     * So you have no option but to pick a size, try, catch EINVAL,
> > +     * enlarge, and re-try.
> > +     *
> > +     * http://lkml.org/lkml/2009/7/28/620
> > +     */
> > +realloc:
> > +    masklen = CPU_ALLOC_SIZE(numcpus);
> > +    mask = CPU_ALLOC(numcpus);
> > +
> > +    if (!mask) {
> > +        virReportOOMError(NULL);
> > +        return -1;
> > +    }
> > +
> > +    CPU_ZERO_S(masklen, mask);
> > +    if (sched_getaffinity(pid, masklen, mask) < 0) {
> > +        CPU_FREE(mask);
> > +        if (errno == EINVAL &&
> > +            numcpus < (1024 << 8)) { /* 262144 cpus ought to be enough for anyone */
> > +            numcpus = numcpus << 2;
> 
>   same
> I would also make numcpus a static variable, so that you don't repeat he
> loop each time you go though one of those APIs. 

Using static variables in this kind of context are not thread-safe
and I don't really want to introduce locking in here. FYI, in the
common case of kernels compiled with a sensible NR_CPUS, there will
only ever be a single pass in the loop. In the uncommon case of using
a NR_CPUS=4096, I picked 1024 and the '<< 2', to ensure there is only
two passes in the loop (first fails, second succeeds). So i don't think
it needs optimizing further

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|