public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc/azanella/pthread-multiple-fixes] nptl: Move setxid flag out of cancelhandling
@ 2021-08-20 17:55 Adhemerval Zanella
0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2021-08-20 17:55 UTC (permalink / raw)
To: glibc-cvs
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7936b35020823080d89b8b99496b2d1868333505
commit 7936b35020823080d89b8b99496b2d1868333505
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Wed Jun 9 18:12:01 2021 -0300
nptl: Move setxid flag out of cancelhandling
Now that the thread state is tracked by PD->joinstate, there is no need
to keep the setxid flag within the PD->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.
Diff:
---
nptl/allocatestack.c | 3 +++
nptl/descr.h | 5 ++---
nptl/nptl_setxid.c | 49 +++++++++++--------------------------------------
nptl/pthread_create.c | 4 ++--
4 files changed, 18 insertions(+), 43 deletions(-)
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index cfe37a3443..ccb1aa21c1 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. */
@@ -301,6 +302,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. */
@@ -422,6 +424,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 4b2db6edab..563b152bff 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -297,9 +297,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;
@@ -339,6 +336,8 @@ struct pthread
/* Lock to synchronize access to the descriptor. */
int lock;
+ /* Indicate whether thread is supposed to change XID. */
+ 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 2f35772411..c337fa8577 100644
--- a/nptl/nptl_setxid.c
+++ b/nptl/nptl_setxid.c
@@ -76,14 +76,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;
@@ -97,8 +90,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))
@@ -109,41 +100,23 @@ 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
- {
- ch = t->cancelhandling;
+ /* If thread is exiting right now, ignore it. */
+ if (atomic_load_acquire (&t->joinstate) == THREAD_STATE_EXITING)
+ return;
- /* 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_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_exchange_release (&t->setxid_flag, 0);
/* Release the futex just in case. */
t->setxid_futex = 1;
@@ -154,7 +127,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 763e32bc3e..c00d6ae00c 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -541,7 +541,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. */
@@ -551,7 +551,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;
^ permalink raw reply [flat|nested] 2+ messages in thread
* [glibc/azanella/pthread-multiple-fixes] nptl: Move setxid flag out of cancelhandling
@ 2021-09-10 14:37 Adhemerval Zanella
0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2021-09-10 14:37 UTC (permalink / raw)
To: glibc-cvs
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d1b5e537f38deec5f34b9350881289e0308bd3ef
commit d1b5e537f38deec5f34b9350881289e0308bd3ef
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Wed Jun 9 18:12:01 2021 -0300
nptl: Move setxid flag out of cancelhandling
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.
Diff:
---
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 3bd9854e43..4d989683e9 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -554,7 +554,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. */
@@ -564,7 +564,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;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-10 14:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 17:55 [glibc/azanella/pthread-multiple-fixes] nptl: Move setxid flag out of cancelhandling Adhemerval Zanella
2021-09-10 14:37 Adhemerval Zanella
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).