From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) by sourceware.org (Postfix) with ESMTPS id 4E2983851C2F for ; Wed, 5 May 2021 14:21:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4E2983851C2F Received: by mail-qt1-x82f.google.com with SMTP id p6so1244486qtk.13 for ; Wed, 05 May 2021 07:21:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=tp5pVGHgMS7Xzkx4yZRNX1Kd07pDNPMKVVlnlWJQePE=; b=bawwvbT+IQn7wolP8p3IfkmUgeACysF+kb4af2+aeyeBzh/2Ziqpr7aQ9z1iJ/mhDn cVpJKRVjdR8yDFRPSY0/Sfl+aJw2T+prtSJY283zVHbeoc5TqExHVO2bzW4uXFuavH6n zYSwz/YFKMqN/LbUQTZe7i8GjXJAyoKXnxcVOGRpjeFKTdmElfwgDqQxLM00KqLR6NXZ Ik1mC/cMHfRkCaTy5SpPdJJmWS5/6nFNkyVVfmPj/hvH0LFcLHKrXWYGhcCkalUckX+e kynyDg80lrBj0QHaEHqsRcyFrjjfaOcsnStaNs7/J+ehs0EFVOhqEfSX4dabqcwcTji7 /jgA== X-Gm-Message-State: AOAM533RVX+dR2ni/x8Iuo1+VZk/eWAF3ii2E6POf5Y3+sxIWljE+O4J WfsEg2eLH2zJY67WDg0ZTIwlJtagow63JA== X-Google-Smtp-Source: ABdhPJzttwLvVcjLpbzfczaLQzPfL9coA9TqDdPGK0WPXFnXwr9Ll07/ztdkujnvLMv3LY5jb9AzSA== X-Received: by 2002:ac8:5655:: with SMTP id 21mr28798312qtt.187.1620224490515; Wed, 05 May 2021 07:21:30 -0700 (PDT) Received: from [192.168.1.4] ([177.194.37.86]) by smtp.gmail.com with ESMTPSA id a20sm13609093qko.36.2021.05.05.07.21.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 05 May 2021 07:21:30 -0700 (PDT) Subject: Re: [PATCH 02/11] nptl: Consolidate aysnc cancel enable/disable implementation in libc To: Florian Weimer , libc-alpha@sourceware.org References: From: Adhemerval Zanella Message-ID: <9f5da3d5-9d40-3615-1e07-376b2a346708@linaro.org> Date: Wed, 5 May 2021 11:21:27 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK 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, 05 May 2021 14:21:33 -0000 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 > --- > 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 , 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 > - . */ > - > -#include "pthreadP.h" > - > - > -#define __pthread_enable_asynccancel __libc_enable_asynccancel > -#define __pthread_disable_asynccancel __libc_disable_asynccancel > -#include 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 , 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 > - . */ > - > -#include > - > - > -#define __pthread_enable_asynccancel __librt_enable_asynccancel > -#define __pthread_disable_asynccancel __librt_disable_asynccancel > -#include 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 > #include > > -/* 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.