From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1791) id 7C555385041D; Fri, 20 Aug 2021 17:55:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7C555385041D Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Adhemerval Zanella To: glibc-cvs@sourceware.org Subject: [glibc/azanella/pthread-multiple-fixes] nptl: Move setxid flag out of cancelhandling X-Act-Checkin: glibc X-Git-Author: Adhemerval Zanella X-Git-Refname: refs/heads/azanella/pthread-multiple-fixes X-Git-Oldrev: ebae3185ca5529ad2147e6f75978a13e2d459b8e X-Git-Newrev: 7936b35020823080d89b8b99496b2d1868333505 Message-Id: <20210820175543.7C555385041D@sourceware.org> Date: Fri, 20 Aug 2021 17:55:43 +0000 (GMT) X-BeenThere: glibc-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Glibc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Aug 2021 17:55:43 -0000 https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7936b35020823080d89b8b99496b2d1868333505 commit 7936b35020823080d89b8b99496b2d1868333505 Author: Adhemerval Zanella 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;