From patchwork Mon May 3 21:00:04 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: 1473387 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=pCOtxOsk; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FYwNd0X4pz9sRR for ; Tue, 4 May 2021 07:00:33 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A9B9839574C5; Mon, 3 May 2021 21:00:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A9B9839574C5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620075619; bh=OukRUrbzJdSRBZYH/b24VI++CZHepPl9UGO445h+GJQ=; 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=pCOtxOskfCQzBXlwxRFznbzXoyivq/qafw9ZtHzHaWYTr4T5OnBIpvAMBo7T0IaCw +0fA3VswJ3ncnFeDXpJfgf2x5aEEJq+WOiCw7ORpChCv8XTASBG0/QJPKrvdRAts7/ 14BEiUpEeRZyxg+IsLLK91eUV46qO+IJWEqAM0Dk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) by sourceware.org (Postfix) with ESMTPS id 1C34D39540D8 for ; Mon, 3 May 2021 21:00:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1C34D39540D8 Received: by mail-qt1-x835.google.com with SMTP id a18so4790521qtj.10 for ; Mon, 03 May 2021 14:00:17 -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=OukRUrbzJdSRBZYH/b24VI++CZHepPl9UGO445h+GJQ=; b=RRpZ0JI2vCZDXynrxOCrnClIlHaFN8VtdqOet2vS7ah4beguSC1BkuvrxtEGPDW5wE eNrv4iqjbdrw9NTCIFL37wRbVjEk9bGHgcc7HX0hdLB5ErNwQ1IQbd+zDU1YZjMvyD6p n9TX85QG6TH5krkSZlxyzUpX2PWrse1C2bKHFJbe4ESDn8E14n5N2APgB1lQLYQhwHFm zmO2lXahs8/OktoVFiuwmsQy7eG+9aW4yhTgnegGyl7creAtGwbW3VYDl/C5H8Km8HQb 2sLCinBbs8s/E+zqp79gzyfXXg9xX3x+rgGXzUGSMsP2UN9fJ7rrxEk9cLKRmXsDczme 6o7w== X-Gm-Message-State: AOAM532WsOjWFLy/9prNjRPDe4XwHcRcEAuSYAOmFVB8ssYeG08saGH8 K5DmlCsBt1L4Hd/KZmn2WyoZyxIBhjLI9g== X-Google-Smtp-Source: ABdhPJw7qi7AHE9LGWNIi2dFZGbTrkhxcvjuCLE8L/h1MeJgZJBp6/9MJoK23MFuXvjeVd/2zZ3SCg== X-Received: by 2002:ac8:548c:: with SMTP id h12mr19668521qtq.272.1620075615998; Mon, 03 May 2021 14:00:15 -0700 (PDT) Received: from birita.. ([177.194.41.149]) by smtp.googlemail.com with ESMTPSA id w196sm4578090qkb.90.2021.05.03.14.00.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 May 2021 14:00:15 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v3 5/6] nptl: Move cancel type out of cancelhandling Date: Mon, 3 May 2021 18:00:04 -0300 Message-Id: <20210503210005.2891859-5-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.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, 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 type is not accessed concurrently internally neither its 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. It allows simplifing the cancellation wrappers and the CANCEL_CANCELED_AND_ASYNCHRONOUS is removed. Checked on x86_64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/cancellation.c | 60 ++++++++++-------------------------- nptl/cleanup_defer.c | 46 +++------------------------ nptl/descr.h | 14 +++------ nptl/libc-cleanup.c | 44 +++----------------------- nptl/nptl-init.c | 2 +- nptl/pthread_cancel.c | 5 ++- nptl/pthread_setcanceltype.c | 42 +++---------------------- 8 files changed, 41 insertions(+), 173 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index eb25a2a034..53444e2870 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -220,6 +220,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->canceltype = PTHREAD_CANCEL_DEFERRED; result->cleanup = NULL; /* No pending event. */ diff --git a/nptl/cancellation.c b/nptl/cancellation.c index 1ba94860fe..d039f424fb 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -32,31 +32,19 @@ attribute_hidden __pthread_enable_asynccancel (void) { struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = oldval | CANCELTYPE_BITMASK; - - if (newval == oldval) - break; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } + int oldval = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_ASYNCHRONOUS); - break; - } + int ch = THREAD_GETMEM (self, cancelhandling); - /* Prepare the next round. */ - oldval = curval; + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (ch & CANCELED_BITMASK) + && !(ch & EXITING_BITMASK) + && !(ch & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); } return oldval; @@ -70,36 +58,22 @@ __pthread_disable_asynccancel (int oldtype) { /* If asynchronous cancellation was enabled before we do not have anything to do. */ - if (oldtype & CANCELTYPE_BITMASK) + if (oldtype == PTHREAD_CANCEL_ASYNCHRONOUS) return; struct pthread *self = THREAD_SELF; - int newval; - - int oldval = THREAD_GETMEM (self, cancelhandling); - - while (1) - { - newval = oldval & ~CANCELTYPE_BITMASK; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - break; - - /* Prepare the next round. */ - oldval = curval; - } + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); /* We cannot return when we are being canceled. Upon return the thread might be things which would have to be undone. The following loop should loop until the cancellation signal is delivered. */ - while (__builtin_expect ((newval & (CANCELING_BITMASK | CANCELED_BITMASK)) - == CANCELING_BITMASK, 0)) + int ch = THREAD_GETMEM (self, cancelhandling); + while (__glibc_unlikely ((ch & CANCELING_BITMASK) + && !(ch & CANCELED_BITMASK))) { - futex_wait_simple ((unsigned int *) &self->cancelhandling, newval, + futex_wait_simple ((unsigned int *) &self->cancelhandling, ch, FUTEX_PRIVATE); - newval = THREAD_GETMEM (self, cancelhandling); + ch = THREAD_GETMEM (self, cancelhandling); } } diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 3f7efc939f..8a41069ebf 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -31,27 +31,9 @@ __pthread_register_cancel_defer (__pthread_unwind_buf_t *buf) ibuf->priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf); ibuf->priv.data.cleanup = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - ibuf->priv.data.canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + ibuf->priv.data.canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); /* Store the new cleanup handler info. */ THREAD_SETMEM (self, cleanup_jmp_buf, (struct pthread_unwind_buf *) buf); @@ -67,25 +49,7 @@ __pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) THREAD_SETMEM (self, cleanup_jmp_buf, ibuf->priv.data.prev); - int cancelhandling; - if (ibuf->priv.data.canceltype != PTHREAD_CANCEL_DEFERRED - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - __pthread_testcancel (); - } + THREAD_SETMEM (self, canceltype, ibuf->priv.data.canceltype); + if (ibuf->priv.data.canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); } diff --git a/nptl/descr.h b/nptl/descr.h index 465af117e5..8ef877b1cd 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 asynchronous cancellation mode is selected. */ -#define CANCELTYPE_BIT 1 -#define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) /* Bit set if canceling has been initiated. */ #define CANCELING_BIT 2 #define CANCELING_BITMASK (0x01 << CANCELING_BIT) @@ -295,13 +292,6 @@ struct pthread /* Bit set if thread is supposed to change XID. */ #define SETXID_BIT 6 #define SETXID_BITMASK (0x01 << SETXID_BIT) - /* Mask for the rest. Helps the compiler to optimize. */ -#define CANCEL_RESTMASK 0xffffff80 - -#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ - | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ - == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) /* Flags. Including those copied from the thread attribute. */ int flags; @@ -407,6 +397,10 @@ struct pthread PTHREAD_CANCEL_DISABLE). */ unsigned char cancelstate; + /* Thread cancel type (PTHREAD_CANCEL_DEFERRED or + PTHREAD_CANCEL_ASYNCHRONOUS). */ + unsigned char canceltype; + /* This member must be last. */ char end_padding[]; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 6286b8b525..180d15bc9e 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -27,27 +27,9 @@ __libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer) buffer->__prev = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + buffer->__canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); THREAD_SETMEM (self, cleanup, buffer); } @@ -60,26 +42,8 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) THREAD_SETMEM (self, cleanup, buffer->__prev); - int cancelhandling; - if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0) - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - + THREAD_SETMEM (self, canceltype, buffer->__canceltype); + if (buffer->__canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __pthread_testcancel (); - } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index b0879bd87e..82f0284979 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -84,7 +84,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) THREAD_SETMEM (self, result, PTHREAD_CANCELED); /* Make sure asynchronous cancellation is still enabled. */ - if ((newval & CANCELTYPE_BITMASK) != 0) + if (self->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) /* Run the registered destructors and terminate the thread. */ __do_cancel (); diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index b3dbf26843..d48d04881c 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -65,7 +65,10 @@ __pthread_cancel (pthread_t th) signal. We avoid this if possible since it's more expensive. */ if (pd->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) + && pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS + && (newval & CANCELED_BITMASK) + && !(newval & EXITING_BITMASK) + && !(newval & TERMINATED_BITMASK)) { /* Mark the cancellation as "in progress". */ if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index ae5df1d591..e7b24ae733 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -29,43 +29,11 @@ __pthread_setcanceltype (int type, int *oldtype) volatile struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (type == PTHREAD_CANCEL_ASYNCHRONOUS - ? oldval | CANCELTYPE_BITMASK - : oldval & ~CANCELTYPE_BITMASK); - - /* Store the old value. */ - if (oldtype != NULL) - *oldtype = ((oldval & CANCELTYPE_BITMASK) - ? PTHREAD_CANCEL_ASYNCHRONOUS : PTHREAD_CANCEL_DEFERRED); - - /* 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 (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldtype != NULL) + *oldtype = self->canceltype; + self->canceltype = type; + if (type == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); return 0; }