From patchwork Wed Oct 18 18:23:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 23675 Received: (qmail 29860 invoked by alias); 18 Oct 2017 18:24:04 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 27453 invoked by uid 89); 18 Oct 2017 18:24:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=mas, transactions, Per, stefan X-HELO: mail-qk0-f181.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=WqIS4oC+rHeRZv/rJsFN4JG2NXt6XNYRRDyP5mCuM54=; b=CB6cq4FppUQOTkCSyffOnDBBs9L4IZYj+cNQRWf4S+9QNWrZo0URE5e6B2WRpmnVbu OzjC3YUJxGeEEVhwIbLDovmmcETw4kdnMfYr17vVjgVUITG8vON5npm8fgY6JxwTSMiz 8668Fcz3ooQhHnjGqzAARkmhj+XYd8fZ31uWyK8Ek5YWcZF9DFoP+L0ctrkvH9ukRWPa RTDc/kWbGB4lP/qRZifWjtyy45towvs8Yoe4VpMQI2IMKv2Lv25eiBe1CLE4oY3hQTu9 e2H/uNEzZwRgXNg5GBEcYIl/H4izzioKRZKYEt/Ib+616P3gh2S3TQpl9L1GbdIv6Ue/ Mr6A== X-Gm-Message-State: AMCzsaXLNfZMf61YKd5yjRwW/jEFkYOqrWEGuXZiVrPkOqcShSVulX/m Vigc2Mgdi5xYjvVvgSI99TYaYg== X-Google-Smtp-Source: ABhQp+THVIGouG/RRfemhs3jADqmanSE/HgJ2lQT+5YZ/QXx49lydhf4kAkg1OL1Y9fnJFR8vmhgvg== X-Received: by 10.55.169.87 with SMTP id s84mr3424965qke.305.1508351032760; Wed, 18 Oct 2017 11:23:52 -0700 (PDT) Subject: Re: [RFC][PATCH] tunables: Add elision tunable To: rcardoso , libc-alpha@sourceware.org Cc: stli@linux.vnet.ibm.com References: <68de6583-4f5c-5fa4-9328-1682e614ef56@linux.vnet.ibm.com> From: Carlos O'Donell Message-ID: <77049f69-b72c-93aa-ca53-0f3c10aa5a69@redhat.com> Date: Wed, 18 Oct 2017 11:23:48 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <68de6583-4f5c-5fa4-9328-1682e614ef56@linux.vnet.ibm.com> On 09/14/2017 07:30 AM, rcardoso wrote: > From 3306d4c07dc673fde097420f0528d2e60c2a0d65 Mon Sep 17 00:00:00 2001 > From: Rogerio Alves > Date: Thu, 6 Jul 2017 13:21:08 -0500 > Subject: [PATCH v3] Add elision tunables. > > This patch adds several new tunables to control the behavior of > elision on supported platforms. This also disables elision > by default on powerpc. > > This patch was initially proposed by Paul Murphy[1] but was > "staled" because the framework have changed since the patch was > originally proposed. > > [1] https://patchwork.sourceware.org/patch/10342/ > > 2017-06-06 Rogerio A. Cardoso , > Paul E. Murphy > > * elf/dl-tunables.list: Add elision parameters. > * manual/tunables.texi: Add entries about elision tunable. > * sysdeps/unix/sysv/linux/powerpc/elision-conf.c: > Add callback functions to dynamically enable/disable elision. > Add multiple callbacks functions to set elision parameters. > * sysdeps/unix/sysv/linux/s390/elision-conf.c: Likewise. > * sysdeps/unix/sysv/linux/x86/elision-conf.c: Likewise. This is not ready. Since elision now depends on tunables, we should always *compile* with elision enabled, and leave the code disabled, but available for runtime selection. This gives us *much* better compile-time testing of the existing code to avoid bit-rot. OK with the following changes. (a) Remove the configure checks for elision. > --- > Changes in v3: Per Stefan Liebler review. Change define order of > TUNABLE_NAMESPACE. > > Changes in v2: Per Stefan Liebler review. Add missing `end dftp` to > manual. Change #ifdef to #if HAVE_TUNABLE. > > elf/dl-tunables.list | 34 ++++++++++++ > manual/tunables.texi | 65 +++++++++++++++++++++++ > sysdeps/unix/sysv/linux/powerpc/elision-conf.c | 73 +++++++++++++++++++++++--- > sysdeps/unix/sysv/linux/s390/elision-conf.c | 70 ++++++++++++++++++++++-- > sysdeps/unix/sysv/linux/x86/elision-conf.c | 72 ++++++++++++++++++++++--- > 5 files changed, 297 insertions(+), 17 deletions(-) > > diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list > index c188c6a..77dfcb4 100644 > --- a/elf/dl-tunables.list > +++ b/elf/dl-tunables.list > @@ -96,4 +96,38 @@ glibc { > default: HWCAP_IMPORTANT > } > } > + > + elision { > + enable { > + type: INT_32 > + minval: 0 > + maxval: 1 > + security_level: SXID_IGNORE OK. > + } > + skip_lock_busy { > + type: INT_32 > + default: 3 > + security_level: SXID_IGNORE > + } > + skip_lock_internal_abort { > + type: INT_32 > + default: 3 > + security_level: SXID_IGNORE > + } > + skip_lock_after_retries { > + type: INT_32 > + default: 3 > + security_level: SXID_IGNORE > + } > + tries { > + type: INT_32 > + default: 3 > + security_level: SXID_IGNORE > + } > + skip_trylock_internal_abort { > + type: INT_32 > + default: 3 > + security_level: SXID_IGNORE OK. > + } > + } > } > diff --git a/manual/tunables.texi b/manual/tunables.texi > index 3c19567..5f660e0 100644 > --- a/manual/tunables.texi > +++ b/manual/tunables.texi > @@ -31,6 +31,7 @@ their own namespace. > @menu > * Tunable names:: The structure of a tunable name > * Memory Allocation Tunables:: Tunables in the memory allocation subsystem > +* Elision Tunables:: Tunables in elision subsystem OK. > * Hardware Capability Tunables:: Tunables that modify the hardware > capabilities seen by @theglibc{} > @end menu > @@ -226,6 +227,70 @@ pre-fill the per-thread cache with. The default, or when set to zero, > is no limit. > @end deftp > > +@node Elision Tunables > +@section Elision Tunables > +@cindex elision tunables > +@cindex tunables, elision > + > +@deftp {Tunable namespace} glibc.elision > +Elision behavior can be modified by setting any of the following tunables in > +the @code{elision} namespace: > +@end deftp > + > +@deftp Tunable glibc.elision.enable > +The @code{glibc.elision.enable} tunable enables lock elision if the feature is > +supported by the hardware. If elision is not supported by the hardware this > +tunable has no effect. OK. > + > +Elision tunables are supported for x86-64, PowerPC, and S390 architectures. > +@end deftp > + > +@deftp Tunable glibc.elision.skip_lock_busy > +The @code{glibc.elision.skip_lock_busy} tunable sets how many times to use a > +non-transactional lock after a transactional failure has occurred because the > +lock is already acquired. Expressed in number of lock acquisition attempts. OK. > + > +The default value of this tunable is @samp{3}. > +@end deftp > + > +@deftp Tunable glibc.elision.skip_lock_internal_abort > +The @code{glibc.elision.skip_lock_internal_abort} tunable sets how many times > +to not attempt to use elision if a transaction aborted due to reasons other > +than other threads' memory accesses. Expressed in number of lock acquisition Awkward wording, suggest: The @code{glibc.elision.skip_lock_internal_abort} tunable sets how many times the thread should avoid using elision if a transaction aborted for any reason other than a different thread's memory accesses. Expressed in number of lock acquisition > +attempts. OK. > + > +The default value of this tunable is @samp{3}. > +@end deftp > + > +@deftp Tunable glibc.elision.skip_lock_after_retries > +The @code{glibc.elision.skip_lock_after_retries} tunable sets how my times to > +try to elide a lock with transactions that only fail due other threads' memory s/other thread's/to a differrent thread's/g > +accesses, before falling back to regular lock. > +Expressed in number of lock elision attempts. > + > +This tunable is not supported by x86. > + > +The default value of this tunable is @samp{3}. > +@end deftp > + > +@deftp Tunable glibc.elision.tries > +The @code{glibc.elision.tries} sets how many times we retry using elision if > +there is chance for the transaction to finish execution (e.g., it wasn't aborted > +due to the lock being already acquired). This tunable is set to @samp{0} if > +elision is not supported by the hardware to avoid retries. > + Awkward wording, suggest: ... If elision is not supported by the hardware this tunable is set to @samp{0} to avoid retries. > +The default value of this tunable is @samp{3}. > +@end deftp > + > +@deftp Tunable glibc.elision.skip_trylock_internal_abort > +The @code{glibc.elision.skip_trylock_internal_abort} tunable sets how many times > +to not attempt to use elision for trylocks if a transaction aborted due to > +reasons other than other threads' memory accesses. Expressed in number of try > +lock attempts. > + Awkward wording, suggest: The @code{glibc.elision.skip_trylock_internal_abort} tunable sets how many times the thread should avoid using elision for trylocks if a transaction aborted for any reason other than a different thread's memory accesses. Expressed in number of lock acquisition > +The default value of this tunable is @samp{3}. > +@end deftp > + > @node Hardware Capability Tunables > @section Hardware Capability Tunables > @cindex hardware capability tunables > diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c > index f631f0a..4fd7741 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c > +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c > @@ -22,6 +22,11 @@ > #include > #include > > +#if HAVE_TUNABLES > +# define TUNABLE_NAMESPACE elision > +#endif > +#include > + > /* Reasonable initial tuning values, may be revised in the future. > This is a conservative initial value. */ > > @@ -50,7 +55,50 @@ struct elision_config __elision_aconf = > DEFAULT locks should be automatically use elision in pthread_mutex_lock(). > Disabled for suid programs. Only used when elision is available. */ > > -int __pthread_force_elision attribute_hidden; > +int __pthread_force_elision attribute_hidden = 0; > + > +#if HAVE_TUNABLES > +static inline void > +__always_inline > +do_set_elision_enable (int32_t elision_enable) > +{ > + /* Enable elision if it's avaliable in hardware. */ > + if (elision_enable && !__libc_enable_secure) > + __pthread_force_elision = (GLRO (dl_hwcap2) > + & PPC_FEATURE2_HAS_HTM) ? 1 : 0; No boolean coercion please. Either make elision_enable a bool, and call do_set_elision_enable(true/false). Or if (elision_enable == 1 && !__libc_enable_secure) Note: That we treat __libc_enable_secure *as-if* it were bool and we need to fix that if you feel like sending another patch... that's a historical mistake. > +} > + > +/* The pthread->elision_enable tunable is 0 or 1 indicating that elision > + should be disabled or enabled respectively. The feature will only be used > + if it's supported by the hardware. */ > + > +void > +TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp) > +{ > + int32_t elision_enable = (int32_t) valp->numval; > + do_set_elision_enable (elision_enable); > +} > + > +#define TUNABLE_CALLBACK_FNDECL(__name, __type) \ > +static inline void \ > +__always_inline \ > +do_set_elision_ ## __name (__type value) \ > +{ \ > + __elision_aconf.__name = value; \ > +} \ > +void \ > +TUNABLE_CALLBACK (set_elision_ ## __name) (tunable_val_t *valp) \ > +{ \ > + __type value = (__type) (valp)->numval; \ > + do_set_elision_ ## __name (value); \ > +} > + > +TUNABLE_CALLBACK_FNDECL (skip_lock_busy, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_lock_internal_abort, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_lock_out_of_tbegin_retries, int32_t); > +TUNABLE_CALLBACK_FNDECL (try_tbegin, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_trylock_internal_abort, int32_t); > +#endif > > /* Initialize elision. */ > > @@ -59,13 +107,26 @@ elision_init (int argc __attribute__ ((unused)), > char **argv __attribute__ ((unused)), > char **environ) > { > -#ifdef ENABLE_LOCK_ELISION > - int elision_available = (GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM) ? 1 : 0; > - __pthread_force_elision = __libc_enable_secure ? 0 : elision_available; > +#if HAVE_TUNABLES > + /* Elision depends on tunables and must be explicitly turned on by setting > + the appropriate tunable on a supported platform. */ > + > + TUNABLE_GET (enable, int32_t, > + TUNABLE_CALLBACK (set_elision_enable)); > + TUNABLE_GET (skip_lock_busy, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_busy)); > + TUNABLE_GET (skip_lock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_internal_abort)); > + TUNABLE_GET (skip_lock_after_retries, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_out_of_tbegin_retries)); > + TUNABLE_GET (tries, int32_t, > + TUNABLE_CALLBACK (set_elision_try_tbegin)); > + TUNABLE_GET (skip_trylock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_trylock_internal_abort)); > #endif > + > if (!__pthread_force_elision) > - /* Disable elision on rwlocks. */ > - __elision_aconf.try_tbegin = 0; > + __elision_aconf.try_tbegin = 0; /* Disable elision on rwlocks. */ OK. > } > > #ifdef SHARED > diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c > index cc0fdef..6a57269 100644 > --- a/sysdeps/unix/sysv/linux/s390/elision-conf.c > +++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c > @@ -22,6 +22,11 @@ > #include > #include > > +#if HAVE_TUNABLES > +# define TUNABLE_NAMESPACE elision > +#endif > +#include > + > /* Reasonable initial tuning values, may be revised in the future. > This is a conservative initial value. */ > > @@ -53,6 +58,48 @@ struct elision_config __elision_aconf = > > int __pthread_force_elision attribute_hidden = 0; > > +#if HAVE_TUNABLES > +static inline void > +__always_inline > +do_set_elision_enable (int32_t elision_enable) > +{ > + /* Enable elision if it's avaliable in hardware. */ > + if (elision_enable && !__libc_enable_secure) No boolean coercion please. Same issue as above. > + __pthread_force_elision = (GLRO (dl_hwcap) & HWCAP_S390_TE) ? 1 : 0; > +} > + > +/* The pthread->elision_enable tunable is 0 or 1 indicating that elision > + should be disabled or enabled respectively. The feature will only be used > + if it's supported by the hardware. */ > + > +void > +TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp) > +{ > + int32_t elision_enable = (int32_t) valp->numval; > + do_set_elision_enable (elision_enable); > +} > + > +#define TUNABLE_CALLBACK_FNDECL(__name, __type) \ > +static inline void \ > +__always_inline \ > +do_set_elision_ ## __name (__type value) \ > +{ \ > + __elision_aconf.__name = value; \ > +} \ > +void \ > +TUNABLE_CALLBACK (set_elision_ ## __name) (tunable_val_t *valp) \ > +{ \ > + __type value = (__type) (valp)->numval; \ > + do_set_elision_ ## __name (value); \ > +} > + > +TUNABLE_CALLBACK_FNDECL (skip_lock_busy, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_lock_internal_abort, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_lock_out_of_tbegin_retries, int32_t); > +TUNABLE_CALLBACK_FNDECL (try_tbegin, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_trylock_internal_abort, int32_t); > +#endif > + OK. > /* Initialize elison. */ > > static void > @@ -60,11 +107,26 @@ elision_init (int argc __attribute__ ((unused)), > char **argv __attribute__ ((unused)), > char **environ) > { > - /* Set when the CPU and the kernel supports transactional execution. > - When false elision is never attempted. */ > - int elision_available = (GLRO (dl_hwcap) & HWCAP_S390_TE) ? 1 : 0; > +#if HAVE_TUNABLES > + /* Elision depends on tunables and must be explicitly turned on by setting > + the appropriate tunable on a supported platform. */ > + > + TUNABLE_GET (enable, int32_t, > + TUNABLE_CALLBACK (set_elision_enable)); > + TUNABLE_GET (skip_lock_busy, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_busy)); > + TUNABLE_GET (skip_lock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_internal_abort)); > + TUNABLE_GET (skip_lock_after_retries, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_out_of_tbegin_retries)); > + TUNABLE_GET (tries, int32_t, > + TUNABLE_CALLBACK (set_elision_try_tbegin)); > + TUNABLE_GET (skip_trylock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_trylock_internal_abort)); > +#endif > > - __pthread_force_elision = __libc_enable_secure ? 0 : elision_available; > + if (!__pthread_force_elision) > + __elision_aconf.try_tbegin = 0; /* Disable elision on rwlocks. */ OK. Without the tunable set __pthread_force_elision is 0 by default and elision is disabled. Matches closely the code I wrote. > } > > #ifdef SHARED > diff --git a/sysdeps/unix/sysv/linux/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c > index 673b000..89a4a8e 100644 > --- a/sysdeps/unix/sysv/linux/x86/elision-conf.c > +++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c > @@ -22,6 +22,11 @@ > #include > #include > > +#if HAVE_TUNABLES > +# define TUNABLE_NAMESPACE elision > +#endif > +#include > + > /* Reasonable initial tuning values, may be revised in the future. > This is a conservative initial value. */ > > @@ -48,21 +53,74 @@ struct elision_config __elision_aconf = > pthread_mutex_lock(). Disabled for suid programs. Only used when elision > is available. */ > > -int __pthread_force_elision attribute_hidden; > +int __pthread_force_elision attribute_hidden = 0; > + > +#if HAVE_TUNABLES > +static inline void > +__always_inline > +do_set_elision_enable (int32_t elision_enable) > +{ > + /* Enable elision if it's avaliable in hardware. */ > + if (elision_enable && !__libc_enable_secure) No boolean coercion. Same issue as above. > + __pthread_force_elision = HAS_CPU_FEATURE (RTM) ? 1 : 0; > +} > + > +/* The pthread->elision_enable tunable is 0 or 1 indicating that elision > + should be disabled or enabled respectively. The feature will only be used > + if it's supported by the hardware. */ > > -/* Initialize elison. */ > +void > +TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp) > +{ > + int32_t elision_enable = (int32_t) valp->numval; > + do_set_elision_enable (elision_enable); > +} > + > +#define TUNABLE_CALLBACK_FNDECL(__name, __type) \ > +static inline void \ > +__always_inline \ > +do_set_elision_ ## __name (__type value) \ > +{ \ > + __elision_aconf.__name = value; \ > +} \ > +void \ > +TUNABLE_CALLBACK (set_elision_ ## __name) (tunable_val_t *valp) \ > +{ \ > + __type value = (__type) (valp)->numval; \ > + do_set_elision_ ## __name (value); \ > +} > + > +TUNABLE_CALLBACK_FNDECL (skip_lock_busy, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_lock_internal_abort, int32_t); > +TUNABLE_CALLBACK_FNDECL (retry_try_xbegin, int32_t); > +TUNABLE_CALLBACK_FNDECL (skip_trylock_internal_abort, int32_t); > +#endif > + > +/* Initialize elision. */ > > static void > elision_init (int argc __attribute__ ((unused)), > char **argv __attribute__ ((unused)), > char **environ) > { > - int elision_available = HAS_CPU_FEATURE (RTM); > -#ifdef ENABLE_LOCK_ELISION > - __pthread_force_elision = __libc_enable_secure ? 0 : elision_available; > +#if HAVE_TUNABLES > + /* Elision depends on tunables and must be explicitly turned on by setting > + the appropriate tunable on a supported platform. */ > + > + TUNABLE_GET (enable, int32_t, > + TUNABLE_CALLBACK (set_elision_enable)); > + TUNABLE_GET (skip_lock_busy, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_busy)); > + TUNABLE_GET (skip_lock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_lock_internal_abort)); > + TUNABLE_GET (tries, int32_t, > + TUNABLE_CALLBACK (set_elision_retry_try_xbegin)); > + TUNABLE_GET (skip_trylock_internal_abort, int32_t, > + TUNABLE_CALLBACK (set_elision_skip_trylock_internal_abort)); > #endif > - if (!elision_available) > - __elision_aconf.retry_try_xbegin = 0; /* Disable elision on rwlocks */ > + > + if (!__pthread_force_elision) > + __elision_aconf.retry_try_xbegin = 0; /* Disable elision on rwlocks. */ > } OK. > > #ifdef SHARED > -- 2.7.4 diff --git a/configure.ac b/configure.ac index 4b83ae5..3252cdd 100644 --- a/configure.ac +++ b/configure.ac @@ -199,16 +199,6 @@ if test "$enable_stackguard_randomize" = yes; then AC_DEFINE(ENABLE_STACKGUARD_RANDOMIZE) fi -AC_ARG_ENABLE([lock-elision], - AC_HELP_STRING([--enable-lock-elision[=yes/no]], - [Enable lock elision for pthread mutexes by default]), - [enable_lock_elision=$enableval], - [enable_lock_elision=no]) -AC_SUBST(enable_lock_elision) -if test "$enable_lock_elision" = yes ; then - AC_DEFINE(ENABLE_LOCK_ELISION) -fi - dnl Generic infrastructure for drop-in additions to libc. AC_ARG_ENABLE([add-ons], AC_HELP_STRING([--enable-add-ons@<:@=DIRS...@:>@], Regenerate configure. (b) Remove config.h.in ENABLE_LOCK_ELISION. (c) Remove elision configure option from manual/install.texi and regenerate INSTALL. (d) In nptl/Makefile disable elision for tst-mutex8 so we can test the error case. diff --git a/nptl/Makefile b/nptl/Makefile index d819349..2b2754e 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -714,6 +714,9 @@ $(objpfx)tst-oddstacklimit.out: $(objpfx)tst-oddstacklimit $(objpfx)tst-basic 1 endif $(objpfx)tst-compat-forwarder: $(objpfx)tst-compat-forwarder-mod.so +# Disable elision for tst-mutex8 so it can verify error case for +# destroying a mutex. +tst-mutex8-ENV = GLIBC_TUNABLES=glibc.elision.enable=0 # The tests here better do not run in parallel ifneq ($(filter %tests,$(MAKECMDGOALS)),) (e) Fix tst-mutex8 diff --git a/nptl/tst-mutex8.c b/nptl/tst-mutex8.c index 1d288d2..ef59db5 100644 --- a/nptl/tst-mutex8.c +++ b/nptl/tst-mutex8.c @@ -127,9 +127,8 @@ check_type (const char *mas, pthread_mutexattr_t *ma) return 1; } - /* Elided mutexes don't fail destroy. If elision is not explicitly disabled - we don't know, so can also not check this. */ -#ifndef ENABLE_LOCK_ELISION + /* Elided mutexes don't fail destroy, but this test is run with + elision disabled so we can test them. */ e = pthread_mutex_destroy (m); if (e == 0) { @@ -142,7 +141,6 @@ check_type (const char *mas, pthread_mutexattr_t *ma) mas); return 1; } -#endif if (pthread_mutex_unlock (m) != 0) { @@ -157,7 +155,6 @@ check_type (const char *mas, pthread_mutexattr_t *ma) } /* Elided mutexes don't fail destroy. */ -#ifndef ENABLE_LOCK_ELISION e = pthread_mutex_destroy (m); if (e == 0) { @@ -171,7 +168,6 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n", mas); return 1; } -#endif if (pthread_mutex_unlock (m) != 0) { @@ -207,7 +203,6 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n", } /* Elided mutexes don't fail destroy. */ -#ifndef ENABLE_LOCK_ELISION e = pthread_mutex_destroy (m); if (e == 0) { @@ -220,7 +215,6 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n", mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas); return 1; } -#endif done = true; if (pthread_cond_signal (&c) != 0) @@ -280,7 +274,6 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas); } /* Elided mutexes don't fail destroy. */ -#ifndef ENABLE_LOCK_ELISION e = pthread_mutex_destroy (m); if (e == 0) { @@ -295,7 +288,6 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas); mas); return 1; } -#endif if (pthread_cancel (th) != 0) { (f) Cleanup elide.h, sysdep.h, and force-elision.h diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h index 1c42814..06986cc 100644 --- a/sysdeps/powerpc/nptl/elide.h +++ b/sysdeps/powerpc/nptl/elide.h @@ -19,7 +19,6 @@ #ifndef ELIDE_PPC_H # define ELIDE_PPC_H -#ifdef ENABLE_LOCK_ELISION # include # include @@ -114,12 +113,4 @@ __elide_unlock (int is_lock_free) # define ELIDE_UNLOCK(is_lock_free) \ __elide_unlock (is_lock_free) -# else - -# define ELIDE_LOCK(adapt_count, is_lock_free) 0 -# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) 0 -# define ELIDE_UNLOCK(is_lock_free) 0 - -#endif /* ENABLE_LOCK_ELISION */ - #endif diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h index 965ea43..1d2ff73 100644 --- a/sysdeps/powerpc/powerpc32/sysdep.h +++ b/sysdeps/powerpc/powerpc32/sysdep.h @@ -90,7 +90,7 @@ GOT_LABEL: ; \ cfi_endproc; \ ASM_SIZE_DIRECTIVE(name) -#if ! IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) +#if ! IS_IN(rtld) # define ABORT_TRANSACTION \ cmpwi 2,0; \ beq 1f; \ diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h index ab5f395..bff184e 100644 --- a/sysdeps/powerpc/powerpc64/sysdep.h +++ b/sysdeps/powerpc/powerpc64/sysdep.h @@ -263,7 +263,7 @@ LT_LABELSUFFIX(name,_name_end): ; \ TRACEBACK_MASK(name,mask); \ END_2(name) -#if !IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) +#if !IS_IN(rtld) # define ABORT_TRANSACTION \ cmpdi 13,0; \ beq 1f; \ diff --git a/sysdeps/powerpc/sysdep.h b/sysdeps/powerpc/sysdep.h index f07b959..d1a9bd9 100644 --- a/sysdeps/powerpc/sysdep.h +++ b/sysdeps/powerpc/sysdep.h @@ -21,10 +21,8 @@ */ #define _SYSDEPS_SYSDEP_H 1 #include -#ifdef ENABLE_LOCK_ELISION #include #include -#endif #define PPC_FEATURE_970 (PPC_FEATURE_POWER4 + PPC_FEATURE_HAS_ALTIVEC) @@ -176,7 +174,7 @@ we abort transaction just before syscalls. [1] Documentation/powerpc/transactional_memory.txt [Syscalls] */ -#if !IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) +#if !IS_IN(rtld) # define ABORT_TRANSACTION \ ({ \ if (THREAD_GET_TM_CAPABLE ()) \ diff --git a/sysdeps/unix/sysv/linux/powerpc/force-elision.h b/sysdeps/unix/sysv/linux/powerpc/force-elision.h index 318f791..d1feeeb 100644 --- a/sysdeps/unix/sysv/linux/powerpc/force-elision.h +++ b/sysdeps/unix/sysv/linux/powerpc/force-elision.h @@ -16,7 +16,6 @@ License along with the GNU C Library; if not, see . */ -#ifdef ENABLE_LOCK_ELISION /* Automatically enable elision for existing user lock kinds. */ #define FORCE_ELISION(m, s) \ if (__pthread_force_elision \ @@ -25,4 +24,3 @@ mutex->__data.__kind |= PTHREAD_MUTEX_ELISION_NP; \ s; \ } -#endif diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.h b/sysdeps/unix/sysv/linux/s390/elision-conf.h index 3143f3b..32f0ed3 100644 --- a/sysdeps/unix/sysv/linux/s390/elision-conf.h +++ b/sysdeps/unix/sysv/linux/s390/elision-conf.h @@ -15,7 +15,6 @@ You should have received a copy of the GNU Lesser General Public License along with the GNU C Library; if not, see . */ -#ifdef ENABLE_LOCK_ELISION #ifndef _ELISION_CONF_H #define _ELISION_CONF_H 1 @@ -41,4 +40,3 @@ extern int __pthread_force_elision attribute_hidden; #define HAVE_ELISION 1 #endif -#endif diff --git a/sysdeps/unix/sysv/linux/s390/force-elision.h b/sysdeps/unix/sysv/linux/s390/force-elision.h index 3ae3bcd..8e1e33e 100644 --- a/sysdeps/unix/sysv/linux/s390/force-elision.h +++ b/sysdeps/unix/sysv/linux/s390/force-elision.h @@ -16,7 +16,6 @@ License along with the GNU C Library; if not, see . */ -#ifdef ENABLE_LOCK_ELISION /* Automatically enable elision for existing user lock kinds. */ #define FORCE_ELISION(m, s) \ if (__pthread_force_elision \ @@ -25,4 +24,3 @@ mutex->__data.__kind |= PTHREAD_MUTEX_ELISION_NP; \ s; \ } -#endif diff --git a/sysdeps/unix/sysv/linux/s390/lowlevellock.h b/sysdeps/unix/sysv/linux/s390/lowlevellock.h index 604137f..48f87a8 100644 --- a/sysdeps/unix/sysv/linux/s390/lowlevellock.h +++ b/sysdeps/unix/sysv/linux/s390/lowlevellock.h @@ -22,7 +22,6 @@ #include /* Transactional lock elision definitions. */ -# ifdef ENABLE_LOCK_ELISION extern int __lll_timedlock_elision (int *futex, short *adapt_count, const struct timespec *timeout, int private) attribute_hidden; @@ -45,6 +44,5 @@ extern int __lll_trylock_elision(int *futex, short *adapt_count) __lll_unlock_elision (&(futex), &(adapt_count), private) # define lll_trylock_elision(futex, adapt_count) \ __lll_trylock_elision(&(futex), &(adapt_count)) -# endif /* ENABLE_LOCK_ELISION */ #endif /* lowlevellock.h */