From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x843.google.com (mail-qt1-x843.google.com [IPv6:2607:f8b0:4864:20::843]) by sourceware.org (Postfix) with ESMTPS id D9F48385BF92 for ; Wed, 1 Apr 2020 14:24:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D9F48385BF92 Received: by mail-qt1-x843.google.com with SMTP id z12so95955qtq.5 for ; Wed, 01 Apr 2020 07:24:57 -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; bh=Hs7a1kD+pZlccVgcRGfFFM02C9+8wZOxbib4YwjPF8c=; b=Z28GqONZwGcq1Co/VkcoRHa/srbJcYuGgOt7kZua6VHkAtXggWgwyYU/gxSRTrGMrF CxtJYMZAFJErBpgfEoxj/uvyK1uZD8NLNYzcULBHNUKJO4AJBcIhHuh4HTx1kiP5WCGD /H/227u1UcjIC6ojjBEKXsxSnCDH4b8ADKQ+jhUvDdL3xqEgPyfHdPQU4AwikY+fNfh8 4isOYTky+wIdOlX3iAWpJabYeQR7pO9l1t5uuWU+qtP+TfT6Kdq043ITHzH2TJ8Sm7hI bW7n7uZ5mEowcD9NoGLzW1ZPGWy6IkbpS3LWBiqIKUIivxfgfqPfszhCThA0sfIbmx3k urvw== X-Gm-Message-State: ANhLgQ1DyQU5+wygjMUqWiTkGk2O4HI39cQu1F75hejFUeenXmgce9/T EqG3eydLjA8C2PUpPFls14AqX22M4kc= X-Google-Smtp-Source: ADFU+vslDDqw8Tisv4aLBEoOlID9OVZgO6GSlc3LngGxGUnrkq6oqLD+JuStXGtFvXgSmbvQXW+SHA== X-Received: by 2002:ac8:376d:: with SMTP id p42mr10816888qtb.378.1585751086258; Wed, 01 Apr 2020 07:24:46 -0700 (PDT) Received: from localhost.localdomain ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id p38sm1592420qtf.50.2020.04.01.07.24.44 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2020 07:24:45 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 2/3] nptl: Move cancel state out of cancelhandling Date: Wed, 1 Apr 2020 11:24:38 -0300 Message-Id: <20200401142439.1906574-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200401142439.1906574-1-adhemerval.zanella@linaro.org> References: <20200401142439.1906574-1-adhemerval.zanella@linaro.org> X-Spam-Status: No, score=-26.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, 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-List-Received-Date: Wed, 01 Apr 2020 14:24:59 -0000 The thread cancellation state is not accessed concurrently internally neither the pthread interface allows changing the state of a different thread than its own. The code is also simplified: the CANCELLATION_P is replaced with a internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is removed. Checked on x86_64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/cancellation.c | 3 ++- nptl/cleanup_defer.c | 2 +- nptl/cleanup_defer_compat.c | 2 +- nptl/descr.h | 13 +++++-------- 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 ++++++++++- 11 files changed, 31 insertions(+), 60 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index c94980c21c..d9174274c2 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -232,6 +232,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 826071321e..7e8cbe9fe1 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 8ad9a90c50..33d4ea6eef 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/cleanup_defer_compat.c b/nptl/cleanup_defer_compat.c index 33e47888f2..a1ad291fcc 100644 --- a/nptl/cleanup_defer_compat.c +++ b/nptl/cleanup_defer_compat.c @@ -83,7 +83,7 @@ _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer, cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } /* If necessary call the cleanup routine after we removed the diff --git a/nptl/descr.h b/nptl/descr.h index 9dcf480bdf..61665bf859 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -269,9 +269,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) @@ -293,11 +290,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)) @@ -394,6 +388,9 @@ struct pthread /* Indicates whether is a C11 thread created by thrd_creat. */ bool c11; + /* Thread cancel state (enable, disable). */ + unsigned char cancelstate; + /* This member must be last. */ char end_padding[]; diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 06fb0d74c5..d55c3b26a4 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -258,18 +258,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 88c1ab8f6a..5b2789d620 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -55,7 +55,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 a96ceafde4..03e202136f 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -105,7 +105,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 4d7f413e19..aa1c8073a8 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 fcaae8abc7..cc0507ae04 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 30408c2008..3ffff4ebef 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 | EXITING_BITMASK + | TERMINATED_BITMASK)) + == CANCELED_BITMASK) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); + } } strong_alias (__pthread_testcancel, pthread_testcancel) hidden_def (__pthread_testcancel) -- 2.17.1