From patchwork Sat Jan 16 20:49:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Malte Skarupke X-Patchwork-Id: 41735 X-Patchwork-Delegate: carlos@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B9286398B844; Sat, 16 Jan 2021 20:50:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B9286398B844 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610830213; bh=1RuzH/CkgNJH7yaXNwexIc8pDSKKmOa0WqZfEbfVRYs=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=U+7lRfBbHWYjjHMYxxqVAGepNKcMiCaoMYBmpO9DYqrkrlB4dJnLPVBs1GY0nTpho TBdGWYIqaJJVcNe9z75wZKcBCPDUN/ElLC88xlKcZrbHKl3cfcJ83So9axz36bKDDi 66p0LERubNsBqGHPlwLTvsx3VIZHeLR4Su+jyOoc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mout.web.de (mout.web.de [212.227.17.12]) by sourceware.org (Postfix) with ESMTPS id 8E9A1384A030 for ; Sat, 16 Jan 2021 20:50:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8E9A1384A030 X-UI-Sender-Class: c548c8c5-30a9-4db5-a2e7-cb6cb037b8f9 Received: from auth1-smtp.messagingengine.com ([66.111.4.227]) by smtp.web.de (mrweb102 [213.165.67.124]) with ESMTPSA (Nemesis) id 0MBTEQ-1lAGhr3Rmo-00AUt2; Sat, 16 Jan 2021 21:50:02 +0100 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailauth.nyi.internal (Postfix) with ESMTP id DE90127C0060; Sat, 16 Jan 2021 15:49:57 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Sat, 16 Jan 2021 15:49:57 -0500 X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrtdeggddugeehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefhvffufffkofgjfhestddtredtre dttdenucfhrhhomhepofgrlhhtvgcuufhkrghruhhpkhgvuceomhgrlhhtvghskhgrrhhu phhkvgesfigvsgdruggvqeenucggtffrrghtthgvrhhnpeduffeufedtudehhfffieejfe ehfeefledvffffffehhfefffelffeiudelteehfeenucfkphepieejrddvheegrdduieeh rdelnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepmh grlhhtvghskhgrrhhuphhkvgdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidq uddtudejtdefvdeliedqudduvdegvdekieelqdhmrghlthgvshhkrghruhhpkhgvpeepfi gvsgdruggvsehfrghsthhmrghilhdrfhhm X-ME-Proxy: Received: from localhost.localdomain (unknown [67.254.165.9]) by mail.messagingengine.com (Postfix) with ESMTPA id 9935D1080064; Sat, 16 Jan 2021 15:49:57 -0500 (EST) To: libc-alpha@sourceware.org Subject: [PATCH 5/5] nptl: Rename __wrefs to __flags because its meaning has changed Date: Sat, 16 Jan 2021 15:49:50 -0500 Message-Id: <20210116204950.16434-5-malteskarupke@web.de> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210116204950.16434-1-malteskarupke@web.de> References: <20210116204950.16434-1-malteskarupke@web.de> X-Provags-ID: V03:K1:G8PJTgJpnRJZjCz9KKmuWMR46roNPCgtLnWhPe2Trc8HBKBvPwS na8+IemLtQsytS/oO2gBou0V6csQhUAb7os7Gml31tWJWT59+92cdU/G+LIz/6pvTLXGONv RzmKR4fWIbrTseBVCKeYkO7vhqvRLwipjbBoUK5yHytGa1dUqj/Lf2BfqPjvagF7+DcYlil be9iz3wUQWgqwA+/DPxxg== X-UI-Out-Filterresults: notjunk:1;V03:K0:RyGi9nK+DRg=:Wm2tGY2V7XDL9JwJSMpPzS KQE5+fEQ82or2djEIz8CD16MRh+AHWmW07uQkqpuUGmDnOkM+7f7iPRlSNJrWx33W5MyqreP8 3Qp5+96KyooSPfSrDZ/Gj26fBhEl8lxZ1dFNzwRYLsN4Fm1pLpmyaUt4iIktnNFUxUbRaSp8v NG12W6BrFwROGT3vAYQTrzXZaoGbyTQmi1PCvHAmYFozkeNU0+07/SfQOgAbZvlFPPQoha4Av IIpE2lSjT2wpk+PTB/n5qDVvZ6jDUB3Isiwd8gfq8eV+jJ8po33VZREdY7lzyOnmhyjZElEw8 XGyzYfg63QBehrSZn8uMbxhBmwm23/NGKJfDbQ5NgS1yOW7iRiYwEW/PFDDqlMbrUWGic5h5h k8d7v4Fubr/E754c4wY6m/Ctsh9D34/nYUUqFk0Use2aAhS9DAsSvDYFCf6hc7r++/cxVPUXb QOBb2WS2AABsCJu8bZBgIFOCBD4RJKhx4a50oe7C5ptaCEKAgN3mFwrEUHBpxjJCOHoHuI6Zp Jgn8dr+a5+L3d8ktvt8bP79l97j238wgxdTC1VMMPZQG4l30bhMJzkWN/y/OJDqgxs3KsttxP B+BAcDlvvXgmR0qUoi2UgTgrnrgkvFtAUIDDUPQc/ZoYiEQj4KgHGD/0Nf/wRlqHAOkwcaIWZ o5V4qscQATwMQxBBjYx4FgoshelDFrcs0eILRiFl7Ch89/lNkW+pUVcTzjqtuOzCODZDetHHO xyCrmuLDqj0wlY3bt6kiTyK8r1nQOfBKMX73o37NuTDHaiDEp5GdXchc08AtdgR2kcksLUzQ3 XWcZA/OPQgC1xPg2l3b003N8o+LqLwz+E5oG47c8EV7Dtj1rIOFHkKUfsEiX7AeIc2YLuD5CB l3cvUiOPpe+HlQEJvgKQ== X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Malte Skarupke via Libc-alpha From: Malte Skarupke Reply-To: Malte Skarupke Cc: Malte Skarupke , triegel@redhat.com, malteskarupke@fastmail.fm Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" 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(-) -- 2.17.1 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 #include -/* 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]; };