[libvirt] [PATCH] util: assume modern CPU_ALLOC macros always exist

Daniel P. Berrangé posted 1 patch 13 weeks ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190709113354.9304-1-berrange@redhat.com
src/util/virprocess.c | 36 ------------------------------------
1 file changed, 36 deletions(-)

[libvirt] [PATCH] util: assume modern CPU_ALLOC macros always exist

Posted by Daniel P. Berrangé 13 weeks ago
Support for the modern CPU_ALLOC macros was added 10 years ago in

  commit a73cd93b2428adbbc62bb919b6cf5ffd27728040
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Mon Nov 16 16:08:29 2009 +0000

    Alternate CPU affinity impl to cope with NR_CPUS > 1024

This is long enough that we can assume it always exists and drop the
back compat code.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virprocess.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index f2533f639f..66834d37d3 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -422,8 +422,6 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map)
 {
     size_t i;
     VIR_DEBUG("Set process affinity on %lld", (long long)pid);
-# ifdef CPU_ALLOC
-    /* New method dynamically allocates cpu mask, allowing unlimted cpus */
     int numcpus = 1024;
     size_t masklen;
     cpu_set_t *mask;
@@ -462,22 +460,6 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map)
         return -1;
     }
     CPU_FREE(mask);
-# else
-    /* Legacy method uses a fixed size cpu mask, only allows up to 1024 cpus */
-    cpu_set_t mask;
-
-    CPU_ZERO(&mask);
-    for (i = 0; i < virBitmapSize(map); i++) {
-        if (virBitmapIsBitSet(map, i))
-            CPU_SET(i, &mask);
-    }
-
-    if (sched_setaffinity(pid, sizeof(mask), &mask) < 0) {
-        virReportSystemError(errno,
-                             _("cannot set CPU affinity on process %d"), pid);
-        return -1;
-    }
-# endif
 
     return 0;
 }
@@ -491,7 +473,6 @@ virProcessGetAffinity(pid_t pid)
     size_t ncpus;
     virBitmapPtr ret = NULL;
 
-# ifdef CPU_ALLOC
     /* 262144 cpus ought to be enough for anyone */
     ncpus = 1024 << 8;
     masklen = CPU_ALLOC_SIZE(ncpus);
@@ -503,14 +484,6 @@ virProcessGetAffinity(pid_t pid)
     }
 
     CPU_ZERO_S(masklen, mask);
-# else
-    ncpus = 1024;
-    if (VIR_ALLOC(mask) < 0)
-        return NULL;
-
-    masklen = sizeof(*mask);
-    CPU_ZERO(mask);
-# endif
 
     if (sched_getaffinity(pid, masklen, mask) < 0) {
         virReportSystemError(errno,
@@ -522,22 +495,13 @@ virProcessGetAffinity(pid_t pid)
           goto cleanup;
 
     for (i = 0; i < ncpus; i++) {
-# ifdef CPU_ALLOC
          /* coverity[overrun-local] */
         if (CPU_ISSET_S(i, masklen, mask))
             ignore_value(virBitmapSetBit(ret, i));
-# else
-        if (CPU_ISSET(i, mask))
-            ignore_value(virBitmapSetBit(ret, i));
-# endif
     }
 
  cleanup:
-# ifdef CPU_ALLOC
     CPU_FREE(mask);
-# else
-    VIR_FREE(mask);
-# endif
 
     return ret;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: assume modern CPU_ALLOC macros always exist

Posted by Martin Kletzander 13 weeks ago
On Tue, Jul 09, 2019 at 12:33:54PM +0100, Daniel P. Berrangé wrote:
>Support for the modern CPU_ALLOC macros was added 10 years ago in
>
>  commit a73cd93b2428adbbc62bb919b6cf5ffd27728040
>  Author: Daniel P. Berrange <berrange@redhat.com>
>  Date:   Mon Nov 16 16:08:29 2009 +0000
>
>    Alternate CPU affinity impl to cope with NR_CPUS > 1024
>
>This is long enough that we can assume it always exists and drop the
>back compat code.
>

Yes, it was added to glibc 12 years ago, so it should be in few distros now.
I'm not sure what is the support in other libc implementations and what is our
libc support matrix, but if we either:

 a) support only glibc, or

 b) checked that other ones have that as well (ideally with a note in the commit
    message)

then I can safely say:

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: assume modern CPU_ALLOC macros always exist

Posted by Daniel P. Berrangé 13 weeks ago
On Tue, Jul 09, 2019 at 05:10:59PM +0200, Martin Kletzander wrote:
> On Tue, Jul 09, 2019 at 12:33:54PM +0100, Daniel P. Berrangé wrote:
> > Support for the modern CPU_ALLOC macros was added 10 years ago in
> > 
> >  commit a73cd93b2428adbbc62bb919b6cf5ffd27728040
> >  Author: Daniel P. Berrange <berrange@redhat.com>
> >  Date:   Mon Nov 16 16:08:29 2009 +0000
> > 
> >    Alternate CPU affinity impl to cope with NR_CPUS > 1024
> > 
> > This is long enough that we can assume it always exists and drop the
> > back compat code.
> > 
> 
> Yes, it was added to glibc 12 years ago, so it should be in few distros now.
> I'm not sure what is the support in other libc implementations and what is our
> libc support matrix, but if we either:
> 
> a) support only glibc, or

We only test on glibc. We don't go out of our way to intentionally
break other libc impls so in general they can be expected to work.

> b) checked that other ones have that as well (ideally with a note in the commit
>    message)

I've not checked any other libcs, but if any don't support this after
12+ years they are seriously broken because they're unusable when the
kernel is built with large NR_CPUS. Thus I don't think we need to care
to check them. ie if they work, everything is fine, if they don't work
that they're unsupported targets for libvirt.

> then I can safely say:
> 
> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list