7:05 p.m.
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
7:05 p.m.
New subject: [PATCH 1/3] util: Accept TIDs for virProcess{Get,Set}Affinity() on BSD
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(a)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
7:05 p.m.
New subject: [PATCH 2/3] util: Prefer cpuset_{get,set}affinity() on BSD
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(a)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
7:05 p.m.
New subject: [PATCH 3/3] util: Add debug print missing from BSD
Signed-off-by: Andrea Bolognani <abologna(a)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
2:54 a.m.
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(a)redhat.com>
src/util/virprocess.c | 112
+++++++++++++++++++++---------------------
1 file changed, 57 insertions(+), 55 deletions(-)
--
2.43.2
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
83
days inactive
86
days old
4 comments
2 participants
participants (2)
-
Andrea Bolognani -
Martin Kletzander