From patchwork Wed May 26 16:57:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43596 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6985D394742F; Wed, 26 May 2021 16:57:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6985D394742F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048267; bh=GDCVr1e5CWrN8/8b9pgzMFxSoVxKMtk+qlgRLUujbzA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=qsXZFHeoDNpnTPafA8P26zqOAAbLYUTyD16lS1v/pahSLbqxsTdTzHCchFtTwqC45 JiCct5a2Pm58Wx8dmPhpkHQuROyEaJdSjNkQg5/+pdEMdizs2bP9AnQk10feKs9Qxu P4GYIBL5c0XYaO/BwPiAMlDMwO6aZH2HIPiKFAjk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by sourceware.org (Postfix) with ESMTPS id 60AB1394741A for ; Wed, 26 May 2021 16:57:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 60AB1394741A Received: by mail-qt1-x834.google.com with SMTP id h24so1374163qtm.12 for ; Wed, 26 May 2021 09:57:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=GDCVr1e5CWrN8/8b9pgzMFxSoVxKMtk+qlgRLUujbzA=; b=eRqJwlLBipi6YJXN4tq4F8hsT3SJaTaKsDP/IoIGn4SThpyk8MEQKU0MXKCi9TX75u wB/buBvObAWTd/UXDbQXQVr/QKJrOs7cb4nR8yCtn0ByC9CYiiVL73W5Z0RTgTnj76Z4 jdDw0qZ4yq2g1dUfQyHVPMBgtX43mD1Ek2I4DXvwPY2IeMgBri0xLS7rzpspU8icc2ND 4zSuhbHdSZt3f3LYKbl9wY9qJiMTXIsBM6xNZZBMmfaaJpxjhYt5l5zFnTHG7lsA0Ib2 Z6LVaHQcUive223RxoiHqNNoJAQwaYC6rLmueoCTeuVC219z+jS7SbkpnGVMWPNe2P51 k3SA== X-Gm-Message-State: AOAM531Ot+RysG35xW70ETjqPIldkgFsKeyrbMoi2x4Q7PshxP26B1vd bKaOZmyEPwCwYBG/XBhKP5RMaf0xoPmU2g== X-Google-Smtp-Source: ABdhPJyjeSKq3t65ZzidmzAPNRpv/W06unhrI9/LypWp7cMEwcb0uxVt8ZTS1qkQRCZjXTBVVButog== X-Received: by 2002:ac8:5d8f:: with SMTP id d15mr39428514qtx.350.1622048263583; Wed, 26 May 2021 09:57:43 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.42 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:43 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 08/11] nptl: Move cancel state out of cancelhandling Date: Wed, 26 May 2021 13:57:25 -0300 Message-Id: <20210526165728.1772546-9-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that thread cancellation state is not accessed concurrently anymore, it is possible to move it out the 'cancelhandling'. The code is also simplified: the CANCELLATION_P is replaced with a internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is removed. The second part of this patchset also keeps the pthread_setcanceltype as cancellation entrypoint by calling pthread_testcancel if the new type is PTHREAD_CANCEL_ASYNCHRONOUS. 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 ++++++++++- 13 files changed, 32 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..a3084fdf60 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)) @@ -407,6 +401,10 @@ struct pthread /* Used on strsignal. */ struct tls_internal_t tls_state; + /* Thread cancel state (PTHREAD_CANCEL_ENABLE or + PTHREAD_CANCEL_DISABLE). */ + unsigned char cancelstate; + /* This member must be last. */ char end_padding[]; 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 c11a2b4551..0588f086a8 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,