public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Subject: [PATCH 04/15] nptl: Move setxid flag out of cancelhandling
Date: Thu, 30 Sep 2021 17:00:40 -0300	[thread overview]
Message-ID: <20210930200051.1017457-5-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20210930200051.1017457-1-adhemerval.zanella@linaro.org>

Now that the thread state is tracked by 'joinstate' field, there is no
need to keep the setxid flag within the 'cancelhandling'.  It simplifies
the atomic code to set and reset it (since there is no need to handle
the thread state concurrent update).

Checked on x86_64-linux-gnu and aarch64-linux-gnu.
---
 nptl/allocatestack.c  |  3 +++
 nptl/descr.h          |  6 ++---
 nptl/nptl_setxid.c    | 55 ++++++++++++++-----------------------------
 nptl/pthread_create.c |  4 ++--
 4 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index fa8100719f..74a8bbc19c 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -99,6 +99,7 @@ get_cached_stack (size_t *sizep, void **memp)
     }
 
   /* Don't allow setxid until cloned.  */
+  result->setxid_flag = 0;
   result->setxid_futex = -1;
 
   /* Dequeue the entry.  */
@@ -303,6 +304,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 #endif
 
       /* Don't allow setxid until cloned.  */
+      pd->setxid_flag = 0;
       pd->setxid_futex = -1;
 
       /* Allocate the DTV for this thread.  */
@@ -424,6 +426,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 #endif
 
 	  /* Don't allow setxid until cloned.  */
+	  pd->setxid_flag = 0;
 	  pd->setxid_futex = -1;
 
 	  /* Allocate the DTV for this thread.  */
diff --git a/nptl/descr.h b/nptl/descr.h
index 76522b85b3..db011af500 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -296,9 +296,6 @@ struct pthread
   /* Bit set if thread terminated and TCB is freed.  */
 #define TERMINATED_BIT		5
 #define TERMINATED_BITMASK	(0x01 << TERMINATED_BIT)
-  /* Bit set if thread is supposed to change XID.  */
-#define SETXID_BIT		6
-#define SETXID_BITMASK		(0x01 << SETXID_BIT)
 
   /* Flags.  Including those copied from the thread attribute.  */
   int flags;
@@ -338,6 +335,9 @@ struct pthread
   /* Lock to synchronize access to the descriptor.  */
   int lock;
 
+  /* Indicate whether thread is supposed to change XID, it acts a boolean
+     variable (0 means no operation pending).  */
+  int setxid_flag;
   /* Lock for synchronizing setxid calls.  */
   unsigned int setxid_futex;
 
diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c
index 1bd7b5f803..244536260d 100644
--- a/nptl/nptl_setxid.c
+++ b/nptl/nptl_setxid.c
@@ -75,14 +75,7 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, void *ctx)
 
   /* Reset the SETXID flag.  */
   struct pthread *self = THREAD_SELF;
-  int flags, newval;
-  do
-    {
-      flags = THREAD_GETMEM (self, cancelhandling);
-      newval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
-					  flags & ~SETXID_BITMASK, flags);
-    }
-  while (flags != newval);
+  atomic_store_release (&self->setxid_flag, 0);
 
   /* And release the futex.  */
   self->setxid_futex = 1;
@@ -96,8 +89,6 @@ libc_hidden_def (__nptl_setxid_sighandler)
 static void
 setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
 {
-  int ch;
-
   /* Wait until this thread is cloned.  */
   if (t->setxid_futex == -1
       && ! atomic_compare_and_exchange_bool_acq (&t->setxid_futex, -2, -1))
@@ -108,41 +99,31 @@ setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
   /* Don't let the thread exit before the setxid handler runs.  */
   t->setxid_futex = 0;
 
-  do
+  /* If thread is exiting right now, ignore it.  */
+  if (atomic_load_acquire (&t->joinstate) == THREAD_STATE_EXITING)
     {
-      ch = t->cancelhandling;
-
-      /* If the thread is exiting right now, ignore it.  */
-      if ((ch & EXITING_BITMASK) != 0)
-        {
-          /* Release the futex if there is no other setxid in
-             progress.  */
-          if ((ch & SETXID_BITMASK) == 0)
-            {
-              t->setxid_futex = 1;
-              futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE);
-            }
-          return;
+      /* Release the futex if there is no other setxid in progress.  */
+      if (atomic_load_relaxed (&t->setxid_flag) == 0)
+	{
+	  t->setxid_futex = 1;
+	  futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE);
         }
+      return;
+    }
+
+  /* Release the futex if there is no other setxid in progress.  */
+  if (atomic_exchange_acquire (&t->setxid_flag, 1) == 0)
+    {
+      t->setxid_futex = 1;
+      futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE);
     }
-  while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
-                                               ch | SETXID_BITMASK, ch));
 }
 
 
 static void
 setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t)
 {
-  int ch;
-
-  do
-    {
-      ch = t->cancelhandling;
-      if ((ch & SETXID_BITMASK) == 0)
-        return;
-    }
-  while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
-                                               ch & ~SETXID_BITMASK, ch));
+  atomic_store_release (&t->setxid_flag, 0);
 
   /* Release the futex just in case.  */
   t->setxid_futex = 1;
@@ -153,7 +134,7 @@ setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t)
 static int
 setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
 {
-  if ((t->cancelhandling & SETXID_BITMASK) == 0)
+  if (atomic_load_relaxed (&t->setxid_flag) == 0)
     return 0;
 
   int val;
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index c7d53edbb8..3659f51c46 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -562,7 +562,7 @@ start_thread (void *arg)
     advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
 			pd->guardsize);
 
-  if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK))
+  if (__glibc_unlikely (atomic_load_relaxed (&pd->setxid_flag) == 1))
     {
       /* Some other thread might call any of the setXid functions and expect
 	 us to reply.  In this case wait until we did that.  */
@@ -572,7 +572,7 @@ start_thread (void *arg)
 	   condition used in the surrounding loop (cancelhandling).  We need
 	   to check and document why this is correct.  */
 	futex_wait_simple (&pd->setxid_futex, 0, FUTEX_PRIVATE);
-      while (pd->cancelhandling & SETXID_BITMASK);
+      while (atomic_load_relaxed (&pd->setxid_flag) == 1);
 
       /* Reset the value so that the stack can be reused.  */
       pd->setxid_futex = 0;
-- 
2.30.2


  parent reply	other threads:[~2021-09-30 20:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 20:00 [PATCH 00/15] Fix various NPTL synchronization issues Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 01/15] nptl: Set cancellation type and state on pthread_exit (BZ #28267) Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 02/15] nptl: Handle robust PI mutexes for !__ASSUME_SET_ROBUST_LIST (BZ #28268) Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 03/15] nptl: Do not use pthread set_tid_address as state synchronization (BZ #19951) Adhemerval Zanella
2021-09-30 20:00 ` Adhemerval Zanella [this message]
2021-09-30 20:00 ` [PATCH 05/15] nptl: Replace struct thread cancelhandling field Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 06/15] nptl: Use exit_lock when accessing TID on pthread_getaffinity_np Adhemerval Zanella
2021-09-30 20:54   ` Florian Weimer
2021-10-04 12:45     ` Cyril Hrubis
2021-10-04 19:27       ` Florian Weimer
2021-10-04 19:53         ` Adhemerval Zanella
2021-10-05  7:35           ` Cyril Hrubis
2021-10-05 13:47             ` Adhemerval Zanella
2021-10-10 14:37           ` Florian Weimer
2021-09-30 20:00 ` [PATCH 07/15] nptl: Use exit_lock when accessing TID on pthread_setaffinity Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 08/15] nptl: Use exit_lock when accessing TID on pthread_getcpuclockid Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 09/15] nptl: Use exit_lock when accessing TID on pthread_setschedparam Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 10/15] nptl: Use exit_lock when accessing TID on pthread_getschedparam Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 11/15] nptl: Use exit_lock when accessing TID on pthread_getname_np Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 12/15] nptl: Use exit_lock when accessing TID on pthread_setname_np Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 13/15] nptl: Use exit_lock when accessing TID on pthread_sigqueue Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 14/15] nptl: Use exit_lock when accessing TID on pthread_setschedprio Adhemerval Zanella
2021-09-30 20:00 ` [PATCH 15/15] nptl: Remove INVALID_TD_P Adhemerval Zanella

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=20210930200051.1017457-5-adhemerval.zanella@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --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).