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@xxxxxxxxxx>
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=43736b71dd051212d5c55be9fa21c45993017fbb
> >> >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2
> >> >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a
> >> >
> >> >Signed-off-by: Roman Bogorodskiy <bogorodskiy@xxxxxxxxx>
> >> >---
> >> > 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
Attachment:
signature.asc
Description: PGP signature
