From: malteskarupke@fastmail.fm
To: libc-alpha@sourceware.org
Cc: Malte Skarupke <malteskarupke@fastmail.fm>
Subject: [PATCH v3 5/6] nptl: Rename __wrefs to __crefs because its meaning has changed
Date: Thu, 6 Oct 2022 17:43:28 -0400 [thread overview]
Message-ID: <20221006214329.1084244-6-malteskarupke@fastmail.fm> (raw)
In-Reply-To: <20221006214329.1084244-1-malteskarupke@fastmail.fm>
From: Malte Skarupke <malteskarupke@fastmail.fm>
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(-)
diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
index 3fb0335135..981eeedd76 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 e45f6271bf..18a3c3553c 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -44,7 +44,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 ce09d5d15a..220db22e9f 100644
--- a/nptl/pthread_cond_common.c
+++ b/nptl/pthread_cond_common.c
@@ -21,7 +21,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.
This also matches __atomic_wide_counter requirements: The highest
value we add is __PTHREAD_COND_MAX_GROUP_SIZE << 2 to __g1_start
(the two extra bits are for the lock in the two LSBs of
@@ -178,7 +178,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 053d0a2fbc..844647b3b7 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -42,7 +42,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)
{
@@ -62,11 +62,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 739b3afb2d..d2e1988cf6 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -36,13 +36,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 2e8be2d3b5..d3735103fc 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -48,7 +48,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 b949805dae..515b4ba60e 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -156,10 +156,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);
@@ -175,8 +175,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. */
@@ -280,13 +280,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
@@ -393,7 +393,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.
@@ -548,7 +548,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 1336e9c79d..9f8cfea5c3 100644
--- a/nptl/tst-cond22.c
+++ b/nptl/tst-cond22.c
@@ -113,7 +113,7 @@ do_test (void)
c.__data.__g1_start.__value32.__low,
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)
{
@@ -159,7 +159,7 @@ do_test (void)
c.__data.__g1_start.__value32.__low,
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 5653507e55..52decc49d6 100644
--- a/sysdeps/nptl/bits/thread-shared-types.h
+++ b/sysdeps/nptl/bits/thread-shared-types.h
@@ -98,7 +98,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];
};
--
2.25.1
next prev parent reply other threads:[~2022-10-06 21:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 21:43 [PATCH v3 0/6] nptl: Fix pthread_cond_signal missing a sleeper malteskarupke
2022-10-06 21:43 ` [PATCH v3 1/6] nptl: Fix pthread_cond_signal missing a sleeper (#BZ 25847) malteskarupke
2022-10-06 21:43 ` [PATCH v3 2/6] nptl: Remove the signal-stealing code. It is no longer needed malteskarupke
2022-10-06 21:43 ` [PATCH v3 3/6] nptl: Optimization by not incrementing wrefs in pthread_cond_wait malteskarupke
2022-10-06 21:43 ` [PATCH v3 4/6] nptl: Make test-cond-printers check the number of waiters malteskarupke
2022-10-06 21:43 ` malteskarupke [this message]
2022-10-06 21:43 ` [PATCH v3 6/6] nptl: Cleaning up __g1_start and related code in pthread_cond_wait malteskarupke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221006214329.1084244-6-malteskarupke@fastmail.fm \
--to=malteskarupke@fastmail.fm \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).