From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x829.google.com (mail-qt1-x829.google.com [IPv6:2607:f8b0:4864:20::829]) by sourceware.org (Postfix) with ESMTPS id A66C43846416 for ; Wed, 26 May 2021 16:57:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A66C43846416 Received: by mail-qt1-x829.google.com with SMTP id s12so1392077qta.3 for ; Wed, 26 May 2021 09:57:45 -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=T21vdovwjxtlzwTo7Ep4F7q+YV0pVAVeGc1yHGpSMoY=; b=E9jSy/m3FYvPH0LiI4icrOareD0igzAguApumHveQe0l97Ju4vt66uFORUyr2HWbOa 9+7SFWNxO4HWdgHgQATFjjBrADyhMn5zwTjo/oHYQcc5SuBhSxNELQ+tNMDoXl4et5Ro seG7R1CgkVvJu4cnTKXueuXQZufdSxw7eXERxvlroYwO+ipOHJoTpxm7tk7niUN4IWYA 7iRQiknsmkfRiydHqd4PiiJHzChe/+vFLt8tDpggI6iL2LZewIVbWn+L9pBIrvHCQtRy yMveWdVMfwouzs6ia1kj/7NCf6oLm2bROq3Vm9zl8ylkv1ioExDrae/wfxIZSBoj8cMQ fk7Q== X-Gm-Message-State: AOAM533x4AXkuC5gaZcRdFyxlE5SZmD1bScRYlwcqG5k4q0YE4d12Tho WOJ895VdOwn8JnHRbk4E8kbGMi9f4NArbw== X-Google-Smtp-Source: ABdhPJzvGL45CkrZ2x5E0mbjjj5DnsyKbG+esTgUeM9iC1BMkr16hXUhnCPhkk/5m/2PLpT5IygEmw== X-Received: by 2002:a05:622a:1044:: with SMTP id f4mr39156545qte.181.1622048264849; Wed, 26 May 2021 09:57:44 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:44 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 09/11] nptl: Move cancel type out of cancelhandling Date: Wed, 26 May 2021 13:57:26 -0300 Message-Id: <20210526165728.1772546-10-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 Content-Transfer-Encoding: 8bit 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-List-Received-Date: Wed, 26 May 2021 16:57:47 -0000 Now that the thread cancellation type is not accessed concurrently anymore, it is possible to move it out the cancelhandling. 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 and aarch64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/cancellation.c | 51 +++++++++--------------------------- nptl/cleanup_defer.c | 46 ++++---------------------------- nptl/descr.h | 14 +++------- nptl/libc-cleanup.c | 44 +++---------------------------- nptl/pthread_cancel.c | 4 +-- nptl/pthread_setcanceltype.c | 42 ++++------------------------- 7 files changed, 33 insertions(+), 169 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 54e95baad7..9be6c42894 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -161,6 +161,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; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index ce00603109..05962784d5 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -31,31 +31,19 @@ int __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; @@ -69,25 +57,10 @@ __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; - } + self->canceltype = PTHREAD_CANCEL_DEFERRED; } libc_hidden_def (__pthread_disable_asynccancel) diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 873ab007fd..7e858d0df0 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); @@ -73,27 +55,9 @@ ___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 (); } versioned_symbol (libc, ___pthread_unregister_cancel_restore, __pthread_unregister_cancel_restore, GLIBC_2_34); diff --git a/nptl/descr.h b/nptl/descr.h index a3084fdf60..04476b3f4e 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 canceled. */ #define CANCELED_BIT 3 #define CANCELED_BITMASK (0x01 << CANCELED_BIT) @@ -292,13 +289,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; @@ -405,6 +395,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/pthread_cancel.c b/nptl/pthread_cancel.c index 0588f086a8..3be3d0fce7 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -53,7 +53,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) /* Set the return value. */ THREAD_SETMEM (self, result, PTHREAD_CANCELED); /* Make sure asynchronous cancellation is still enabled. */ - if ((ch & CANCELTYPE_BITMASK) != 0) + if (self->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); } @@ -104,7 +104,7 @@ __pthread_cancel (pthread_t th) #endif THREAD_SETMEM (pd, result, PTHREAD_CANCELED); - if ((oldch & CANCELTYPE_BITMASK) != 0) + if (pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); return 0; } 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; } -- 2.30.2