[PATCH] meson: improve CPU affinity routines check
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T. This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined. For now, change detection to the following: - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined 1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c4599301... https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b... https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2... Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 4 +++- src/util/virprocess.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [ # Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], + + [ 'sched.h', 'cpu_set_t' ], ] if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include <sys/time.h> # include <sys/resource.h> #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include <sched.h> #endif @@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); } -#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; } -#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */ int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote:
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined
Makes sense, although even the sched_getaffinity() is guarded by the new _WITH_CPU_SET_T so there should be no error with the current code, am I right? And if I understand correctly we cannot use the FreeBSD implementation as is because they do not have all the CPU_* macros we need, right?
1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c4599301... https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b... https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2...
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 4 +++- src/util/virprocess.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [
# Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], + + [ 'sched.h', 'cpu_set_t' ], ]
if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include <sys/time.h> # include <sys/resource.h> #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include <sched.h> #endif
@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); }
-#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T
int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; }
-#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */
int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
Martin Kletzander wrote:
On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote:
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined
Makes sense, although even the sched_getaffinity() is guarded by the new _WITH_CPU_SET_T so there should be no error with the current code, am I
Nope, as I tried to explain in the commit message, the current code is not working properly as it's using cc.has_function() for 'sched_getaffinity' and this check passes because it's checking the specified function in the standard library, so it works regardless of _WITH_CPU_SET_T. And as it finds the sched_getaffinity(), the build still fails because we don't set _WITH_CPU_SET_T.
right? And if I understand correctly we cannot use the FreeBSD implementation as is because they do not have all the CPU_* macros we need, right?
Frankly speaking, I haven't yet tested this implementation. Currently, virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. Generally, it would be a good thing to use the same implementation for both Linux and FreeBSD, however, this Linux-compatible interface in FreeBSD is not yet available in any FreeBSD release, so we'll have to keep the old code for some time anyway, and also I need to test if it actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T with meson.
1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c4599301... https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b... https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2...
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 4 +++- src/util/virprocess.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [
# Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], + + [ 'sched.h', 'cpu_set_t' ], ]
if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include <sys/time.h> # include <sys/resource.h> #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include <sched.h> #endif
@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); }
-#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T
int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; }
-#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */
int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
Roman Bogorodskiy
On Mon, Nov 22, 2021 at 03:22:11PM +0400, Roman Bogorodskiy wrote:
Martin Kletzander wrote:
On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote:
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined
Makes sense, although even the sched_getaffinity() is guarded by the new _WITH_CPU_SET_T so there should be no error with the current code, am I
Nope, as I tried to explain in the commit message, the current code is not working properly as it's using cc.has_function() for 'sched_getaffinity' and this check passes because it's checking the specified function in the standard library, so it works regardless of _WITH_CPU_SET_T. And as it finds the sched_getaffinity(), the build still fails because we don't set _WITH_CPU_SET_T.
Looking at the patches you linked to it seems like sched_getaffinity() is hidden under #ifdef _WITH_CPU_SET_T, which made me think that cc.has_function() should not find it. Anyway the code is fine as it is, I was just wondering. Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
right? And if I understand correctly we cannot use the FreeBSD implementation as is because they do not have all the CPU_* macros we need, right?
Frankly speaking, I haven't yet tested this implementation. Currently, virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. Generally, it would be a good thing to use the same implementation for both Linux and FreeBSD, however, this Linux-compatible interface in FreeBSD is not yet available in any FreeBSD release, so we'll have to keep the old code for some time anyway, and also I need to test if it actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T with meson.
Maybe one day =)
1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c4599301... https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b... https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2...
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 4 +++- src/util/virprocess.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [
# Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], + + [ 'sched.h', 'cpu_set_t' ], ]
if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include <sys/time.h> # include <sys/resource.h> #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include <sched.h> #endif
@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); }
-#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T
int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; }
-#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */
int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
Roman Bogorodskiy
Martin Kletzander wrote:
On Mon, Nov 22, 2021 at 03:22:11PM +0400, Roman Bogorodskiy wrote:
Martin Kletzander wrote:
On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote:
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined
Makes sense, although even the sched_getaffinity() is guarded by the new _WITH_CPU_SET_T so there should be no error with the current code, am I
Nope, as I tried to explain in the commit message, the current code is not working properly as it's using cc.has_function() for 'sched_getaffinity' and this check passes because it's checking the specified function in the standard library, so it works regardless of _WITH_CPU_SET_T. And as it finds the sched_getaffinity(), the build still fails because we don't set _WITH_CPU_SET_T.
Looking at the patches you linked to it seems like sched_getaffinity() is hidden under #ifdef _WITH_CPU_SET_T, which made me think that cc.has_function() should not find it.
Apparently, has_function() doesn't need it to present in the header, at least as I understand from this snippet from meson-logs/meson-log.txt: Running compile: Working directory: /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0 Command line: ccache cc /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0/testfile.c -o /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0/output.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration -std=gnu99 Code: #define sched_getaffinity meson_disable_define_of_sched_getaffinity #include <limits.h> #undef sched_getaffinity #ifdef __cplusplus extern "C" #endif char sched_getaffinity (void); #if defined __stub_sched_getaffinity || defined __stub___sched_getaffinity fail fail fail this function is not going to work #endif int main(void) { return sched_getaffinity (); } Compiler stdout: Compiler stderr: Checking for function "sched_getaffinity" : YES
Anyway the code is fine as it is, I was just wondering.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Thanks, will push shortly. PS Studying meson-log.txt, I also noticed that detection of cpuset_getaffinity() is also not working properly, I'll address it separately.
right? And if I understand correctly we cannot use the FreeBSD implementation as is because they do not have all the CPU_* macros we need, right?
Frankly speaking, I haven't yet tested this implementation. Currently, virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. Generally, it would be a good thing to use the same implementation for both Linux and FreeBSD, however, this Linux-compatible interface in FreeBSD is not yet available in any FreeBSD release, so we'll have to keep the old code for some time anyway, and also I need to test if it actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T with meson.
Maybe one day =)
1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c4599301... https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b... https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2...
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 4 +++- src/util/virprocess.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [
# Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], + + [ 'sched.h', 'cpu_set_t' ], ]
if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include <sys/time.h> # include <sys/resource.h> #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include <sched.h> #endif
@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); }
-#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T
int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; }
-#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */
int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
Roman Bogorodskiy
Roman Bogorodskiy
participants (2)
-
Martin Kletzander -
Roman Bogorodskiy