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 7/9] nptl: Fix indentation
Date: Fri, 27 Jan 2023 21:18:27 -0500	[thread overview]
Message-ID: <20230128021829.7990-8-malteskarupke@fastmail.fm> (raw)
In-Reply-To: <20230128021829.7990-1-malteskarupke@fastmail.fm>

From: Malte Skarupke <malteskarupke@fastmail.fm>

In my previous change I turned a nested loop into a simple loop. I'm doing
the resulting indentation changes in a separate commit to make the diff on
the previous commit easier to review.
---
 nptl/pthread_cond_wait.c | 110 +++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 8b7db51148..bf05ac6b22 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -383,65 +383,65 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
     }
 
 
-      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;
-
-	  if (seq < (g1_start >> 1))
-	    {
-	      /* If the group is closed already,
-	         then this waiter originally had enough extra signals to
-	         consume, up until the time its group was closed.  */
-	       break;
-	    }
-
-	  /* If there is an available signal, don't block.
-	     If __g1_start has advanced at all, then we must be in G1
-	     by now, perhaps in the process of switching back to an older
-	     G2, but in either case we're allowed to consume the available
-	     signal and should not block anymore.  */
-	  if ((int)(signals - lowseq) >= 2)
-	    {
-	      /* 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,
+  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;
+
+      if (seq < (g1_start >> 1))
+        {
+          /* If the group is closed already,
+             then this waiter originally had enough extra signals to
+             consume, up until the time its group was closed.  */
+           break;
+        }
+
+      /* If there is an available signal, don't block.
+         If __g1_start has advanced at all, then we must be in G1
+         by now, perhaps in the process of switching back to an older
+         G2, but in either case we're allowed to consume the available
+         signal and should not block anymore.  */
+      if ((int)(signals - lowseq) >= 2)
+        {
+	  /* 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;
-	  struct _condvar_cleanup_buffer cbuffer;
-	  cbuffer.wseq = wseq;
-	  cbuffer.cond = cond;
-	  cbuffer.mutex = mutex;
-	  cbuffer.private = private;
-	  __pthread_cleanup_push (&buffer, __condvar_cleanup_waiting, &cbuffer);
-
-	  err = __futex_abstimed_wait_cancelable64 (
-	    cond->__data.__g_signals + g, signals, clockid, abstime, private);
-
-	  __pthread_cleanup_pop (&buffer, 0);
-
-	  if (__glibc_unlikely (err == ETIMEDOUT || err == EOVERFLOW))
-	    {
-	      /* If we timed out, we effectively cancel waiting.  */
-	      __condvar_cancel_waiting (cond, seq, g, private);
-	      result = err;
 	      break;
-	    }
+	    else
+	      continue;
 	}
 
+      // Now block.
+      struct _pthread_cleanup_buffer buffer;
+      struct _condvar_cleanup_buffer cbuffer;
+      cbuffer.wseq = wseq;
+      cbuffer.cond = cond;
+      cbuffer.mutex = mutex;
+      cbuffer.private = private;
+      __pthread_cleanup_push (&buffer, __condvar_cleanup_waiting, &cbuffer);
+
+      err = __futex_abstimed_wait_cancelable64 (
+        cond->__data.__g_signals + g, signals, clockid, abstime, private);
+
+      __pthread_cleanup_pop (&buffer, 0);
+
+      if (__glibc_unlikely (err == ETIMEDOUT || err == EOVERFLOW))
+        {
+          /* If we timed out, we effectively cancel waiting.  */
+          __condvar_cancel_waiting (cond, seq, g, private);
+          result = err;
+          break;
+        }
+    }
+
   /* 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
      mutex.  */
-- 
2.25.1


  parent reply	other threads:[~2023-01-28  2:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-28  2:18 [PATCH 0/9] Patch to fix glibc condition variable bug (BZ 25847) malteskarupke
2023-01-28  2:18 ` [PATCH 1/9] pthreads NPTL: lost wakeup fix 2 malteskarupke
2023-01-28  2:18 ` [PATCH 2/9] nptl: Update comments and indentation for new condvar implementation malteskarupke
2023-01-28  2:18 ` [PATCH 3/9] nptl: Remove unnecessary catch-all-wake in condvar group switch malteskarupke
2023-01-28  2:18 ` [PATCH 4/9] nptl: Remove unnecessary quadruple check in pthread_cond_wait malteskarupke
2023-01-28  2:18 ` [PATCH 5/9] nptl: Remove g_refs from condition variables malteskarupke
2023-01-28  2:18 ` [PATCH 6/9] nptl: Use a single loop in pthread_cond_wait instaed of a nested loop malteskarupke
2023-01-28  2:18 ` malteskarupke [this message]
2023-01-28  2:18 ` [PATCH 8/9] nptl: rename __condvar_quiesce_and_switch_g1 malteskarupke
2023-01-28  2:18 ` [PATCH 9/9] nptl: Use all of g1_start and g_signals malteskarupke
2023-02-08  2:48 ` [PATCH 0/9] Patch to fix glibc condition variable bug (BZ 25847) Malte Skarupke
2023-02-08  2:51   ` Sam James
2023-03-18 12:08     ` Malte Skarupke

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=20230128021829.7990-8-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).