From patchwork Mon May 3 21:00:03 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: 43239 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 8A2D1395742E; Mon, 3 May 2021 21:00:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8A2D1395742E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620075618; bh=gBJp2fdvTbw+QwStCVivUImNP5wBw5xFMUPTD5ObOdc=; 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=okk3Qp11qjFH4+IALeQahhq7b9BYfdCL0KEIbnmdJGm60r/hNdDlDPAMSncPEBgvl 6TEMSOxU1ifMtan3JNWwZ9z4Po6/R7NIpOakTFuNW17T8myXGnUQgNhdD5+JypRyo6 NSICTK0azud5J/R15f9+Yem0I10VwT62EecqEFKQ= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x729.google.com (mail-qk1-x729.google.com [IPv6:2607:f8b0:4864:20::729]) by sourceware.org (Postfix) with ESMTPS id 618D139551D7 for ; Mon, 3 May 2021 21:00:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 618D139551D7 Received: by mail-qk1-x729.google.com with SMTP id q127so6567904qkb.1 for ; Mon, 03 May 2021 14:00:15 -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=gBJp2fdvTbw+QwStCVivUImNP5wBw5xFMUPTD5ObOdc=; b=rqkdT/fwuycJdFqF9J890cY2AcZEl/XzLWGDo93MiWgHlNM1KG7TiinCv/5zNLhXoM uvtOF4QLSeR0+a2FOxHf8FjsTSKzhdvBgFSVdJhUrlZkjLxb+5zzG3Sc3O7N/lZVdp5e xu/1SILyX6DoCULrrKcted/7XA0/eARa7Lf4ZkxdvMkJTQD+CYseGaQZvKvBL7IqB8Be sTn4ks4oMxiaXSUsOiegxmwHF4pZ6J+nAkrRXlGMEByaRnqkprKo5V2rZgp9L2CSMSBM Xd6/Ytlf2cWnlLtsAjfbPIoXXPXHzSxAi9MgZFol3gIvTMUCsF8nYi7D/w8uXdGFeJC4 82ZA== X-Gm-Message-State: AOAM531+dfvCdnp9/jeg0KdKNUnq7dvDLl2GVp9CrXSxt4GqfiuebTf3 p1fTuv9MGZq4/fgRwx6KAJK66Kt+WKNmKw== X-Google-Smtp-Source: ABdhPJyvGiR+Y4TdqK3erYesTQyduw81HcPvJITnyc00anoH/0b7fUS3tBjDuymBQbMXHaIso5em2Q== X-Received: by 2002:a05:620a:4451:: with SMTP id w17mr20359157qkp.59.1620075614533; Mon, 03 May 2021 14:00:14 -0700 (PDT) Received: from birita.. ([177.194.41.149]) by smtp.googlemail.com with ESMTPSA id w196sm4578090qkb.90.2021.05.03.14.00.13 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 May 2021 14:00:14 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v3 4/6] nptl: Move cancel state out of cancelhandling Date: Mon, 3 May 2021 18:00:03 -0300 Message-Id: <20210503210005.2891859-4-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210503210005.2891859-1-adhemerval.zanella@linaro.org> References: <20210503210005.2891859-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" Changes from v2: * Rebased against master. Changes from v1: * Rebased against master. --- The thread cancellation state is not accessed concurrently internally neither the pthread interface allows changing the state of a different thread than its own. By removing the cancel state out of the internal thread cancel handling state there is no need to check if cancelled bit was set in CAS operation. 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. --- 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 | 3 ++- nptl/pthread_join_common.c | 5 ++++- nptl/pthread_setcancelstate.c | 36 +++-------------------------------- nptl/pthread_setcanceltype.c | 3 ++- nptl/pthread_testcancel.c | 11 ++++++++++- 13 files changed, 33 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 f1270c3109..eb25a2a034 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -219,6 +219,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; /* No pending event. */ diff --git a/nptl/cancellation.c b/nptl/cancellation.c index 2ee633eabc..1ba94860fe 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -45,7 +45,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 08271e352f..3f7efc939f 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -86,6 +86,6 @@ __pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } diff --git a/nptl/descr.h b/nptl/descr.h index d423a53bbf..465af117e5 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) @@ -301,11 +298,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)) @@ -409,6 +403,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 4116970b77..7d2f33be0d 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -259,18 +259,6 @@ extern int __pthread_debug attribute_hidden; #endif -/* 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 060484cdc8..b3dbf26843 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -64,7 +64,8 @@ __pthread_cancel (pthread_t th) /* If the cancellation is handled asynchronously just send a signal. We avoid this if possible since it's more expensive. */ - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (pd->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { /* Mark the cancellation as "in progress". */ if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index a99c26e27e..e4efd83715 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 & (CANCELING_BITMASK | 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 b6964904bb..c0d22eb77c 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -24,7 +24,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 (); + } } libc_hidden_def (__pthread_testcancel)