public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: malteskarupke@fastmail.fm
To: libc-alpha@sourceware.org
Cc: Malte Skarupke <malteskarupke@fastmail.fm>
Subject: [PATCH v2 6/9] nptl: Use a single loop in pthread_cond_wait instaed of a nested loop
Date: Thu,  4 May 2023 00:55:00 -0400	[thread overview]
Message-ID: <20230504045503.83276-7-malteskarupke@fastmail.fm> (raw)
In-Reply-To: <20230504045503.83276-1-malteskarupke@fastmail.fm>

From: Malte Skarupke <malteskarupke@fastmail.fm>

The loop was a little more complicated than necessary. There was only one
break statement out of the inner loop, and the outer loop was nearly empty.
So just remove the outer loop, moving its code to the one break statement in
the inner loop. This allows us to replace all gotos with break statements.
---
 nptl/pthread_cond_wait.c | 41 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 8a9219e064..8b7db51148 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -382,17 +382,15 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
       return err;
     }
 
-  /* Now wait until a signal is available in our group or it is closed.
-     Acquire MO so that if we observe (signals == lowseq) after group
-     switching in __condvar_quiesce_and_switch_g1, we synchronize with that
-     store and will see the prior update of __g1_start done while switching
-     groups too.  */
-  unsigned int signals = atomic_load_acquire (cond->__data.__g_signals + g);
-
-  do
-    {
+
       while (1)
 	{
+	  /* Now wait until a signal is available in our group or it is closed.
+	     Acquire MO so that if we observe (signals == lowseq) after group
+	     switching in __condvar_quiesce_and_switch_g1, we synchronize with that
+	     store and will see the prior update of __g1_start done while switching
+	     groups too.  */
+	  unsigned int signals = atomic_load_acquire (cond->__data.__g_signals + g);
 	  uint64_t g1_start = __condvar_load_g1_start_relaxed (cond);
 	  unsigned int lowseq = (g1_start & 1) == g ? signals : g1_start & ~1U;
 
@@ -401,7 +399,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	      /* If the group is closed already,
 	         then this waiter originally had enough extra signals to
 	         consume, up until the time its group was closed.  */
-	       goto done;
+	       break;
 	    }
 
 	  /* If there is an available signal, don't block.
@@ -410,7 +408,16 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	     G2, but in either case we're allowed to consume the available
 	     signal and should not block anymore.  */
 	  if ((int)(signals - lowseq) >= 2)
-	    break;
+	    {
+	      /* Try to grab a signal.  See above for MO.  (if we do another loop
+		 iteration we need to see the correct value of g1_start)  */
+		      if (atomic_compare_exchange_weak_acquire (
+		      		cond->__data.__g_signals + g,
+			&signals, signals - 2))
+		      	break;
+		      else
+		      	continue;
+	    }
 
 	  // Now block.
 	  struct _pthread_cleanup_buffer buffer;
@@ -431,19 +438,9 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	      /* If we timed out, we effectively cancel waiting.  */
 	      __condvar_cancel_waiting (cond, seq, g, private);
 	      result = err;
-	      goto done;
+	      break;
 	    }
-
-	  /* Reload signals.  See above for MO.  */
-	  signals = atomic_load_acquire (cond->__data.__g_signals + g);
 	}
-    }
-  /* Try to grab a signal.  See above for MO.  (if we do another loop
-     iteration we need to see the correct value of g1_start)  */
-  while (!atomic_compare_exchange_weak_acquire (cond->__data.__g_signals + g,
-						&signals, signals - 2));
-
- done:
 
   /* Confirm that we have been woken.  We do that before acquiring the mutex
      to allow for execution of pthread_cond_destroy while having acquired the
-- 
2.34.1


  parent reply	other threads:[~2023-05-04  4:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-04  4:54 [PATCH v2 0/9] Patch to fix glibc condition variable bug (BZ 25847) malteskarupke
2023-05-04  4:54 ` [PATCH v2 1/9] pthreads NPTL: lost wakeup fix 2 malteskarupke
2023-05-04  4:54 ` [PATCH v2 2/9] nptl: Update comments and indentation for new condvar implementation malteskarupke
2023-05-04  4:54 ` [PATCH v2 3/9] nptl: Remove unnecessary catch-all-wake in condvar group switch malteskarupke
2023-05-04  4:54 ` [PATCH v2 4/9] nptl: Remove unnecessary quadruple check in pthread_cond_wait malteskarupke
2023-05-04  4:54 ` [PATCH v2 5/9] nptl: Remove g_refs from condition variables malteskarupke
2023-05-04  4:55 ` malteskarupke [this message]
2023-05-04  4:55 ` [PATCH v2 7/9] nptl: Fix indentation malteskarupke
2023-05-04  4:55 ` [PATCH v2 8/9] nptl: rename __condvar_quiesce_and_switch_g1 malteskarupke
2023-05-04  4:55 ` [PATCH v2 9/9] nptl: Use all of g1_start and g_signals 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=20230504045503.83276-7-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).