From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH 02/11] nptl: Consolidate aysnc cancel enable/disable implementation in libc
Date: Wed, 5 May 2021 11:21:27 -0300 [thread overview]
Message-ID: <9f5da3d5-9d40-3615-1e07-376b2a346708@linaro.org> (raw)
In-Reply-To: <fd42c6bf2b221dd7b390a3bfac4f24523b8d23f5.1620049437.git.fweimer@redhat.com>
On 03/05/2021 10:51, Florian Weimer via Libc-alpha wrote:
> Previously, the source file nptl/cancellation.c was compiled multiple
> times, for libc, libpthread, librt. This commit switches to a single
> implementation, with new __pthread_enable_asynccancel@@GLIBC_PRIVATE,
> __pthread_disable_asynccancel@@GLIBC_PRIVATE exports.
>
> The almost-unused CANCEL_ASYNC and CANCEL_RESET macros are replaced
> by LIBC_CANCEL_ASYNC and LIBC_CANCEL_ASYNC macros. They call the
> __pthread_* functions unconditionally now. The macros are still
> needed because shared code uses them; Hurd has different definitions.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/Makefile | 6 ++--
> manual/llio.texi | 4 +--
> nptl/Makefile | 4 +--
> nptl/Versions | 2 ++
> nptl/cancellation.c | 4 +--
> nptl/libc-cancellation.c | 24 --------------
> nptl/pthreadP.h | 2 --
> nptl/pthread_create.c | 4 +--
> rt/Makefile | 1 -
> sysdeps/nptl/Makefile | 3 +-
> sysdeps/nptl/librt-cancellation.c | 24 --------------
> sysdeps/nptl/lowlevellock-futex.h | 8 ++---
> sysdeps/unix/sysv/linux/socketcall.h | 5 ---
> sysdeps/unix/sysv/linux/sysdep-cancel.h | 44 ++++---------------------
> 14 files changed, 25 insertions(+), 110 deletions(-)
> delete mode 100644 nptl/libc-cancellation.c
> delete mode 100644 sysdeps/nptl/librt-cancellation.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index f09988f7d2..4f99af626f 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -528,8 +528,10 @@ $(objpfx)dl-allobjs.os: $(all-rtld-routines:%=$(objpfx)%.os)
> # discovery mechanism is not compatible with the libc implementation
> # when compiled for libc.
> rtld-stubbed-symbols = \
> - __libc_disable_asynccancel \
> - __libc_enable_asynccancel \
> + __GI___pthread_disable_asynccancel \
> + __GI___pthread_enable_asynccancel \
> + __pthread_disable_asynccancel \
> + __pthread_enable_asynccancel \
> calloc \
> free \
> malloc \
Ok.
> diff --git a/manual/llio.texi b/manual/llio.texi
> index c0a53e1a6e..cbc4909fd5 100644
> --- a/manual/llio.texi
> +++ b/manual/llio.texi
> @@ -2534,13 +2534,13 @@ aiocb64}, since the LFS transparently replaces the old interface.
> @c sigemptyset ok
> @c sigaddset ok
> @c setjmp ok
> -@c CANCEL_ASYNC -> pthread_enable_asynccancel ok
> +@c LIBC_CANCEL_ASYNC -> __pthread_enable_asynccancel ok
> @c do_cancel ok
> @c pthread_unwind ok
> @c Unwind_ForcedUnwind or longjmp ok [@ascuheap @acsmem?]
> @c lll_lock @asulock @aculock
> @c lll_unlock @asulock @aculock
> -@c CANCEL_RESET -> pthread_disable_asynccancel ok
> +@c LIBC_CANCEL_RESET -> __pthread_disable_asynccancel ok
> @c lll_futex_wait ok
> @c ->start_routine ok -----
> @c call_tls_dtors @asulock @ascuheap @aculock @acsmem
Ok.
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 884cb69bb4..1337b9e648 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -30,6 +30,7 @@ extra-libs-others := $(extra-libs)
>
> routines = \
> alloca_cutoff \
> + cancellation \
> cleanup_compat \
> cleanup_defer_compat \
> cleanup_routine \
> @@ -39,7 +40,6 @@ routines = \
> elision-trylock \
> elision-unlock \
> futex-internal \
> - libc-cancellation \
> libc-cleanup \
> libc_multiple_threads \
> libc_pthread_init \
Ok.
> @@ -157,7 +157,6 @@ shared-only-routines = forward
> static-only-routines = pthread_atfork
>
> libpthread-routines = \
> - cancellation \
> cleanup \
> cleanup_defer \
> events \
> @@ -239,7 +238,6 @@ CFLAGS-pthread_setcanceltype.c += -fexceptions -fasynchronous-unwind-tables
> # These are internal functions which similar functionality as setcancelstate
> # and setcanceltype.
> CFLAGS-cancellation.c += -fasynchronous-unwind-tables
> -CFLAGS-libc-cancellation.c += -fasynchronous-unwind-tables
>
> # Calling pthread_exit() must cause the registered cancel handlers to
> # be executed. Therefore exceptions have to be thrown through this
Ok.
> diff --git a/nptl/Versions b/nptl/Versions
> index ce09c73727..e845cbf804 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -281,6 +281,8 @@ libc {
> __pthread_cleanup_push;
> __pthread_cleanup_upto;
> __pthread_current_priority;
> + __pthread_disable_asynccancel;
> + __pthread_enable_asynccancel;
> __pthread_force_elision;
> __pthread_getattr_default_np;
> __pthread_keys;
Ok.
> diff --git a/nptl/cancellation.c b/nptl/cancellation.c
> index 2ee633eabc..c20845adc0 100644
> --- a/nptl/cancellation.c
> +++ b/nptl/cancellation.c
> @@ -28,7 +28,6 @@
> AS-safe, with the exception of the actual cancellation, because they
> are called by wrappers around AS-safe functions like write().*/
> int
> -attribute_hidden
> __pthread_enable_asynccancel (void)
> {
> struct pthread *self = THREAD_SELF;
> @@ -60,11 +59,11 @@ __pthread_enable_asynccancel (void)
>
> return oldval;
> }
> +libc_hidden_def (__pthread_enable_asynccancel)
>
> /* See the comment for __pthread_enable_asynccancel regarding
> the AS-safety of this function. */
> void
> -attribute_hidden
> __pthread_disable_asynccancel (int oldtype)
> {
> /* If asynchronous cancellation was enabled before we do not have
> @@ -102,3 +101,4 @@ __pthread_disable_asynccancel (int oldtype)
> newval = THREAD_GETMEM (self, cancelhandling);
> }
> }
> +libc_hidden_def (__pthread_disable_asynccancel)
Ok.
> diff --git a/nptl/libc-cancellation.c b/nptl/libc-cancellation.c
> deleted file mode 100644
> index 29f5a5864b..0000000000
> --- a/nptl/libc-cancellation.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* Copyright (C) 2002-2021 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include "pthreadP.h"
> -
> -
> -#define __pthread_enable_asynccancel __libc_enable_asynccancel
> -#define __pthread_disable_asynccancel __libc_disable_asynccancel
> -#include <nptl/cancellation.c>
Ok.
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index ee77928fc7..2d9e42173e 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -569,8 +569,6 @@ libc_hidden_proto (__pthread_exit)
> extern int __pthread_join (pthread_t threadid, void **thread_return);
> extern int __pthread_setcanceltype (int type, int *oldtype);
> libc_hidden_proto (__pthread_setcanceltype)
> -extern int __pthread_enable_asynccancel (void) attribute_hidden;
> -extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden;
> extern void __pthread_testcancel (void);
> libc_hidden_proto (__pthread_testcancel)
> extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
Ok.
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index d89a83b38a..775287d0e4 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -353,7 +353,7 @@ START_THREAD_DEFN
> have ownership (see CONCURRENCY NOTES above). */
> if (__glibc_unlikely (pd->stopped_start))
> {
> - int oldtype = CANCEL_ASYNC ();
> + int oldtype = LIBC_CANCEL_ASYNC ();
>
> /* Get the lock the parent locked to force synchronization. */
> lll_lock (pd->lock, LLL_PRIVATE);
> @@ -363,7 +363,7 @@ START_THREAD_DEFN
> /* And give it up right away. */
> lll_unlock (pd->lock, LLL_PRIVATE);
>
> - CANCEL_RESET (oldtype);
> + LIBC_CANCEL_RESET (oldtype);
> }
>
> LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg);
Ok.
> diff --git a/rt/Makefile b/rt/Makefile
> index c1a0fdeb46..97c9bbd9de 100644
> --- a/rt/Makefile
> +++ b/rt/Makefile
> @@ -59,7 +59,6 @@ include ../Rules
> CFLAGS-aio_suspend.c += -fexceptions
> CFLAGS-mq_timedreceive.c += -fexceptions -fasynchronous-unwind-tables
> CFLAGS-mq_timedsend.c += -fexceptions -fasynchronous-unwind-tables
> -CFLAGS-librt-cancellation.c += -fasynchronous-unwind-tables
>
> LDFLAGS-rt.so = -Wl,--enable-new-dtags,-z,nodelete
>
Ok.
> diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile
> index adcced422b..632cd3686b 100644
> --- a/sysdeps/nptl/Makefile
> +++ b/sysdeps/nptl/Makefile
> @@ -21,8 +21,7 @@ libpthread-sysdep_routines += errno-loc
> endif
>
> ifeq ($(subdir),rt)
> -librt-sysdep_routines += timer_routines librt-cancellation
> -CFLAGS-librt-cancellation.c += -fexceptions -fasynchronous-unwind-tables
> +librt-sysdep_routines += timer_routines
>
> tests += tst-mqueue8x
> CFLAGS-tst-mqueue8x.c += -fexceptions
Ok.
> diff --git a/sysdeps/nptl/librt-cancellation.c b/sysdeps/nptl/librt-cancellation.c
> deleted file mode 100644
> index 1ad0eb11ff..0000000000
> --- a/sysdeps/nptl/librt-cancellation.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* Copyright (C) 2002-2021 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <nptl/pthreadP.h>
> -
> -
> -#define __pthread_enable_asynccancel __librt_enable_asynccancel
> -#define __pthread_disable_asynccancel __librt_disable_asynccancel
> -#include <nptl/cancellation.c>
Ok.
> diff --git a/sysdeps/nptl/lowlevellock-futex.h b/sysdeps/nptl/lowlevellock-futex.h
> index ca96397a4a..66ebfe50f4 100644
> --- a/sysdeps/nptl/lowlevellock-futex.h
> +++ b/sysdeps/nptl/lowlevellock-futex.h
> @@ -121,18 +121,18 @@
> /* Like lll_futex_wait, but acting as a cancellable entrypoint. */
> # define lll_futex_wait_cancel(futexp, val, private) \
> ({ \
> - int __oldtype = CANCEL_ASYNC (); \
> + int __oldtype = LIBC_CANCEL_ASYNC (); \
> long int __err = lll_futex_wait (futexp, val, LLL_SHARED); \
> - CANCEL_RESET (__oldtype); \
> + LIBC_CANCEL_RESET (__oldtype); \
> __err; \
> })
>
> /* Like lll_futex_timed_wait, but acting as a cancellable entrypoint. */
> # define lll_futex_timed_wait_cancel(futexp, val, timeout, private) \
> ({ \
> - int __oldtype = CANCEL_ASYNC (); \
> + int __oldtype = LIBC_CANCEL_ASYNC (); \
> long int __err = lll_futex_timed_wait (futexp, val, timeout, private); \
> - CANCEL_RESET (__oldtype); \
> + LIBC_CANCEL_RESET (__oldtype); \
> __err; \
> })
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/socketcall.h b/sysdeps/unix/sysv/linux/socketcall.h
> index 07702fc4f1..3084623216 100644
> --- a/sysdeps/unix/sysv/linux/socketcall.h
> +++ b/sysdeps/unix/sysv/linux/socketcall.h
> @@ -89,11 +89,6 @@
> })
>
>
> -#if IS_IN (libc)
> -# define __pthread_enable_asynccancel __libc_enable_asynccancel
> -# define __pthread_disable_asynccancel __libc_disable_asynccancel
> -#endif
> -
> #define SOCKETCALL_CANCEL(name, args...) \
> ({ \
> int oldtype = LIBC_CANCEL_ASYNC ();
Ok.
\
> diff --git a/sysdeps/unix/sysv/linux/sysdep-cancel.h b/sysdeps/unix/sysv/linux/sysdep-cancel.h
> index da2f08fde9..c64cfff37c 100644
> --- a/sysdeps/unix/sysv/linux/sysdep-cancel.h
> +++ b/sysdeps/unix/sysv/linux/sysdep-cancel.h
> @@ -24,44 +24,14 @@
> #include <tls.h>
> #include <errno.h>
>
> -/* The two functions are in libc.so and not exported. */
> -extern int __libc_enable_asynccancel (void) attribute_hidden;
> -extern void __libc_disable_asynccancel (int oldtype) attribute_hidden;
> -
> -/* The two functions are in librt.so and not exported. */
> -extern int __librt_enable_asynccancel (void) attribute_hidden;
> -extern void __librt_disable_asynccancel (int oldtype) attribute_hidden;
> -
> -/* The two functions are in libpthread.so and not exported. */
> -extern int __pthread_enable_asynccancel (void) attribute_hidden;
> -extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden;
> -
> /* Set cancellation mode to asynchronous. */
> -#define CANCEL_ASYNC() \
> - __pthread_enable_asynccancel ()
> -/* Reset to previous cancellation mode. */
> -#define CANCEL_RESET(oldtype) \
> - __pthread_disable_asynccancel (oldtype)
> -
> -#if IS_IN (libc)
> -/* Same as CANCEL_ASYNC, but for use in libc.so. */
> -# define LIBC_CANCEL_ASYNC() \
> - __libc_enable_asynccancel ()
> -/* Same as CANCEL_RESET, but for use in libc.so. */
> -# define LIBC_CANCEL_RESET(oldtype) \
> - __libc_disable_asynccancel (oldtype)
> -#elif IS_IN (libpthread)
> -# define LIBC_CANCEL_ASYNC() CANCEL_ASYNC ()
> -# define LIBC_CANCEL_RESET(val) CANCEL_RESET (val)
> -#elif IS_IN (librt)
> -# define LIBC_CANCEL_ASYNC() \
> - __librt_enable_asynccancel ()
> -# define LIBC_CANCEL_RESET(val) \
> - __librt_disable_asynccancel (val)
> -#else
> -# define LIBC_CANCEL_ASYNC() 0 /* Just a dummy value. */
> -# define LIBC_CANCEL_RESET(val) ((void)(val)) /* Nothing, but evaluate it. */
> -#endif
> +extern int __pthread_enable_asynccancel (void);
> +libc_hidden_proto (__pthread_enable_asynccancel)
> +#define LIBC_CANCEL_ASYNC() __pthread_enable_asynccancel ()
>
> +/* Reset to previous cancellation mode. */
> +extern void __pthread_disable_asynccancel (int oldtype);
> +libc_hidden_proto (__pthread_disable_asynccancel)
> +#define LIBC_CANCEL_RESET(oldtype) __pthread_disable_asynccancel (oldtype)
>
> #endif
>
Ok.
next prev parent reply other threads:[~2021-05-05 14:21 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-03 13:51 [PATCH 00/11] nptl: Move semaphore functions into libc Florian Weimer
2021-05-03 13:51 ` [PATCH 01/11] nptl: Move pthread_testcancel " Florian Weimer
2021-05-05 14:16 ` Adhemerval Zanella
2021-05-03 13:51 ` [PATCH 02/11] nptl: Consolidate aysnc cancel enable/disable implementation in libc Florian Weimer
2021-05-05 14:21 ` Adhemerval Zanella [this message]
2021-05-03 13:51 ` [PATCH 03/11] nptl: Move sem_clockwait into libc Florian Weimer
2021-05-05 14:30 ` Adhemerval Zanella
2021-05-03 13:51 ` [PATCH 04/11] nptl: Move sem_close, sem_open " Florian Weimer
2021-05-05 14:33 ` Adhemerval Zanella
2021-05-05 15:30 ` Florian Weimer
2021-05-03 13:51 ` [PATCH 05/11] nptl: Move sem_destroy " Florian Weimer
2021-05-05 14:35 ` Adhemerval Zanella
2021-05-03 13:51 ` [PATCH 06/11] nptl: Move sem_getvalue " Florian Weimer
2021-05-05 14:35 ` Adhemerval Zanella
2021-05-03 13:52 ` [PATCH 07/11] nptl: Move sem_init " Florian Weimer
2021-05-05 14:36 ` Adhemerval Zanella
2021-05-03 13:52 ` [PATCH 08/11] nptl: Move sem_post " Florian Weimer
2021-05-05 14:36 ` Adhemerval Zanella
2021-05-03 13:52 ` [PATCH 09/11] nptl: Move sem_timedwait " Florian Weimer
2021-05-05 14:37 ` Adhemerval Zanella
2021-05-03 13:52 ` [PATCH 10/11] nptl: Move sem_unlink " Florian Weimer
2021-05-05 14:37 ` Adhemerval Zanella
2021-05-03 13:52 ` [PATCH 11/11] nptl: Move sem_trywait, sem_wait " Florian Weimer
2021-05-05 14:38 ` 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=9f5da3d5-9d40-3615-1e07-376b2a346708@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--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).