[PATCH 0/3] util: Fix setting process affinity on BSD
Commit c07cf0a68693 tried to ensure that the Linux-compatible syscalls would not be used on FreeBSD, but something must have changed between then and now because I can clearly see sched_{get,set}affinity() being used instead of their cpuset_* counterparts on FreeBSD 14. Ensure that the BSD variants are always preferred. Before that, fix them so that they accept either a PID or a TID. The fact that this wasn't the case until now makes me suspect that scenarios in which thread-level affinity is applied have not been tested at all on the platform, because doing something as simple as <vcpu placement='static' cpuset='1-2'>1</vcpu> is enough to prevent the VM from starting, and before FreeBSD 14 the Linux compatibility APIs shouldn't have been part of the picture at all. Andrea Bolognani (3): util: Accept TIDs for virProcess{Get,Set}Affinity() on BSD util: Prefer cpuset_{get,set}affinity() on BSD util: Add debug print missing from BSD src/util/virprocess.c | 112 +++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 55 deletions(-) -- 2.43.2
Depending on the situation, the IDs that we pass to these functions can be either referring to processes or threads. Linux doesn't have separate interfaces for one or the other, but FreeBSD does and we're explicitly telling it that the ID refers to a process. When it refers to a thread instead, the call will fail, and the VM will not be able to start. Luckily, another possible choice is CPU_WHICH_TIDPID, which makes things behave the same as Linux. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virprocess.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 047b0aa0cd..8384bd1378 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -551,7 +551,7 @@ int virProcessSetAffinity(pid_t pid, CPU_SET(i, &mask); } - if (cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_PID, pid, + if (cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, sizeof(mask), &mask) != 0) { if (quiet) { VIR_DEBUG("cannot set CPU affinity on process %d: %s", @@ -574,7 +574,7 @@ virProcessGetAffinity(pid_t pid) virBitmap *ret = NULL; CPU_ZERO(&mask); - if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_PID, pid, + if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, sizeof(mask), &mask) != 0) { virReportSystemError(errno, _("cannot get CPU affinity of process %1$d"), pid); -- 2.43.2
FreeBSD 14 implements sched_{get,set}affinity() for compatibility with Linux, but we should still use the native syscalls instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virprocess.c | 110 +++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 8384bd1378..426d56c1ce 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -444,7 +444,61 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); } -#if WITH_DECL_CPU_SET_T +#if defined(WITH_BSD_CPU_AFFINITY) + +int virProcessSetAffinity(pid_t pid, + virBitmap *map, + bool quiet) +{ + size_t i; + cpuset_t mask; + + CPU_ZERO(&mask); + for (i = 0; i < virBitmapSize(map); i++) { + if (virBitmapIsBitSet(map, i)) + CPU_SET(i, &mask); + } + + if (cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, + sizeof(mask), &mask) != 0) { + if (quiet) { + VIR_DEBUG("cannot set CPU affinity on process %d: %s", + pid, g_strerror(errno)); + } else { + virReportSystemError(errno, + _("cannot set CPU affinity on process %1$d"), pid); + return -1; + } + } + + return 0; +} + +virBitmap * +virProcessGetAffinity(pid_t pid) +{ + size_t i; + cpuset_t mask; + virBitmap *ret = NULL; + + CPU_ZERO(&mask); + if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, + sizeof(mask), &mask) != 0) { + virReportSystemError(errno, + _("cannot get CPU affinity of process %1$d"), pid); + return NULL; + } + + ret = virBitmapNew(sizeof(mask) * 8); + + for (i = 0; i < sizeof(mask) * 8; i++) + if (CPU_ISSET(i, &mask)) + ignore_value(virBitmapSetBit(ret, i)); + + return ret; +} + +#elif WITH_DECL_CPU_SET_T int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -536,60 +590,6 @@ virProcessGetAffinity(pid_t pid) return ret; } -#elif defined(WITH_BSD_CPU_AFFINITY) - -int virProcessSetAffinity(pid_t pid, - virBitmap *map, - bool quiet) -{ - size_t i; - cpuset_t mask; - - CPU_ZERO(&mask); - for (i = 0; i < virBitmapSize(map); i++) { - if (virBitmapIsBitSet(map, i)) - CPU_SET(i, &mask); - } - - if (cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, - sizeof(mask), &mask) != 0) { - if (quiet) { - VIR_DEBUG("cannot set CPU affinity on process %d: %s", - pid, g_strerror(errno)); - } else { - virReportSystemError(errno, - _("cannot set CPU affinity on process %1$d"), pid); - return -1; - } - } - - return 0; -} - -virBitmap * -virProcessGetAffinity(pid_t pid) -{ - size_t i; - cpuset_t mask; - virBitmap *ret = NULL; - - CPU_ZERO(&mask); - if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TIDPID, pid, - sizeof(mask), &mask) != 0) { - virReportSystemError(errno, - _("cannot get CPU affinity of process %1$d"), pid); - return NULL; - } - - ret = virBitmapNew(sizeof(mask) * 8); - - for (i = 0; i < sizeof(mask) * 8; i++) - if (CPU_ISSET(i, &mask)) - ignore_value(virBitmapSetBit(ret, i)); - - return ret; -} - #else /* WITH_DECL_CPU_SET_T */ int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, -- 2.43.2
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virprocess.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 426d56c1ce..647f687cb8 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -453,6 +453,8 @@ int virProcessSetAffinity(pid_t pid, size_t i; cpuset_t mask; + VIR_DEBUG("Set process affinity on %lld", (long long)pid); + CPU_ZERO(&mask); for (i = 0; i < virBitmapSize(map); i++) { if (virBitmapIsBitSet(map, i)) -- 2.43.2
On Fri, Feb 23, 2024 at 02:05:38AM +0100, Andrea Bolognani wrote:
Commit c07cf0a68693 tried to ensure that the Linux-compatible syscalls would not be used on FreeBSD, but something must have changed between then and now because I can clearly see sched_{get,set}affinity() being used instead of their cpuset_* counterparts on FreeBSD 14.
At first I thought this is because back then FreeBSD did not yet have sched_setaffinity in the linux ABI, but that was added in 7.1, so that's probably not it. I did not dig into it more since this change makes sense regardless.
Ensure that the BSD variants are always preferred.
Before that, fix them so that they accept either a PID or a TID. The fact that this wasn't the case until now makes me suspect that scenarios in which thread-level affinity is applied have not been tested at all on the platform, because doing something as simple as
<vcpu placement='static' cpuset='1-2'>1</vcpu>
is enough to prevent the VM from starting, and before FreeBSD 14 the Linux compatibility APIs shouldn't have been part of the picture at all.
Andrea Bolognani (3): util: Accept TIDs for virProcess{Get,Set}Affinity() on BSD util: Prefer cpuset_{get,set}affinity() on BSD util: Add debug print missing from BSD
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
src/util/virprocess.c | 112 +++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 55 deletions(-)
-- 2.43.2 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
participants (2)
-
Andrea Bolognani -
Martin Kletzander