From: malteskarupke@fastmail.fm
To: libc-alpha@sourceware.org
Cc: Malte Skarupke <malteskarupke@fastmail.fm>
Subject: [PATCH v3 2/6] nptl: Remove the signal-stealing code. It is no longer needed.
Date: Thu, 6 Oct 2022 17:43:25 -0400 [thread overview]
Message-ID: <20221006214329.1084244-3-malteskarupke@fastmail.fm> (raw)
In-Reply-To: <20221006214329.1084244-1-malteskarupke@fastmail.fm>
From: Malte Skarupke <malteskarupke@fastmail.fm>
After my last change, stealing of signals can no longer happen. This
patch removes the code that handled the case when a signal was stolen.
---
nptl/pthread_cond_wait.c | 63 ----------------------------------------
1 file changed, 63 deletions(-)
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 7b9116c930..0502b5ad3f 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -527,69 +527,6 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
while (!atomic_compare_exchange_weak_acquire (cond->__data.__g_signals + g,
&signals, signals - 2));
- /* We consumed a signal but we could have consumed from a more recent group
- that aliased with ours due to being in the same group slot. If this
- might be the case our group must be closed as visible through
- __g1_start. */
- uint64_t g1_start = __condvar_load_g1_start_relaxed (cond);
- if (seq < (g1_start >> 1))
- {
- /* We potentially stole a signal from a more recent group but we do not
- know which group we really consumed from.
- We do not care about groups older than current G1 because they are
- closed; we could have stolen from these, but then we just add a
- spurious wake-up for the current groups.
- We will never steal a signal from current G2 that was really intended
- for G2 because G2 never receives signals (until it becomes G1). We
- could have stolen a signal from G2 that was conservatively added by a
- previous waiter that also thought it stole a signal -- but given that
- that signal was added unnecessarily, it's not a problem if we steal
- it.
- Thus, the remaining case is that we could have stolen from the current
- G1, where "current" means the __g1_start value we observed. However,
- if the current G1 does not have the same slot index as we do, we did
- not steal from it and do not need to undo that. This is the reason
- for putting a bit with G2's index into__g1_start as well. */
- if (((g1_start & 1) ^ 1) == g)
- {
- /* We have to conservatively undo our potential mistake of stealing
- a signal. We can stop trying to do that when the current G1
- changes because other spinning waiters will notice this too and
- __condvar_quiesce_and_switch_g1 has checked that there are no
- futex waiters anymore before switching G1.
- Relaxed MO is fine for the __g1_start load because we need to
- merely be able to observe this fact and not have to observe
- something else as well.
- ??? Would it help to spin for a little while to see whether the
- current G1 gets closed? This might be worthwhile if the group is
- small or close to being closed. */
- unsigned int s = atomic_load_relaxed (cond->__data.__g_signals + g);
- while (__condvar_load_g1_start_relaxed (cond) == g1_start)
- {
- /* Try to add a signal. We don't need to acquire the lock
- because at worst we can cause a spurious wake-up. If the
- group is in the process of being closed (LSB is true), this
- has an effect similar to us adding a signal. */
- if (((s & 1) != 0)
- || atomic_compare_exchange_weak_relaxed
- (cond->__data.__g_signals + g, &s, s + 2))
- {
- /* If we added a signal, we also need to add a wake-up on
- the futex. We also need to do that if we skipped adding
- a signal because the group is being closed because
- while __condvar_quiesce_and_switch_g1 could have closed
- the group, it might stil be waiting for futex waiters to
- leave (and one of those waiters might be the one we stole
- the signal from, which cause it to block using the
- futex). */
- futex_wake (cond->__data.__g_signals + g, 1, private);
- break;
- }
- /* TODO Back off. */
- }
- }
- }
-
done:
/* Decrement group reference count and confirm that we have been woken. We do
--
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 ` malteskarupke [this message]
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 ` [PATCH v3 5/6] nptl: Rename __wrefs to __crefs because its meaning has changed malteskarupke
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-3-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).