[v2,5/6] nptl: Rename __wrefs to __crefs because its meaning has changed

Message ID 20210908025239.1326480-6-malteskarupke@web.de
State Superseded
Headers
Series [v2,1/6] nptl: Fix pthread_cond_signal missing a sleeper (#BZ 25847) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Malte Skarupke Sept. 8, 2021, 2:52 a.m. UTC
  When I remove the increment/decrement of wrefs in pthread_cond_wait,
it no longer really had the meaning of representing the number of
waiters. It is still used as a reference count for threads that call
pthread_cancel, so crefs it is.
---
 nptl/nptl-printers.py                   |  6 +++---
 nptl/pthread_cond_broadcast.c           |  2 +-
 nptl/pthread_cond_common.c              |  4 ++--
 nptl/pthread_cond_destroy.c             | 10 +++++-----
 nptl/pthread_cond_init.c                |  4 ++--
 nptl/pthread_cond_signal.c              |  2 +-
 nptl/pthread_cond_wait.c                | 16 ++++++++--------
 nptl/tst-cond22.c                       |  4 ++--
 sysdeps/nptl/bits/thread-shared-types.h |  2 +-
 9 files changed, 25 insertions(+), 25 deletions(-)

--
2.25.1
  

Patch

diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
index eddd62b234..99155e352b 100644
--- a/nptl/nptl-printers.py
+++ b/nptl/nptl-printers.py
@@ -312,7 +312,7 @@  class ConditionVariablePrinter(object):
         """

         data = cond['__data']
-        self.wrefs = data['__wrefs']
+        self.crefs = data['__crefs']
         self.grefs = data['__g_refs']
         self.values = []

@@ -359,12 +359,12 @@  class ConditionVariablePrinter(object):
     def read_attributes(self):
         """Read the condvar's attributes."""

-        if (self.wrefs & PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0:
+        if (self.crefs & PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0:
             self.values.append(('Clock ID', 'CLOCK_MONOTONIC'))
         else:
             self.values.append(('Clock ID', 'CLOCK_REALTIME'))

-        if (self.wrefs & PTHREAD_COND_SHARED_MASK) != 0:
+        if (self.crefs & PTHREAD_COND_SHARED_MASK) != 0:
             self.values.append(('Shared', 'Yes'))
         else:
             self.values.append(('Shared', 'No'))
diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index 2ae16cbb8f..51a1249f08 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -45,7 +45,7 @@  ___pthread_cond_broadcast (pthread_cond_t *cond)
   unsigned int grefs1 = atomic_load_relaxed (cond->__data.__g_refs + 1);
   if ((grefs0 >> 1) == 0 && (grefs1 >> 1) == 0)
     return 0;
-  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__crefs);
   int private = __condvar_get_private (flags);

   __condvar_acquire_lock (cond, private);
diff --git a/nptl/pthread_cond_common.c b/nptl/pthread_cond_common.c
index 001dbd41e2..9af6a6fbec 100644
--- a/nptl/pthread_cond_common.c
+++ b/nptl/pthread_cond_common.c
@@ -20,7 +20,7 @@ 
 #include <stdint.h>
 #include <pthread.h>

-/* We need 3 least-significant bits on __wrefs for something else.  */
+/* We need 3 least-significant bits on __crefs for something else.  */
 #define __PTHREAD_COND_MAX_GROUP_SIZE ((unsigned) 1 << 29)

 #if __HAVE_64B_ATOMICS == 1
@@ -318,7 +318,7 @@  __condvar_set_orig_size (pthread_cond_t *cond, unsigned int size)
     atomic_store_relaxed (&cond->__data.__g1_orig_size, (size << 2) | 2);
 }

-/* Returns FUTEX_SHARED or FUTEX_PRIVATE based on the provided __wrefs
+/* Returns FUTEX_SHARED or FUTEX_PRIVATE based on the provided __crefs
    value.  */
 static int __attribute__ ((unused))
 __condvar_get_private (int flags)
diff --git a/nptl/pthread_cond_destroy.c b/nptl/pthread_cond_destroy.c
index e14ec89adf..ba971152e5 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -43,7 +43,7 @@  __pthread_cond_destroy (pthread_cond_t *cond)
 {
   LIBC_PROBE (cond_destroy, 1, cond);

-  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__crefs);
   int private = __condvar_get_private (flags);
   for (unsigned g = 0; g < 2; ++g)
     {
@@ -63,11 +63,11 @@  __pthread_cond_destroy (pthread_cond_t *cond)

   /* Same as above, except to synchronize with canceled threads.  This wake
      flag never gets cleared, so it's enough to set it once.  */
-  unsigned int wrefs = atomic_fetch_or_acquire (&cond->__data.__wrefs, 4) | 4;
-  while (wrefs >> 3 != 0)
+  unsigned int crefs = atomic_fetch_or_acquire (&cond->__data.__crefs, 4) | 4;
+  while (crefs >> 3 != 0)
     {
-      futex_wait_simple (&cond->__data.__wrefs, wrefs, private);
-      wrefs = atomic_load_acquire (&cond->__data.__wrefs);
+      futex_wait_simple (&cond->__data.__crefs, crefs, private);
+      crefs = atomic_load_acquire (&cond->__data.__crefs);
     }
   /* The memory the condvar occupies can now be reused.  */
   return 0;
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 6aa002ed1f..61d2e56881 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -37,13 +37,13 @@  __pthread_cond_init (pthread_cond_t *cond, const pthread_condattr_t *cond_attr)

   /* Iff not equal to ~0l, this is a PTHREAD_PROCESS_PRIVATE condvar.  */
   if (icond_attr != NULL && (icond_attr->value & 1) != 0)
-    cond->__data.__wrefs |= __PTHREAD_COND_SHARED_MASK;
+    cond->__data.__crefs |= __PTHREAD_COND_SHARED_MASK;
   int clockid = (icond_attr != NULL
 		 ? ((icond_attr->value >> 1) & ((1 << COND_CLOCK_BITS) - 1))
 		 : CLOCK_REALTIME);
   /* If 0, CLOCK_REALTIME is used; CLOCK_MONOTONIC otherwise.  */
   if (clockid != CLOCK_REALTIME)
-    cond->__data.__wrefs |= __PTHREAD_COND_CLOCK_MONOTONIC_MASK;
+    cond->__data.__crefs |= __PTHREAD_COND_CLOCK_MONOTONIC_MASK;

   LIBC_PROBE (cond_init, 2, cond, cond_attr);

diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 2043d8ac64..df018b1b61 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -49,7 +49,7 @@  ___pthread_cond_signal (pthread_cond_t *cond)
   unsigned int grefs1 = atomic_load_relaxed (cond->__data.__g_refs + 1);
   if ((grefs0 >> 1) == 0 && (grefs1 >> 1) == 0)
     return 0;
-  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__crefs);
   int private = __condvar_get_private (flags);

   __condvar_acquire_lock (cond, private);
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index dde007475c..b833e543a5 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -157,10 +157,10 @@  __condvar_cleanup_waiting (void *arg)
      determine when all waiters have woken. Since we will do more work in
      this function, we are using an extra channel to communicate to
      pthread_cond_destroy that it is not allowed to finish yet: We
-     increment the refcount starting at the fourth bit on __wrefs. Relaxed
+     increment the refcount starting at the fourth bit on __crefs. Relaxed
      MO is enough. The synchronization happens because __condvar_dec_grefs
      uses release MO. */
-  atomic_fetch_add_relaxed (&cond->__data.__wrefs, 8);
+  atomic_fetch_add_relaxed (&cond->__data.__crefs, 8);
   __condvar_dec_grefs (cond, g, cbuffer->private);

   __condvar_cancel_waiting (cond, cbuffer->wseq >> 1, g, cbuffer->private);
@@ -176,8 +176,8 @@  __condvar_cleanup_waiting (void *arg)
      are the last waiter (prior value of __wrefs was 1 << 3), then wake any
      threads waiting in pthread_cond_destroy.  Release MO to synchronize with
      these threads.  Don't bother clearing the wake-up request flag.  */
-  if ((atomic_fetch_add_release (&cond->__data.__wrefs, -8) >> 2) == 3)
-    futex_wake (&cond->__data.__wrefs, INT_MAX, cbuffer->private);
+  if ((atomic_fetch_add_release (&cond->__data.__crefs, -8) >> 2) == 3)
+    futex_wake (&cond->__data.__crefs, INT_MAX, cbuffer->private);

   /* XXX If locking the mutex fails, should we just stop execution?  This
      might be better than silently ignoring the error.  */
@@ -281,13 +281,13 @@  __condvar_cleanup_waiting (void *arg)
    __g1_orig_size: Initial size of G1
      * The two least-significant bits represent the condvar-internal lock.
      * Only accessed while having acquired the condvar-internal lock.
-   __wrefs: Flags and count of waiters who called pthread_cancel.
+   __crefs: Flags and count of waiters who called pthread_cancel.
      * Bit 2 is true if waiters should run futex_wake when they remove the
        last reference.  pthread_cond_destroy uses this as futex word.
      * Bit 1 is the clock ID (0 == CLOCK_REALTIME, 1 == CLOCK_MONOTONIC).
      * Bit 0 is true iff this is a process-shared condvar.
      * Simple reference count used by __condvar_cleanup_waiting and pthread_cond_destroy.
-     (If the format of __wrefs is changed, update the pretty printers.)
+     (If the format of __crefs is changed, update the pretty printers.)
    For each of the two groups, we have:
    __g_refs: Futex waiter reference count.
      * LSB is true if waiters should run futex_wake when they remove the
@@ -394,7 +394,7 @@  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
      make us see the closed flag on __g_signals that designates a concurrent
      attempt to reuse the group's slot. */
   atomic_fetch_add_acquire (cond->__data.__g_refs + g, 2);
-  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__crefs);
   int private = __condvar_get_private (flags);

   /* Now that we are registered as a waiter, we can release the mutex.
@@ -549,7 +549,7 @@  ___pthread_cond_timedwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex,

   /* Relaxed MO is suffice because clock ID bit is only modified
      in condition creation.  */
-  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__crefs);
   clockid_t clockid = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
                     ? CLOCK_MONOTONIC : CLOCK_REALTIME;
   return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
diff --git a/nptl/tst-cond22.c b/nptl/tst-cond22.c
index 64f19ea0a5..d4fcaacc98 100644
--- a/nptl/tst-cond22.c
+++ b/nptl/tst-cond22.c
@@ -110,7 +110,7 @@  do_test (void)
 	  c.__data.__wseq, c.__data.__g1_start,
 	  c.__data.__g_signals[0], c.__data.__g_refs[0], c.__data.__g_size[0],
 	  c.__data.__g_signals[1], c.__data.__g_refs[1], c.__data.__g_size[1],
-	  c.__data.__g1_orig_size, c.__data.__wrefs);
+	  c.__data.__g1_orig_size, c.__data.__crefs);

   if (pthread_create (&th, NULL, tf, (void *) 1l) != 0)
     {
@@ -153,7 +153,7 @@  do_test (void)
 	  c.__data.__wseq, c.__data.__g1_start,
 	  c.__data.__g_signals[0], c.__data.__g_refs[0], c.__data.__g_size[0],
 	  c.__data.__g_signals[1], c.__data.__g_refs[1], c.__data.__g_size[1],
-	  c.__data.__g1_orig_size, c.__data.__wrefs);
+	  c.__data.__g1_orig_size, c.__data.__crefs);

   return status;
 }
diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h
index 44bf1e358d..494a43a675 100644
--- a/sysdeps/nptl/bits/thread-shared-types.h
+++ b/sysdeps/nptl/bits/thread-shared-types.h
@@ -112,7 +112,7 @@  struct __pthread_cond_s
   unsigned int __g_refs[2] __LOCK_ALIGNMENT;
   unsigned int __g_size[2];
   unsigned int __g1_orig_size;
-  unsigned int __wrefs;
+  unsigned int __crefs;
   unsigned int __g_signals[2];
 };