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

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

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>

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 :|
participants (2)
-
Daniel P. Berrangé
-
Martin Kletzander