From f67e8636dad3038b584887bd1edf4515fbd7ac4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 13 Dec 2022 15:05:06 +0200 Subject: [PATCH 1/2] glib/gthread-posix: Use `cc.compiles()` instead of `cc.links()` for checking for `__NR_futex` `cc.compiles()` is minimally faster. We only want to check here whether `__NR_futex` is defined and don't want to check anything at link-time. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 2305b3a08e..c763857ea9 100644 --- a/meson.build +++ b/meson.build @@ -887,7 +887,7 @@ if host_system == 'qnx' endif # Check for futex(2) -if cc.links('''#include +if cc.compiles('''#include #include #include int main (int argc, char ** argv) { -- GitLab From a79c6af23eff5ee978db62e048828c9a992a1261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 12 Dec 2022 19:58:21 +0200 Subject: [PATCH 2/2] glib/gthread-posix: Conditionally use `futex` and/or `futex_time64` syscalls as necessary and use the correct `struct timespec` definition On some systems only `futex_time64` exists (e.g. riscv32) while on others only `futex` exists (old Linux, 64 bit platforms), so it is necessary to check for both and try calling both at runtime. Additionally use the correct `struct timespec` definition. There is not necessarily any relation between the libc's definition and the kernel's. Specifically, the libc headers might use 64-bit `time_t` while the kernel headers use 32-bit `__kernel_old_time_t` on certain systems. To get around this problem we a) check if `futex_time64` is available, which only exists on 32-bit platforms and always uses 64-bit `time_t`. b) otherwise (or if that returns `ENOSYS`), we call the normal `futex` syscall with the `struct timespec` used by the kernel, which uses `__kernel_long_t` for both its fields. We use that instead of `__kernel_old_time_t` because it is equivalent and available in the kernel headers for a longer time. --- glib/gbitlock.c | 15 ++---- glib/gthread-posix.c | 120 ++++++++++++++++++++++++++---------------- glib/gthreadprivate.h | 49 +++++++++++++++++ meson.build | 9 ++++ 4 files changed, 137 insertions(+), 56 deletions(-) diff --git a/glib/gbitlock.c b/glib/gbitlock.c index 81bfb339f3..9c34de80c8 100644 --- a/glib/gbitlock.c +++ b/glib/gbitlock.c @@ -35,6 +35,7 @@ #ifdef G_BIT_LOCK_FORCE_FUTEX_EMULATION #undef HAVE_FUTEX +#undef HAVE_FUTEX_TIME64 #endif #ifndef HAVE_FUTEX @@ -42,7 +43,7 @@ static GMutex g_futex_mutex; static GSList *g_futex_address_list = NULL; #endif -#ifdef HAVE_FUTEX +#if defined(HAVE_FUTEX) || defined(HAVE_FUTEX_TIME64) /* * We have headers for futex(2) on the build machine. This does not * imply that every system that ever runs the resulting glib will have @@ -51,14 +52,6 @@ static GSList *g_futex_address_list = NULL; * * If anyone actually gets bit by this, please file a bug. :) */ -#include -#include -#include - -#ifndef FUTEX_WAIT_PRIVATE -#define FUTEX_WAIT_PRIVATE FUTEX_WAIT -#define FUTEX_WAKE_PRIVATE FUTEX_WAKE -#endif /* < private > * g_futex_wait: @@ -81,7 +74,7 @@ static void g_futex_wait (const gint *address, gint value) { - syscall (__NR_futex, address, (gsize) FUTEX_WAIT_PRIVATE, (gsize) value, NULL); + g_futex_simple (address, (gsize) FUTEX_WAIT_PRIVATE, (gsize) value, NULL); } /* < private > @@ -98,7 +91,7 @@ g_futex_wait (const gint *address, static void g_futex_wake (const gint *address) { - syscall (__NR_futex, address, (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); + g_futex_simple (address, (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); } #else diff --git a/glib/gthread-posix.c b/glib/gthread-posix.c index d96fca5ca2..84f62881d8 100644 --- a/glib/gthread-posix.c +++ b/glib/gthread-posix.c @@ -74,7 +74,7 @@ #include #endif -#if defined(HAVE_FUTEX) && \ +#if (defined(HAVE_FUTEX) || defined(HAVE_FUTEX_TIME64)) && \ (defined(HAVE_STDATOMIC_H) || defined(__ATOMIC_SEQ_CST)) #define USE_NATIVE_MUTEX #endif @@ -1397,15 +1397,6 @@ g_system_thread_set_name (const gchar *name) /* {{{1 GMutex and GCond futex implementation */ #if defined(USE_NATIVE_MUTEX) - -#include -#include - -#ifndef FUTEX_WAIT_PRIVATE -#define FUTEX_WAIT_PRIVATE FUTEX_WAIT -#define FUTEX_WAKE_PRIVATE FUTEX_WAKE -#endif - /* We should expand the set of operations available in gatomic once we * have better C11 support in GCC in common distributions (ie: 4.9). * @@ -1500,8 +1491,8 @@ g_mutex_lock_slowpath (GMutex *mutex) */ while (exchange_acquire (&mutex->i[0], G_MUTEX_STATE_CONTENDED) != G_MUTEX_STATE_EMPTY) { - syscall (__NR_futex, &mutex->i[0], (gsize) FUTEX_WAIT_PRIVATE, - G_MUTEX_STATE_CONTENDED, NULL); + g_futex_simple (&mutex->i[0], (gsize) FUTEX_WAIT_PRIVATE, + G_MUTEX_STATE_CONTENDED, NULL); } } @@ -1519,7 +1510,7 @@ g_mutex_unlock_slowpath (GMutex *mutex, g_abort (); } - syscall (__NR_futex, &mutex->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); + g_futex_simple (&mutex->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); } void @@ -1587,7 +1578,7 @@ g_cond_wait (GCond *cond, guint sampled = (guint) g_atomic_int_get (&cond->i[0]); g_mutex_unlock (mutex); - syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, NULL); + g_futex_simple (&cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, NULL); g_mutex_lock (mutex); } @@ -1596,7 +1587,7 @@ g_cond_signal (GCond *cond) { g_atomic_int_inc (&cond->i[0]); - syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); + g_futex_simple (&cond->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) 1, NULL); } void @@ -1604,7 +1595,7 @@ g_cond_broadcast (GCond *cond) { g_atomic_int_inc (&cond->i[0]); - syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) INT_MAX, NULL); + g_futex_simple (&cond->i[0], (gsize) FUTEX_WAKE_PRIVATE, (gsize) INT_MAX, NULL); } gboolean @@ -1614,12 +1605,6 @@ g_cond_wait_until (GCond *cond, { struct timespec now; struct timespec span; -#ifdef __NR_futex_time64 - long span_arg[2]; - G_STATIC_ASSERT (sizeof (span_arg[0]) == 4); -#else - struct timespec span_arg; -#endif guint sampled; int res; @@ -1640,37 +1625,82 @@ g_cond_wait_until (GCond *cond, if (span.tv_sec < 0) return FALSE; - /* On x32 (ILP32 ABI on x86_64) and potentially sparc64, the raw futex() - * syscall takes a 32-bit timespan argument *regardless* of whether userspace - * is using 32-bit or 64-bit `struct timespec`. This means that we can’t - * unconditionally pass a `struct timespec` pointer into the syscall. + /* `struct timespec` as defined by the libc headers does not necessarily + * have any relation to the one used by the kernel for the `futex` syscall. * - * Assume that any such platform is new enough to define the - * `__NR_futex_time64` workaround syscall (which accepts 64-bit timespecs, - * introduced in kernel 5.1), and use that as a proxy for whether to pass in - * `long[2]` or `struct timespec`. + * Specifically, the libc headers might use 64-bit `time_t` while the kernel + * headers use 32-bit `__kernel_old_time_t` on certain systems. * - * As per https://lwn.net/Articles/776427/, the `time64` syscalls only exist - * on 32-bit platforms, so in this case `sizeof(long)` should always be - * 32 bits. + * To get around this problem we + * a) check if `futex_time64` is available, which only exists on 32-bit + * platforms and always uses 64-bit `time_t`. + * b) otherwise (or if that returns `ENOSYS`), we call the normal `futex` + * syscall with the `struct timespec` used by the kernel, which uses + * `__kernel_long_t` for both its fields. We use that instead of + * `__kernel_old_time_t` because it is equivalent and available in the + * kernel headers for a longer time. * - * Don’t bother actually calling `__NR_futex_time64` as the `span` is relative - * and hence very unlikely to overflow, even if using 32-bit longs. + * Also some 32-bit systems do not define `__NR_futex` at all and only + * define `__NR_futex_time64`. */ -#ifdef __NR_futex_time64 - span_arg[0] = span.tv_sec; - span_arg[1] = span.tv_nsec; -#else - span_arg = span; -#endif sampled = cond->i[0]; g_mutex_unlock (mutex); - res = syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, &span_arg); - success = (res < 0 && errno == ETIMEDOUT) ? FALSE : TRUE; - g_mutex_lock (mutex); - return success; +#ifdef __NR_futex_time64 + { + struct + { + gint64 tv_sec; + gint64 tv_nsec; + } span_arg; + + span_arg.tv_sec = span.tv_sec; + span_arg.tv_nsec = span.tv_nsec; + + res = syscall (__NR_futex_time64, &cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, &span_arg); + + /* If the syscall does not exist (`ENOSYS`), we retry again below with the + * normal `futex` syscall. This can happen if newer kernel headers are + * used than the kernel that is actually running. + */ +# ifdef __NR_futex + if (res >= 0 || errno != ENOSYS) +# endif /* defined(__NR_futex) */ + { + success = (res < 0 && errno == ETIMEDOUT) ? FALSE : TRUE; + g_mutex_lock (mutex); + + return success; + } + } +#endif + +#ifdef __NR_futex + { + struct + { + __kernel_long_t tv_sec; + __kernel_long_t tv_nsec; + } span_arg; + + /* Make sure to only ever call this if the end time actually fits into the target type */ + if (G_UNLIKELY (sizeof (__kernel_long_t) < 8 && span.tv_sec > G_MAXINT32)) + g_error ("%s: Can’t wait for more than %us", G_STRFUNC, G_MAXINT32); + + span_arg.tv_sec = span.tv_sec; + span_arg.tv_nsec = span.tv_nsec; + + res = syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, &span_arg); + success = (res < 0 && errno == ETIMEDOUT) ? FALSE : TRUE; + g_mutex_lock (mutex); + + return success; + } +#endif /* defined(__NR_futex) */ + + /* We can't end up here because of the checks above */ + g_assert_not_reached (); } #endif diff --git a/glib/gthreadprivate.h b/glib/gthreadprivate.h index 2ae705d4f5..6eaf422753 100644 --- a/glib/gthreadprivate.h +++ b/glib/gthreadprivate.h @@ -40,6 +40,55 @@ struct _GRealThread /* system thread implementation (gthread-posix.c, gthread-win32.c) */ +#if defined(HAVE_FUTEX) || defined(HAVE_FUTEX_TIME64) +#include +#include +#include + +#ifndef FUTEX_WAIT_PRIVATE +#define FUTEX_WAIT_PRIVATE FUTEX_WAIT +#define FUTEX_WAKE_PRIVATE FUTEX_WAKE +#endif + +/* Wrapper macro to call `futex_time64` and/or `futex` with simple + * parameters and without returning the return value. + * + * If the `futex_time64` syscall does not exist (`ENOSYS`), we retry again + * with the normal `futex` syscall. This can happen if newer kernel headers + * are used than the kernel that is actually running. + * + * This must not be called with a timeout parameter as that differs + * in size between the two syscall variants! + */ +#if defined(__NR_futex) && defined(__NR_futex_time64) +#define g_futex_simple(uaddr, futex_op, ...) \ + G_STMT_START \ + { \ + int res = syscall (__NR_futex_time64, uaddr, (gsize) futex_op, __VA_ARGS__); \ + if (res < 0 && errno == ENOSYS) \ + syscall (__NR_futex, uaddr, (gsize) futex_op, __VA_ARGS__); \ + } \ + G_STMT_END +#elif defined(__NR_futex_time64) +#define g_futex_simple(uaddr, futex_op, ...) \ + G_STMT_START \ + { \ + syscall (__NR_futex_time64, uaddr, (gsize) futex_op, __VA_ARGS__); \ + } \ + G_STMT_END +#elif defined(__NR_futex) +#define g_futex_simple(uaddr, futex_op, ...) \ + G_STMT_START \ + { \ + syscall (__NR_futex, uaddr, (gsize) futex_op, __VA_ARGS__); \ + } \ + G_STMT_END +#else /* !defined(__NR_futex) && !defined(__NR_futex_time64) */ +#error "Neither __NR_futex nor __NR_futex_time64 are defined but were found by meson" +#endif /* defined(__NR_futex) && defined(__NR_futex_time64) */ + +#endif + /* Platform-specific scheduler settings for a thread */ typedef struct { diff --git a/meson.build b/meson.build index c763857ea9..75a3db504e 100644 --- a/meson.build +++ b/meson.build @@ -896,6 +896,15 @@ if cc.compiles('''#include }''', name : 'futex(2) system call') glib_conf.set('HAVE_FUTEX', 1) endif +if cc.compiles('''#include + #include + #include + int main (int argc, char ** argv) { + syscall (__NR_futex_time64, NULL, FUTEX_WAKE, FUTEX_WAIT); + return 0; + }''', name : 'futex(2) system call') + glib_conf.set('HAVE_FUTEX_TIME64', 1) +endif # Check for eventfd(2) if cc.links('''#include -- GitLab