gbitlock: add g_pointer_bit_lock_and_get()

Usually, after g_pointer_bit_lock() we want to read the pointer that we
have. In many cases, when we g_pointer_bit_lock() a pointer, we can
access it afterwards without atomic, as nobody is going to modify the
pointer then.

However, gdataset also supports g_datalist_set_flags(), so the pointer
may change at any time and we must always use atomics to read it. For
that reason, g_datalist_lock_and_get() does an atomic read right after
g_pointer_bit_lock().

g_pointer_bit_lock() can easily access the value that it just set. Add
g_pointer_bit_lock_and_get() which can return the value that gets set
afterwards.

Aside from saving the second atomic-get in certain scenarios, the
returned value is also atomically the one that we just set.

glib/gbitlock.c

@@ -412,6 +412,76 @@ pointer_bit_lock_mask_ptr (gpointer ptr, guint lock_bit, gboolean set, guintptr
     return (gpointer) (x_ptr & ~lock_mask);
 }
 
+/**
+ * g_pointer_bit_lock_and_get:
+ * @address: (not nullable): a pointer to a #gpointer-sized value
+ * @lock_bit: a bit value between 0 and 31
+ * @out_ptr: (out) (optional): returns the set pointer atomically.
+ *   This is the value after setting the lock, it thus always has the
+ *   lock bit set, while previously @address had the lockbit unset.
+ *   You may also use g_pointer_bit_lock_mask_ptr() to clear the lock bit.
+ *
+ * This is equivalent to g_bit_lock, but working on pointers (or other
+ * pointer-sized values).
+ *
+ * For portability reasons, you may only lock on the bottom 32 bits of
+ * the pointer.
+ *
+ * Since: 2.80
+ **/
+void
+(g_pointer_bit_lock_and_get) (gpointer address,
+                              guint lock_bit,
+                              guintptr *out_ptr)
+{
+  guint class = ((gsize) address) % G_N_ELEMENTS (g_bit_lock_contended);
+  guintptr mask;
+  guintptr v;
+
+  g_return_if_fail (lock_bit < 32);
+
+  mask = 1u << lock_bit;
+
+#ifdef USE_ASM_GOTO
+  if (G_LIKELY (!out_ptr))
+    {
+      while (TRUE)
+        {
+          __asm__ volatile goto ("lock bts %1, (%0)\n"
+                                 "jc %l[contended]"
+                                 : /* no output */
+                                 : "r"(address), "r"((gsize) lock_bit)
+                                 : "cc", "memory"
+                                 : contended);
+          return;
+
+        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);
+            }
+        }
+    }
+#endif
+
+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);
+      goto retry;
+    }
+
+  if (out_ptr)
+    *out_ptr = (v | mask);
+}
+
 /**
  * g_pointer_bit_lock:
  * @address: (not nullable): a pointer to a #gpointer-sized value
@@ -430,60 +500,9 @@ pointer_bit_lock_mask_ptr (gpointer ptr, guint lock_bit, gboolean set, guintptr
  **/
 void
 (g_pointer_bit_lock) (volatile void *address,
-                      gint           lock_bit)
+                      gint lock_bit)
 {
-  void *address_nonvolatile = (void *) address;
+  g_pointer_bit_lock_and_get ((gpointer *) address, (guint) lock_bit, NULL);
-
-  g_return_if_fail (lock_bit < 32);
-
-  {
-#ifdef USE_ASM_GOTO
- retry:
-    __asm__ volatile goto ("lock bts %1, (%0)\n"
-                           "jc %l[contended]"
-                           : /* no output */
-                           : "r" (address), "r" ((gsize) lock_bit)
-                           : "cc", "memory"
-                           : contended);
-    return;
-
- contended:
-    {
-      gpointer *pointer_address = address_nonvolatile;
-      gsize mask = 1u << lock_bit;
-      gsize v;
-
-      v = (gsize) g_atomic_pointer_get (pointer_address);
-      if (v & mask)
-        {
-          guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended);
-
-          g_atomic_int_add (&g_bit_lock_contended[class], +1);
-          g_futex_wait (g_futex_int_address (address_nonvolatile), v);
-          g_atomic_int_add (&g_bit_lock_contended[class], -1);
-        }
-    }
-    goto retry;
-#else
-  gpointer *pointer_address = address_nonvolatile;
-  gsize mask = 1u << lock_bit;
-  guintptr v;
-
- retry:
-  v = g_atomic_pointer_or (pointer_address, mask);
-  if (v & mask)
-    /* already locked */
-    {
-      guint class = ((gsize) address_nonvolatile) % G_N_ELEMENTS (g_bit_lock_contended);
-
-      g_atomic_int_add (&g_bit_lock_contended[class], +1);
-      g_futex_wait (g_futex_int_address (address_nonvolatile), (guint) v);
-      g_atomic_int_add (&g_bit_lock_contended[class], -1);
-
-      goto retry;
-    }
-#endif
-  }
 }
 
 /**

glib/gbitlock.h

@@ -44,6 +44,12 @@ void      g_bit_unlock                    (volatile gint *address,
 GLIB_AVAILABLE_IN_ALL
 void      g_pointer_bit_lock              (volatile void *address,
                                            gint           lock_bit);
+
+GLIB_AVAILABLE_IN_2_80
+void      g_pointer_bit_lock_and_get      (gpointer address,
+                                           guint lock_bit,
+                                           guintptr *out_ptr);
+
 GLIB_AVAILABLE_IN_ALL
 gboolean  g_pointer_bit_trylock           (volatile void *address,
                                            gint           lock_bit);
@@ -72,6 +78,12 @@ void g_pointer_bit_unlock_and_set         (void *address,
     g_pointer_bit_lock ((address), (lock_bit));                              \
   }))
 
+#define g_pointer_bit_lock_and_get(address, lock_bit, out_ptr)     \
+  (G_GNUC_EXTENSION ({                                             \
+    G_STATIC_ASSERT (sizeof *(address) == sizeof (gpointer));      \
+    g_pointer_bit_lock_and_get ((address), (lock_bit), (out_ptr)); \
+  }))
+
 #define g_pointer_bit_trylock(address, lock_bit) \
   (G_GNUC_EXTENSION ({                                                       \
     G_STATIC_ASSERT (sizeof *(address) == sizeof (gpointer));                \

glib/tests/1bit-mutex.c

@@ -41,6 +41,7 @@
      defining our own version of the g_bit_*lock symbols
    */
   #undef g_pointer_bit_lock
+  #undef g_pointer_bit_lock_and_get
   #undef g_pointer_bit_trylock
   #undef g_pointer_bit_unlock
   #undef g_pointer_bit_unlock_and_set
@@ -49,6 +50,7 @@
   #define g_bit_trylock                 _emufutex_g_bit_trylock
   #define g_bit_unlock                  _emufutex_g_bit_unlock
   #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
   #define g_pointer_bit_unlock          _emufutex_g_pointer_bit_unlock
   #define g_pointer_bit_lock_mask_ptr   _emufutex_g_pointer_bit_lock_mask_ptr

gobject/tests/threadtests.c

@@ -515,6 +515,7 @@ test_threaded_g_pointer_bit_unlock_and_set (void)
   GObject *obj;
   gpointer plock;
   gpointer ptr;
+  guintptr ptr2;
   gpointer mangled_obj;
 
 #if defined(__GNUC__)
@@ -548,14 +549,15 @@ test_threaded_g_pointer_bit_unlock_and_set (void)
   g_assert_true (plock == obj);
 
   plock = obj;
-  g_pointer_bit_lock (&plock, 0);
+  g_pointer_bit_lock_and_get (&plock, 0, &ptr2);
+  g_assert_true ((gpointer) ptr2 == plock);
   g_assert_true (plock != obj);
   g_atomic_pointer_set (&plock, mangled_obj);
   g_pointer_bit_unlock_and_set (&plock, 0, obj, 0);
   g_assert_true (plock == obj);
 
   plock = obj;
-  g_pointer_bit_lock (&plock, 0);
+  g_pointer_bit_lock_and_get (&plock, 0, NULL);
   g_assert_true (plock != obj);
   g_atomic_pointer_set (&plock, mangled_obj);
   g_pointer_bit_unlock_and_set (&plock, 0, obj, 0x7);