From: Malte Skarupke <malteskarupke@web.de>
To: libc-alpha@sourceware.org
Cc: malteskarupke@fastmail.fm, triegel@redhat.com,
Malte Skarupke <malteskarupke@web.de>
Subject: [PATCH 5/5] nptl: Rename __wrefs to __flags because its meaning has changed
Date: Sat, 16 Jan 2021 15:49:50 -0500 [thread overview]
Message-ID: <20210116204950.16434-5-malteskarupke@web.de> (raw)
In-Reply-To: <20210116204950.16434-1-malteskarupke@web.de>
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. So the name "wrefs" is no longer accurate. It is still used
as a reference count in an edge case, in the interaction between
pthread_cancel and pthread_cond_destroy, but that edge case shouldn't
be what this variable is named after.
The name __flags seems more appropriate since in the most common case
this variable is just used to store the flags of the condition
variable.
---
nptl/nptl-printers.py | 6 +++---
nptl/pthread_cond_broadcast.c | 2 +-
nptl/pthread_cond_common.c | 4 ++--
nptl/pthread_cond_destroy.c | 8 ++++----
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, 24 insertions(+), 24 deletions(-)
diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
index 1a404befe5..0e37d8b4dd 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.flags = data['__flags']
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.flags & 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.flags & 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 e10432ce7c..cbb249b35b 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.__flags);
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 3251c7f0ec..22270e8805 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 __flags 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 __flags
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 1c27385f89..3dfe4a48db 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.__flags);
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;
+ unsigned int wrefs = atomic_fetch_or_acquire (&cond->__data.__flags, 4) | 4;
while (wrefs >> 3 != 0)
{
- futex_wait_simple (&cond->__data.__wrefs, wrefs, private);
- wrefs = atomic_load_acquire (&cond->__data.__wrefs);
+ futex_wait_simple (&cond->__data.__flags, wrefs, private);
+ wrefs = atomic_load_acquire (&cond->__data.__flags);
}
/* 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 595b1b3528..3031d52a42 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.__flags |= __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.__flags |= __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 0cd534cc40..979c9d72d5 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -43,7 +43,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.__flags);
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 0ee0247874..0993728e5d 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -164,9 +164,9 @@ __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 fourth bit on __wrefs. Relaxed MO is enough. The synchronization
+ the fourth bit on __flags. 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.__flags, 8);
__condvar_dec_grefs (cond, g, cbuffer->private);
__condvar_cancel_waiting (cond, cbuffer->wseq >> 1, g, cbuffer->private);
@@ -182,8 +182,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.__flags, -8) >> 2) == 3)
+ futex_wake (&cond->__data.__flags, INT_MAX, cbuffer->private);
/* XXX If locking the mutex fails, should we just stop execution? This
might be better than silently ignoring the error. */
@@ -287,13 +287,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.
+ __flags: 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 __flags 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
@@ -410,7 +410,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
synchronize with the dummy read-modify-write in
__condvar_quiesce_and_switch_g1 if we read from that. */
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.__flags);
int private = __condvar_get_private (flags);
/* Now that we are registered as a waiter, we can release the mutex.
@@ -558,7 +558,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.__flags);
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..e1338ebf94 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.__flags);
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.__flags);
return status;
}
diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h
index fbbdd0bb36..84cedfcaa0 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 __flags;
unsigned int __g_signals[2];
};
--
2.17.1
next prev parent reply other threads:[~2021-01-16 20:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-16 20:49 [PATCH 1/5] nptl: Fix pthread_cond_signal missing a sleeper (#BZ 25847) Malte Skarupke
2021-01-16 20:49 ` [PATCH 2/5] nptl: Remove the signal-stealing code. It is no longer needed Malte Skarupke
2021-01-16 20:49 ` [PATCH 3/5] nptl: Optimization by not incrementing wrefs in pthread_cond_wait Malte Skarupke
2021-01-18 23:41 ` Torvald Riegel
2021-01-16 20:49 ` [PATCH 4/5] nptl: Make test-cond-printers check the number of waiters Malte Skarupke
2021-01-16 20:49 ` Malte Skarupke [this message]
2021-01-18 23:47 ` [PATCH 5/5] nptl: Rename __wrefs to __flags because its meaning has changed Torvald Riegel
2021-01-18 22:43 ` [PATCH 1/5] nptl: Fix pthread_cond_signal missing a sleeper (#BZ 25847) Torvald Riegel
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=20210116204950.16434-5-malteskarupke@web.de \
--to=malteskarupke@web.de \
--cc=libc-alpha@sourceware.org \
--cc=malteskarupke@fastmail.fm \
--cc=triegel@redhat.com \
/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).