From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 [thread overview]
Message-ID: <20210526165728.1772546-9-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org>
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,
--
2.30.2
next prev parent reply other threads:[~2021-05-26 16:57 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-26 16:57 [PATCH 00/11] nptl: pthread cancellation refactor Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 01/11] nptl: Move Linux createthread to nptl Adhemerval Zanella
2021-05-26 17:14 ` Florian Weimer
2021-05-26 16:57 ` [PATCH 02/11] nptl: Move createthread to pthread_create Adhemerval Zanella
2021-05-26 17:16 ` Florian Weimer
2021-05-26 17:29 ` Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 03/11] support: Add xpthread_attr_setaffinity_np wrapper Adhemerval Zanella
2021-05-26 17:17 ` Florian Weimer
2021-05-27 22:35 ` Joseph Myers
2021-05-28 1:08 ` Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 04/11] nptl: Add pthread_attr_setaffinity_np failure test Adhemerval Zanella
2021-05-26 17:21 ` Florian Weimer
2021-05-26 17:30 ` Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 05/11] nptl: Deallocate the thread stack on setup failure (BZ #19511) Adhemerval Zanella
2021-05-26 17:33 ` Florian Weimer
2021-05-26 17:51 ` Adhemerval Zanella
2021-05-26 17:58 ` Florian Weimer
2021-05-26 19:19 ` Adhemerval Zanella
2021-05-26 18:21 ` Andreas Schwab
2021-05-26 18:40 ` Adhemerval Zanella
2021-05-26 19:26 ` Adhemerval Zanella
2021-05-27 7:43 ` Florian Weimer
2021-05-26 16:57 ` [PATCH 06/11] nptl: Install cancellation handler on pthread_cancel Adhemerval Zanella
2021-05-26 17:38 ` Florian Weimer
2021-05-26 17:52 ` Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 07/11] nptl: Remove CANCELING_BITMASK Adhemerval Zanella
2021-05-26 18:02 ` Florian Weimer
2021-06-15 22:07 ` Florian Weimer
2021-06-15 23:33 ` Adhemerval Zanella
2021-06-16 12:46 ` Adhemerval Zanella
2021-05-26 16:57 ` Adhemerval Zanella [this message]
2021-05-26 18:20 ` [PATCH 08/11] nptl: Move cancel state out of cancelhandling Florian Weimer
2021-05-27 16:40 ` Adhemerval Zanella
2021-05-27 16:48 ` Florian Weimer
2021-05-27 16:57 ` Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 09/11] nptl: Move cancel type " Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 10/11] nptl: Implement raise in terms of pthread_kill Adhemerval Zanella
2021-05-26 16:57 ` [PATCH 11/11] nptl: Use pthread_kill on pthread_cancel Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210526165728.1772546-9-adhemerval.zanella@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).