diff --git a/glib/gbitlock.c b/glib/gbitlock.c index 900897517..a7a53ddd9 100644 --- a/glib/gbitlock.c +++ b/glib/gbitlock.c @@ -31,6 +31,7 @@ #include #include +#include "gtestutils.h" #include "gthreadprivate.h" #ifdef G_BIT_LOCK_FORCE_FUTEX_EMULATION @@ -175,7 +176,7 @@ g_futex_wake (const gint *address) static gint g_bit_lock_contended[CONTENTION_CLASSES]; /* (atomic) */ G_ALWAYS_INLINE static inline guint -bit_lock_contended_class (gpointer address) +bit_lock_contended_class (gconstpointer address) { return ((gsize) address) % G_N_ELEMENTS (g_bit_lock_contended); } @@ -186,6 +187,100 @@ bit_lock_contended_class (gpointer address) #endif #endif +static const gint *g_futex_int_address (const void *address); + +G_ALWAYS_INLINE static inline void +bit_lock_futex_wait (gconstpointer address, gboolean is_pointer_pointer, gint value) +{ + const guint CLASS = bit_lock_contended_class (address); + + g_atomic_int_add (&g_bit_lock_contended[CLASS], +1); + if (is_pointer_pointer) + address = g_futex_int_address (address); + g_futex_wait (address, value); + g_atomic_int_add (&g_bit_lock_contended[CLASS], -1); +} + +G_ALWAYS_INLINE static inline void +bit_lock_futex_maybe_wake (gconstpointer address, gboolean is_pointer_pointer) +{ + const guint CLASS = bit_lock_contended_class (address); + + /* Warning: unlocking may allow another thread to proceed and destroy the + * memory that @address points to. We thus must not dereference it anymore. + */ + + if (g_atomic_int_get (&g_bit_lock_contended[CLASS])) + { + if (is_pointer_pointer) + address = g_futex_int_address (address); + g_futex_wake (address); + } +} + +/** + * g_bit_lock_and_get: + * @address: a pointer to an integer + * @lock_bit: a bit value between 0 and 31 + * @out_val: (out) (optional): return location for the new value of the integer + * + * Sets the indicated @lock_bit in @address and atomically returns the new value. + * + * This is like [func@GLib.bit_lock], except it can atomically return the new value at + * @address (right after obtaining the lock). Thus the value returned in @out_val + * always has the @lock_bit set. + * + * Since: 2.86 + **/ +void +g_bit_lock_and_get (gint *address, + guint lock_bit, + gint *out_val) +{ + const guint MASK = 1u << lock_bit; + guint v; + +#ifdef G_ENABLE_DEBUG + g_assert (lock_bit < 32u); +#endif + +#ifdef USE_ASM_GOTO + if (G_LIKELY (!out_val)) + { + while (TRUE) + { + __asm__ volatile goto("lock bts %1, (%0)\n" + "jc %l[contended]" + : /* no output */ + : "r"(address), "r"(lock_bit) + : "cc", "memory" + : contended); + return; + + contended: + { + guint v; + + v = (guint) g_atomic_int_get (address); + if (v & MASK) + bit_lock_futex_wait (address, FALSE, (gint) v); + } + } + } +#endif + +retry: + v = g_atomic_int_or ((guint *) address, MASK); + if (v & MASK) + { + bit_lock_futex_wait (address, FALSE, (gint) v); + goto retry; + } + + if (out_val) + *out_val = (gint) (v | MASK); +} + /** * g_bit_lock: * @address: a pointer to an integer @@ -212,52 +307,7 @@ void g_bit_lock (volatile gint *address, gint lock_bit) { - gint *address_nonvolatile = (gint *) address; - -#ifdef USE_ASM_GOTO - retry: - __asm__ volatile goto ("lock bts %1, (%0)\n" - "jc %l[contended]" - : /* no output */ - : "r" (address), "r" (lock_bit) - : "cc", "memory" - : contended); - return; - - contended: - { - guint mask = 1u << lock_bit; - guint v; - - v = (guint) g_atomic_int_get (address_nonvolatile); - if (v & mask) - { - guint class = bit_lock_contended_class (address_nonvolatile); - - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (address_nonvolatile, v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); - } - } - goto retry; -#else - guint mask = 1u << lock_bit; - guint v; - - retry: - v = g_atomic_int_or (address_nonvolatile, mask); - if (v & mask) - /* already locked */ - { - guint class = bit_lock_contended_class (address_nonvolatile); - - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (address_nonvolatile, v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); - - goto retry; - } -#endif + g_bit_lock_and_get ((gint *) address, (guint) lock_bit, NULL); } /** @@ -345,15 +395,62 @@ g_bit_unlock (volatile gint *address, /* Warning: unlocking may allow another thread to proceed and destroy the * memory that @address points to. We thus must not dereference it anymore. */ - - { - guint class = bit_lock_contended_class (address_nonvolatile); - - if (g_atomic_int_get (&g_bit_lock_contended[class])) - g_futex_wake (address_nonvolatile); - } + bit_lock_futex_maybe_wake (address_nonvolatile, FALSE); } +/** + * g_bit_unlock_and_set: + * @address: a pointer to an integer + * @lock_bit: a bit value between 0 and 31 + * @val: the new value to set + * @preserve_mask: mask for bits from @address to preserve + * + * This is like [func@GLib.bit_unlock]() but also atomically sets @address to + * @val. + * + * If @preserve_mask is not zero, then the @preserve_mask bits will be + * preserved in @address and are not set to @val. + * + * Note that the @lock_bit bit will be always unset regardless of + * @val, @preserve_mask and the currently set value in @address. + * + * Since: 2.86 + **/ +void +g_bit_unlock_and_set (gint *address, + guint lock_bit, + gint val, + gint preserve_mask) + +{ + const guint MASK = 1u << lock_bit; + +#ifdef G_ENABLE_DEBUG + g_assert (lock_bit < 32u); +#endif + + if (G_UNLIKELY (preserve_mask != 0)) + { + guint old_val; + guint new_val; + + old_val = (guint) g_atomic_int_get (address); + + again: + new_val = ((old_val & ((guint) preserve_mask)) | (((guint) val) & ~((guint) preserve_mask))) & ~MASK; + if (!g_atomic_int_compare_and_exchange_full (address, (gint) old_val, (gint) new_val, (gint *) &old_val)) + goto again; + } + else + { + g_atomic_int_set (address, (gint) (((guint) val) & ~MASK)); + } + + /* Warning: unlocking may allow another thread to proceed and destroy the + * memory that @address points to. We thus must not dereference it anymore. + */ + bit_lock_futex_maybe_wake (address, FALSE); +} /* We emulate pointer-sized futex(2) because the kernel API only * supports integers. @@ -379,7 +476,7 @@ g_bit_unlock (volatile gint *address, * * g_futex_wake (g_futex_int_address (int_address)); */ -static const gint * +G_ALWAYS_INLINE static inline const gint * g_futex_int_address (const void *address) { const gint *int_address = address; @@ -444,7 +541,6 @@ void guint lock_bit, guintptr *out_ptr) { - guint class = bit_lock_contended_class (address); guintptr mask; guintptr v; @@ -468,11 +564,7 @@ void contended: v = (guintptr) g_atomic_pointer_get ((gpointer *) address); if (v & mask) - { - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (g_futex_int_address (address), v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); - } + bit_lock_futex_wait (address, TRUE, (gint) v); } } #endif @@ -480,11 +572,8 @@ void retry: v = g_atomic_pointer_or ((gpointer *) address, mask); if (v & mask) - /* already locked */ { - g_atomic_int_add (&g_bit_lock_contended[class], +1); - g_futex_wait (g_futex_int_address (address), (guint) v); - g_atomic_int_add (&g_bit_lock_contended[class], -1); + bit_lock_futex_wait (address, TRUE, (gint) v); goto retry; } @@ -606,13 +695,7 @@ void /* Warning: unlocking may allow another thread to proceed and destroy the * memory that @address points to. We thus must not dereference it anymore. */ - - { - guint class = bit_lock_contended_class (address_nonvolatile); - - if (g_atomic_int_get (&g_bit_lock_contended[class])) - g_futex_wake (g_futex_int_address (address_nonvolatile)); - } + bit_lock_futex_maybe_wake (address_nonvolatile, TRUE); } } @@ -653,8 +736,8 @@ g_pointer_bit_lock_mask_ptr (gpointer ptr, guint lock_bit, gboolean set, guintpt * @ptr: the new pointer value to set * @preserve_mask: if non-zero, those bits of the current pointer in @address * are preserved. - * Note that the @lock_bit bit will be always set according to @set, - * regardless of @preserve_mask and the currently set value in @address. + * Note that the @lock_bit bit will be always unset regardless of + * @ptr, @preserve_mask and the currently set value in @address. * * This is equivalent to g_pointer_bit_unlock() and atomically setting * the pointer value. @@ -672,7 +755,6 @@ void (g_pointer_bit_unlock_and_set) (void *address, guintptr preserve_mask) { gpointer *pointer_address = address; - guint class = bit_lock_contended_class (address); gpointer ptr2; g_return_if_fail (lock_bit < 32u); @@ -692,8 +774,7 @@ void (g_pointer_bit_unlock_and_set) (void *address, g_atomic_pointer_set (pointer_address, ptr2); } - if (g_atomic_int_get (&g_bit_lock_contended[class]) > 0) - g_futex_wake (g_futex_int_address (address)); + bit_lock_futex_maybe_wake (address, TRUE); /* It makes no sense, if unlocking mangles the pointer. Assert against * that. diff --git a/glib/gbitlock.h b/glib/gbitlock.h index f44a52c37..1411157d4 100644 --- a/glib/gbitlock.h +++ b/glib/gbitlock.h @@ -34,6 +34,11 @@ G_BEGIN_DECLS GLIB_AVAILABLE_IN_ALL void g_bit_lock (volatile gint *address, gint lock_bit); +GLIB_AVAILABLE_IN_2_86 +void g_bit_lock_and_get (gint *address, + guint lock_bit, + gint *out_val); + GLIB_AVAILABLE_IN_ALL gboolean g_bit_trylock (volatile gint *address, gint lock_bit); @@ -41,6 +46,12 @@ GLIB_AVAILABLE_IN_ALL void g_bit_unlock (volatile gint *address, gint lock_bit); +GLIB_AVAILABLE_IN_2_86 +void g_bit_unlock_and_set (gint *address, + guint lock_bit, + gint new_val, + gint preserve_mask); + GLIB_AVAILABLE_IN_ALL void g_pointer_bit_lock (volatile void *address, gint lock_bit); diff --git a/glib/tests/1bit-mutex.c b/glib/tests/1bit-mutex.c index f6a90f0bd..2904753c7 100644 --- a/glib/tests/1bit-mutex.c +++ b/glib/tests/1bit-mutex.c @@ -47,8 +47,10 @@ #undef g_pointer_bit_unlock_and_set #define g_bit_lock _emufutex_g_bit_lock + #define g_bit_lock_and_get _emufutex_g_bit_lock_and_get #define g_bit_trylock _emufutex_g_bit_trylock #define g_bit_unlock _emufutex_g_bit_unlock + #define g_bit_unlock_and_set _emufutex_g_bit_unlock_and_set #define g_pointer_bit_lock _emufutex_g_pointer_bit_lock #define g_pointer_bit_lock_and_get _emufutex_g_pointer_bit_lock_and_get #define g_pointer_bit_trylock _emufutex_g_pointer_bit_trylock diff --git a/glib/tests/bitlock.c b/glib/tests/bitlock.c index a21672099..df30cdfdb 100644 --- a/glib/tests/bitlock.c +++ b/glib/tests/bitlock.c @@ -1,5 +1,7 @@ #include +#include "gtestutils.h" + static void test_bitlocks (void) { @@ -28,12 +30,147 @@ test_bitlocks (void) } } +#define PARALLEL_N_THREADS 5 +#define PARALLEL_LOCKBIT 31 +#define PARALLEL_TOGGLEBIT 30 +#define PARALLEL_SETBIT 29 +#define PARALLEL_LOCKMASK (1 << PARALLEL_LOCKBIT) +#define PARALLEL_TOGGLEMASK (1 << PARALLEL_TOGGLEBIT) +#define PARALLEL_SETMASK (1 << PARALLEL_SETBIT) +#define PARALLEL_MAX_COUNT_SELF 500 +#define PARALLEL_MAX_COUNT_ALL (10 * PARALLEL_MAX_COUNT_SELF) +static int parallel_thread_ready = 0; +static int parallel_atomic = 0; + +static void +_test_parallel_randomly_toggle (void) +{ + if (g_random_boolean ()) + g_atomic_int_or (¶llel_atomic, PARALLEL_TOGGLEMASK); + else + g_atomic_int_and (¶llel_atomic, ~PARALLEL_TOGGLEMASK); +} + +static gpointer +_test_parallel_run (gpointer thread_arg) +{ + const int IDX = GPOINTER_TO_INT (thread_arg); + int count_self = 0; + + (void) IDX; + + g_atomic_int_inc (¶llel_thread_ready); + while (g_atomic_int_get (¶llel_thread_ready) != PARALLEL_N_THREADS) + g_usleep (10); + + while (TRUE) + { + gint val; + gint val2; + gint new_val; + gint count_all; + + _test_parallel_randomly_toggle (); + + /* take a lock. */ + if (g_random_boolean ()) + { + g_bit_lock (¶llel_atomic, PARALLEL_LOCKBIT); + val = g_atomic_int_get (¶llel_atomic); + } + else + { + g_bit_lock_and_get (¶llel_atomic, PARALLEL_LOCKBIT, &val); + } + + _test_parallel_randomly_toggle (); + + /* the toggle bit is random. Clear it. */ + val &= ~PARALLEL_TOGGLEMASK; + + /* these bits must be set. */ + g_assert_true (val & PARALLEL_LOCKMASK); + g_assert_true (val & PARALLEL_SETMASK); + + /* If we fetch again, we must get the same value. Nobody changes the + * value while we hold the lock, except for the toggle bit. */ + val2 = g_atomic_int_get (¶llel_atomic); + val2 &= ~PARALLEL_TOGGLEMASK; + g_assert_cmpint (val, ==, val2); + + count_all = (val & ~(PARALLEL_LOCKMASK | PARALLEL_SETMASK)); + + if ((g_random_int () % 5) == 0) + { + /* regular unlock without any changes. */ + g_bit_unlock (¶llel_atomic, PARALLEL_LOCKBIT); + continue; + } + + /* unlock-and-set with an increased counter. */ + new_val = MIN (count_all + 1, PARALLEL_MAX_COUNT_ALL); + if (g_random_boolean ()) + new_val |= PARALLEL_SETMASK; + if (g_random_boolean ()) + new_val |= PARALLEL_TOGGLEMASK; + + g_bit_unlock_and_set (¶llel_atomic, PARALLEL_LOCKBIT, new_val, ((new_val & PARALLEL_SETMASK) && g_random_boolean ()) ? 0 : PARALLEL_SETMASK); + + count_self++; + + if (count_self < PARALLEL_MAX_COUNT_SELF) + continue; + if (count_all < PARALLEL_MAX_COUNT_ALL) + continue; + + break; + } + + /* To indicate success, we return a pointer to ¶llel_atomic. */ + return ¶llel_atomic; +} + +static void +test_parallel (void) +{ + GThread *threads[PARALLEL_N_THREADS]; + gpointer ptr; + int i; + gint val; + + g_atomic_int_or (¶llel_atomic, PARALLEL_SETMASK); + + for (i = 0; i < PARALLEL_N_THREADS; i++) + { + threads[i] = g_thread_new ("bitlock-parallel", _test_parallel_run, GINT_TO_POINTER (i)); + } + + for (i = 0; i < PARALLEL_N_THREADS; i++) + { + ptr = g_thread_join (threads[i]); + g_assert_true (ptr == ¶llel_atomic); + + /* After we join the first thread, we already expect that the resulting + * atomic's counter is set to PARALLEL_MAX_COUNT_ALL. This stays until + * the end. */ + val = g_atomic_int_get (¶llel_atomic); + if (i == PARALLEL_N_THREADS - 1) + { + /* at last, the atomic must be unlocked. */ + g_assert_true (!(val & PARALLEL_LOCKMASK)); + } + val &= ~(PARALLEL_LOCKMASK | PARALLEL_TOGGLEMASK | PARALLEL_SETMASK); + g_assert_cmpint (val, ==, PARALLEL_MAX_COUNT_ALL); + } +} + int main (int argc, char **argv) { g_test_init (&argc, &argv, NULL); g_test_add_func ("/bitlock/performance/uncontended", test_bitlocks); + g_test_add_func ("/bitlock/performance/parallel", test_parallel); return g_test_run (); }