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

[Daniel Veillard]

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
> 
> * src/util/processinfo.c: Support new unlimited size CPU set type
> ---
>  src/util/processinfo.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 84 insertions(+), 0 deletions(-)
> 
> diff --git a/src/util/processinfo.c b/src/util/processinfo.c
> index aaffd88..a0dac34 100644
> --- a/src/util/processinfo.c
> +++ b/src/util/processinfo.c
> @@ -38,6 +38,48 @@ int virProcessInfoSetAffinity(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
> +     */

  Urgh ....
> +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. 

> +            goto realloc;
> +        }
> +        virReportSystemError(NULL, errno,
> +                             _("cannot set CPU affinity on process %d"), pid);
> +        return -1;
> +    }
> +
> +    for (i = 0 ; i < maxcpu ; i++)
> +        if (CPU_ISSET_S(i, masklen, mask))
> +            VIR_USE_CPU(map, i);
> +#else
> +    /* Legacy method uses a fixed size cpu mask, only allows upto 1024 cpus */
>      cpu_set_t mask;
>  
>      CPU_ZERO(&mask);
> @@ -73,6 +156,7 @@ int virProcessInfoGetAffinity(pid_t pid,
>      for (i = 0 ; i < maxcpu ; i++)
>          if (CPU_ISSET(i, &mask))
>              VIR_USE_CPU(map, i);
> +#endif
>  
>      return 0;
>  }

  That patch sounds less urgent might be worth waiting past 0.7.3,
ACK though

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/