From 2b5174253155bdace1262ea2ab53d11347ecdefd Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella Date: Tue, 31 Mar 2020 15:43:25 -0300 Subject: [PATCH] nptl: Move cancel state out of cancelhandling Now that thread cancellation state is not accessed concurrently anymore, it is possible to move it out the 'cancelhandling'. The code is also simplified: CANCELLATION_P is replaced with a internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is removed. With this behavior pthread_setcancelstate does not require to act on cancellation if cancel type is asynchronous (is already handled either by pthread_setcanceltype or by the signal handler). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- manual/pattern.texi | 1 - manual/process.texi | 3 +-- nptl/allocatestack.c | 1 + nptl/cancellation.c | 3 ++- nptl/cleanup_defer.c | 2 +- nptl/descr.h | 14 ++++++-------- nptl/libc-cleanup.c | 2 +- nptl/pthreadP.h | 12 ------------ nptl/pthread_cancel.c | 2 +- nptl/pthread_join_common.c | 5 ++++- nptl/pthread_setcancelstate.c | 36 +++-------------------------------- nptl/pthread_setcanceltype.c | 3 ++- nptl/pthread_testcancel.c | 11 ++++++++++- sysdeps/nptl/dl-tls_init_tp.c | 2 ++ 14 files changed, 34 insertions(+), 63 deletions(-) diff --git a/manual/pattern.texi b/manual/pattern.texi index 39ae97a3c4..4fa4c25525 100644 --- a/manual/pattern.texi +++ b/manual/pattern.texi @@ -1820,7 +1820,6 @@ the beginning of the vector. @c (disable cancellation around exec_comm; it may do_cancel the @c second time, if async cancel is enabled) @c THREAD_ATOMIC_CMPXCHG_VAL dup ok -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel @ascuplugin @ascuheap @acsmem @c THREAD_ATOMIC_BIT_SET dup ok @c pthread_unwind @ascuplugin @ascuheap @acsmem diff --git a/manual/process.texi b/manual/process.texi index 54e65f76c6..134d5c6143 100644 --- a/manual/process.texi +++ b/manual/process.texi @@ -68,7 +68,7 @@ until the subprogram terminates before you can do anything else. @c CLEANUP_HANDLER @ascuplugin @ascuheap @acsmem @c libc_cleanup_region_start @ascuplugin @ascuheap @acsmem @c pthread_cleanup_push_defer @ascuplugin @ascuheap @acsmem -@c CANCELLATION_P @ascuplugin @ascuheap @acsmem +@c __pthread_testcancel @ascuplugin @ascuheap @acsmem @c CANCEL_ENABLED_AND_CANCELED ok @c do_cancel @ascuplugin @ascuheap @acsmem @c cancel_handler ok @@ -86,7 +86,6 @@ until the subprogram terminates before you can do anything else. @c SINGLE_THREAD_P ok @c LIBC_CANCEL_ASYNC @ascuplugin @ascuheap @acsmem @c libc_enable_asynccancel @ascuplugin @ascuheap @acsmem -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel dup @ascuplugin @ascuheap @acsmem @c LIBC_CANCEL_RESET ok @c libc_disable_asynccancel ok diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 2114bd2e27..54e95baad7 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -160,6 +160,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; + result->cancelstate = PTHREAD_CANCEL_ENABLE; result->cleanup = NULL; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index b15f25d8f6..ce00603109 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -44,7 +44,8 @@ __pthread_enable_asynccancel (void) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 6d85359118..873ab007fd 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -92,7 +92,7 @@ ___pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } versioned_symbol (libc, ___pthread_unregister_cancel_restore, diff --git a/nptl/descr.h b/nptl/descr.h index a120365f88..35f5330e7f 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -277,9 +277,6 @@ struct pthread /* Flags determining processing of cancellation. */ int cancelhandling; - /* Bit set if cancellation is disabled. */ -#define CANCELSTATE_BIT 0 -#define CANCELSTATE_BITMASK (0x01 << CANCELSTATE_BIT) /* Bit set if asynchronous cancellation mode is selected. */ #define CANCELTYPE_BIT 1 #define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) @@ -298,11 +295,8 @@ struct pthread /* Mask for the rest. Helps the compiler to optimize. */ #define CANCEL_RESTMASK 0xffffff80 -#define CANCEL_ENABLED_AND_CANCELED(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELED_BITMASK | EXITING_BITMASK \ - | CANCEL_RESTMASK | TERMINATED_BITMASK)) == CANCELED_BITMASK) -#define CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELTYPE_BITMASK | CANCELED_BITMASK \ +#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ + (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) @@ -404,6 +398,10 @@ struct pthread /* Indicates whether is a C11 thread created by thrd_creat. */ bool c11; + /* Thread cancel state (PTHREAD_CANCEL_ENABLE or + PTHREAD_CANCEL_DISABLE). */ + unsigned char cancelstate; + /* Used on strsignal. */ struct tls_internal_t tls_state; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 14ccfe9285..6286b8b525 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -79,7 +79,7 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 48d48e7008..3e7a4f52ab 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -245,18 +245,6 @@ libc_hidden_proto (__pthread_current_priority) #define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) #define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0) -/* Cancellation test. */ -#define CANCELLATION_P(self) \ - do { \ - int cancelhandling = THREAD_GETMEM (self, cancelhandling); \ - if (CANCEL_ENABLED_AND_CANCELED (cancelhandling)) \ - { \ - THREAD_SETMEM (self, result, PTHREAD_CANCELED); \ - __do_cancel (); \ - } \ - } while (0) - - extern void __pthread_unwind (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute __attribute ((__noreturn__)) #if !defined SHARED && !IS_IN (libpthread) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 33e82c3717..f4f08363cf 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -45,7 +45,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) int ch = atomic_load_relaxed (&self->cancelhandling); /* Cancelation not enabled, not cancelled, or already exitting. */ - if ((ch & CANCELSTATE_BITMASK) != 0 + if (self->cancelstate == PTHREAD_CANCEL_DISABLE || (ch & CANCELED_BITMASK) == 0 || (ch & EXITING_BITMASK) != 0) return; diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index f842c91a08..7303069316 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -59,7 +59,10 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK | TERMINATED_BITMASK)) == 0)) - && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling)) + && !(self->cancelstate == PTHREAD_CANCEL_ENABLE + && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) + == CANCELED_BITMASK)) /* This is a deadlock situation. The threads are waiting for each other to finish. Note that this is a "may" error. To be 100% sure we catch this error we would have to lock the data diff --git a/nptl/pthread_setcancelstate.c b/nptl/pthread_setcancelstate.c index e3696ca348..7e2b6e4974 100644 --- a/nptl/pthread_setcancelstate.c +++ b/nptl/pthread_setcancelstate.c @@ -31,39 +31,9 @@ __pthread_setcancelstate (int state, int *oldstate) self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (state == PTHREAD_CANCEL_DISABLE - ? oldval | CANCELSTATE_BITMASK - : oldval & ~CANCELSTATE_BITMASK); - - /* Store the old value. */ - if (oldstate != NULL) - *oldstate = ((oldval & CANCELSTATE_BITMASK) - ? PTHREAD_CANCEL_DISABLE : PTHREAD_CANCEL_ENABLE); - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the memory has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* Update the cancel handling word. This has to be done - atomically since other bits could be modified as well. */ - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) - __do_cancel (); - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldstate != NULL) + *oldstate = self->cancelstate; + self->cancelstate = state; return 0; } diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index 5f061d512b..ae5df1d591 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -53,7 +53,8 @@ __pthread_setcanceltype (int type, int *oldtype) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c index a9e941ddb7..920374643a 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -23,7 +23,16 @@ void ___pthread_testcancel (void) { - CANCELLATION_P (THREAD_SELF); + struct pthread *self = THREAD_SELF; + int cancelhandling = THREAD_GETMEM (self, cancelhandling); + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (cancelhandling & CANCELED_BITMASK) + && !(cancelhandling & EXITING_BITMASK) + && !(cancelhandling & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); + } } versioned_symbol (libc, ___pthread_testcancel, pthread_testcancel, GLIBC_2_34); versioned_symbol (libc, ___pthread_testcancel, __pthread_testcancel, diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c index 1f7790297f..378b26a2f5 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -94,4 +94,6 @@ __tls_init_tp (void) It will be bigger than it actually is, but for unwind.c/pt-longjmp.c purposes this is good enough. */ THREAD_SETMEM (pd, stackblock_size, (size_t) __libc_stack_end); + + THREAD_SETMEM (pd, cancelstate, PTHREAD_CANCEL_ENABLE); }