From patchwork Thu Nov 16 18:44:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: rcardoso@linux.vnet.ibm.com X-Patchwork-Id: 24295 Received: (qmail 118254 invoked by alias); 16 Nov 2017 18:44:43 -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 117847 invoked by uid 89); 16 Nov 2017 18:44:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=HTo:U*carlos X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [RFC][PATCH] tunables: Add elision tunable To: "Carlos O'Donell" , libc-alpha@sourceware.org References: <68de6583-4f5c-5fa4-9328-1682e614ef56@linux.vnet.ibm.com> <581813ef-53f8-ba33-4026-21e8970d0155@linux.vnet.ibm.com> From: Rogerio Alves Date: Thu, 16 Nov 2017 16:44:24 -0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: X-TM-AS-GCONF: 00 x-cbid: 17111618-0044-0000-0000-000003B13334 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008076; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000240; SDB=6.00946810; UDB=6.00477961; IPR=6.00727102; BA=6.00005695; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00018049; XFM=3.00000015; UTC=2017-11-16 18:44:28 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17111618-0045-0000-0000-000007E05448 Message-Id: <82fe9665-1329-98c1-7a15-c1b953bda91d@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-11-16_06:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=27 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711160249 Hi Carlos, All documentation suggestions was made on patch v5. Thanks for the review. Regards Rogerio. Em 16-11-2017 16:21, Carlos O'Donell escreveu: > On 11/16/2017 04:56 AM, rcardoso wrote: >> From 8b5efb9bb4f7f5b8c7188c092cbaa3c0eeed293c Mon Sep 17 00:00:00 2001 >> From: Rogerio Alves >> Date: Thu, 6 Jul 2017 13:21:08 -0500 >> Subject: [PATCH v4] Add elision tunables. > > This version looks great. It looks like you have addressed all the other > concerns I had. It looks like you addressed all of Siddhesh's concerns > also. > > OK to commit with documentation fixes. > >> This patch adds several new tunables to control the behavior of >> elision on supported platforms[1]. 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[2]. >> >> [1] This part of the patch was initially proposed by >> Paul Murphy but was "staled" because the framework have changed >> since the patch was originally proposed: >> >> https://patchwork.sourceware.org/patch/10342/ >> >> [2] This part of the patch was inititally proposed as a RFC by >> Carlos O'Donnell. Make sense to me integrate this on the patch: >> >> https://sourceware.org/ml/libc-alpha/2017-05/msg00335.html >> >> 2017-06-06 Rogerio A. Cardoso , >> Paul E. Murphy , >> Carlos O'Donnell >> >> * 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. >> Deleted __libc_enable_secure check. >> * sysdeps/unix/sysv/linux/s390/elision-conf.c: Likewise. >> * sysdeps/unix/sysv/linux/x86/elision-conf.c: Likewise. >> * configure.ac: Option enable_lock_elision was deleted. >> * config.h.in: ENABLE_LOCK_ELISION flag was deleted. >> * manual/install.texi: Elision configure option was removed. >> * INSTALL: Regenerated to remove enable_lock_elision. >> * nptl/Makefile: >> Disable elision so it can verify error case for destroying a mutex. >> * sysdeps/powerpc/nptl/elide.h: >> Cleanup ENABLE_LOCK_ELISION check. >> Deleted macros for the case when ENABLE_LOCK_ELISION was not defined. >> * nptl/tst-mutex8.c: >> Deleted all #ifndef ENABLE_LOCK_ELISION from the test. >> * sysdeps/powerpc/powerpc32/sysdep.h: >> Deleted all ENABLE_LOCK_ELISION checks. >> * sysdeps/powerpc/powerpc64/sysdep.h: Likewise. >> * sysdeps/powerpc/sysdep.h: Likewise. >> * sysdeps/unix/sysv/linux/powerpc/force-elision.h: Likewise. >> * sysdeps/unix/sysv/linux/s390/elision-conf.h: Likewise. >> * sysdeps/unix/sysv/linux/s390/force-elision.h: Likewise. >> * sysdeps/unix/sysv/linux/s390/lowlevellock.h: Likewise. >> --- >> Changes in v4: Per Carlos O'Donnell and Siddhesh Poyarekar reviews. Change >> SXID_IGNORE to SXID_ERASE. Remove libc secure check on elision. Improved >> documentation. Always build with elision support. Elision disabled by default >> at runtime. >> >> 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. >> >> INSTALL | 3 - >> config.h.in | 3 - >> configure.ac | 10 ---- >> elf/dl-tunables.list | 34 +++++++++++ >> manual/install.texi | 3 - >> manual/tunables.texi | 68 ++++++++++++++++++++++ >> nptl/Makefile | 4 ++ >> nptl/tst-mutex8.c | 12 +--- >> sysdeps/powerpc/nptl/elide.h | 9 --- >> sysdeps/powerpc/powerpc32/sysdep.h | 2 +- >> sysdeps/powerpc/powerpc64/sysdep.h | 2 +- >> sysdeps/powerpc/sysdep.h | 4 +- >> sysdeps/unix/sysv/linux/powerpc/elision-conf.c | 75 +++++++++++++++++++++++-- >> sysdeps/unix/sysv/linux/powerpc/force-elision.h | 2 - >> sysdeps/unix/sysv/linux/s390/elision-conf.c | 72 ++++++++++++++++++++++-- >> sysdeps/unix/sysv/linux/s390/elision-conf.h | 2 - >> sysdeps/unix/sysv/linux/s390/force-elision.h | 2 - >> sysdeps/unix/sysv/linux/s390/lowlevellock.h | 2 - >> sysdeps/unix/sysv/linux/x86/elision-conf.c | 74 +++++++++++++++++++++--- >> 19 files changed, 315 insertions(+), 68 deletions(-) >> >> diff --git a/INSTALL b/INSTALL >> index bc972b2..76cf167 100644 >> --- a/INSTALL >> +++ b/INSTALL >> @@ -115,9 +115,6 @@ will be used, and CFLAGS sets optimization options for the compiler. >> formats may change over time. Consult the 'timezone' subdirectory >> for more details. >> >> -'--enable-lock-elision=yes' >> - Enable lock elision for pthread mutexes by default. >> - > > OK. > >> '--enable-stack-protector' >> '--enable-stack-protector=strong' >> '--enable-stack-protector=all' >> diff --git a/config.h.in b/config.h.in >> index c140ff3..5622de8 100644 >> --- a/config.h.in >> +++ b/config.h.in >> @@ -134,9 +134,6 @@ >> /* Define if __stack_chk_guard canary should be randomized at program startup. */ >> #undef ENABLE_STACKGUARD_RANDOMIZE >> >> -/* Define if lock elision should be enabled by default. */ >> -#undef ENABLE_LOCK_ELISION >> - > > OK. > >> /* Package description. */ >> #undef PKGVERSION >> >> diff --git a/configure.ac b/configure.ac >> index 9f25c9f..fffc92a 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 >> - > > OK. > >> AC_ARG_ENABLE([hidden-plt], >> AC_HELP_STRING([--disable-hidden-plt], >> [do not hide internal function calls to avoid PLT]), >> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list >> index c188c6a..ec0fe20 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_ERASE >> + } >> + skip_lock_busy { >> + type: INT_32 >> + default: 3 >> + security_level: SXID_ERASE >> + } >> + skip_lock_internal_abort { >> + type: INT_32 >> + default: 3 >> + security_level: SXID_ERASE >> + } >> + skip_lock_after_retries { >> + type: INT_32 >> + default: 3 >> + security_level: SXID_ERASE >> + } >> + tries { >> + type: INT_32 >> + default: 3 >> + security_level: SXID_ERASE >> + } >> + skip_trylock_internal_abort { >> + type: INT_32 >> + default: 3 >> + security_level: SXID_ERASE >> + } >> + } > > OK. Good use of SXID_ERASE, and simplifies further code. > >> } >> diff --git a/manual/install.texi b/manual/install.texi >> index 96b988e..8d91bb9 100644 >> --- a/manual/install.texi >> +++ b/manual/install.texi >> @@ -145,9 +145,6 @@ Note that you need to make sure the external tools are kept in sync with >> the versions that @theglibc{} expects as the data formats may change over >> time. Consult the @file{timezone} subdirectory for more details. >> >> -@item --enable-lock-elision=yes >> -Enable lock elision for pthread mutexes by default. >> - > > OK. > >> @item --enable-stack-protector >> @itemx --enable-stack-protector=strong >> @itemx --enable-stack-protector=all >> diff --git a/manual/tunables.texi b/manual/tunables.texi >> index f503dae..ef62b36 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 >> @@ -212,6 +213,73 @@ 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 >> +Contended locks are usually slow and can lead to performance and scalability >> +problems on multithread code. Lock elision uses memory transactions to elide >> +locks under the right conditions as a fast path to speed up. > > Suggest: > > Contended locks are usually slow and can lead to performance and scalability > issues in multithread code. Lock elision will use transactional memory, > under certain conditions, to elide locks and improve performance. > Done. >> +Elision behavior can be modified by setting any of the following tunables in > > s/any of//g > Done. >> +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. >> + >> +Elision tunables are supported for x86-64, PowerPC, and S390 architectures. > > We should use the official hardware names here. > > Suggest: > Elision tunables are supported for 64-bit Intel, IBM POWER, and z System architectures. > Done. >> +@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. >> + >> +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 >> +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. >> + >> +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 different threads' >> +memory accesses, before falling back to regular lock. > > Suggest: > > The @code{glibc.elision.skip_lock_after_retries} tunable sets how my times to > try to elide a lock with transactions, that only failed due to a different > thread's memory accesses, before falling back to regular lock. > Done. >> +Expressed in number of lock elision attempts. >> + >> +This tunable is not supported by x86. > > Suggest: > > This tunable is only supported on IBM POWER, and z System architectures. > >> + >> +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). If elision is not supported >> +by the hardware this tunable is set to @samp{0} to avoid retries. > > Avoid the use of 'we' > > Suggest: > The @code{glibc.elision.tries} sets how many times to retry elision if > there is chance for the transaction to finish execution e.g. it wasn't > aborted due to the lock being already acquired. If elision is not supported > by the hardware this tunable is set to @samp{0} to avoid retries. > Done. >> + >> +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 the thread should avoid using trylocks if a transaction aborted due to >> +reasons other than other threads' memory accesses. Expressed in number of try >> +lock attempts. > > Suggest: > The @code{glibc.elision.skip_trylock_internal_abort} tunable sets how many > times the thread should avoid trying the lock if a transaction aborted due to > reasons other than a different thread's memory accesses. > Expressed in number of lock test (tries) attempts. > Done. >> + >> +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/nptl/Makefile b/nptl/Makefile >> index b0215e1..faf39a5 100644 >> --- a/nptl/Makefile >> +++ b/nptl/Makefile >> @@ -714,6 +714,10 @@ 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 > > OK. > >> + >> # The tests here better do not run in parallel >> ifneq ($(filter %tests,$(MAKECMDGOALS)),) >> .NOTPARALLEL: >> 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. */ > > OK. > >> 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 > > OK. > >> >> 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 > > OK. > >> >> 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 > > OK. > >> >> if (pthread_cancel (th) != 0) >> { >> 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 */ >> - > > OK. > >> #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) > > OK. > >> # 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) > > OK. > >> # 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 > > OK. > >> >> #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) > > OK. > >> # define ABORT_TRANSACTION \ >> ({ \ >> if (THREAD_GET_TM_CAPABLE ()) \ >> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c >> index f631f0a..06361e6 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 >> + > > OK. > >> /* Reasonable initial tuning values, may be revised in the future. >> This is a conservative initial value. */ >> >> @@ -50,7 +55,52 @@ 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. It's not necessary to check >> + if __libc_enable_secure isn't enabled since elision_enable will be set >> + according to the default, which is disabled. */ >> + if (elision_enable == 1) >> + __pthread_force_elision = (GLRO (dl_hwcap2) >> + & PPC_FEATURE2_HAS_HTM) ? 1 : 0; >> +} > > OK. > >> + >> +/* 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); >> +} > > OK. > >> + >> +#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 elision. */ >> >> @@ -59,13 +109,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/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 > > OK. > >> diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c >> index cc0fdef..ab334cb 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 >> + > > OK. > >> /* Reasonable initial tuning values, may be revised in the future. >> This is a conservative initial value. */ >> >> @@ -53,6 +58,50 @@ 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. It's not necessary to check >> + if __libc_enable_secure isn't enabled since elision_enable will be set >> + according to the default, which is disabled. */ >> + if (elision_enable == 1) >> + __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 +109,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)); > > OK. > >> +#endif >> >> - __pthread_force_elision = __libc_enable_secure ? 0 : elision_available; >> + if (!__pthread_force_elision) >> + __elision_aconf.try_tbegin = 0; /* Disable elision on rwlocks. */ >> } >> >> #ifdef SHARED >> 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 > > OK. > >> 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 > > OK. > >> 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 */ > > OK. > >> >> #endif /* lowlevellock.h */ >> diff --git a/sysdeps/unix/sysv/linux/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c >> index 673b000..7e9fbf9 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 >> + > > OK. > >> /* Reasonable initial tuning values, may be revised in the future. >> This is a conservative initial value. */ >> >> @@ -48,21 +53,76 @@ 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. It's not necessary to check >> + if __libc_enable_secure isn't enabled since elision_enable will be set >> + according to the default, which is disabled. */ >> + if (elision_enable == 1) >> + __pthread_force_elision = HAS_CPU_FEATURE (RTM) ? 1 : 0; > > OK. > >> +} >> + >> +/* 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)); > > OK. > >> #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. */ >> } >> >> #ifdef SHARED >> -- 2.7.4 > > From e0bbb587d9eec03dae9a3485856754e9b3254763 Mon Sep 17 00:00:00 2001 From: Rogerio Alves Date: Thu, 6 Jul 2017 13:21:08 -0500 Subject: [PATCH v5] Add elision tunables. This patch adds several new tunables to control the behavior of elision on supported platforms[1]. 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[2]. [1] This part of the patch was initially proposed by Paul Murphy but was "staled" because the framework have changed since the patch was originally proposed: https://patchwork.sourceware.org/patch/10342/ [2] This part of the patch was inititally proposed as a RFC by Carlos O'Donnell. Make sense to me integrate this on the patch: https://sourceware.org/ml/libc-alpha/2017-05/msg00335.html 2017-06-06 Rogerio A. Cardoso , Paul E. Murphy , Carlos O'Donnell * 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. Deleted __libc_enable_secure check. * sysdeps/unix/sysv/linux/s390/elision-conf.c: Likewise. * sysdeps/unix/sysv/linux/x86/elision-conf.c: Likewise. * configure.ac: Option enable_lock_elision was deleted. * config.h.in: ENABLE_LOCK_ELISION flag was deleted. * manual/install.texi: Elision configure option was removed. * INSTALL: Regenerated to remove enable_lock_elision. * nptl/Makefile: Disable elision so it can verify error case for destroying a mutex. * sysdeps/powerpc/nptl/elide.h: Cleanup ENABLE_LOCK_ELISION check. Deleted macros for the case when ENABLE_LOCK_ELISION was not defined. * nptl/tst-mutex8.c: Deleted all #ifndef ENABLE_LOCK_ELISION from the test. * sysdeps/powerpc/powerpc32/sysdep.h: Deleted all ENABLE_LOCK_ELISION checks. * sysdeps/powerpc/powerpc64/sysdep.h: Likewise. * sysdeps/powerpc/sysdep.h: Likewise. * sysdeps/unix/sysv/linux/powerpc/force-elision.h: Likewise. * sysdeps/unix/sysv/linux/s390/elision-conf.h: Likewise. * sysdeps/unix/sysv/linux/s390/force-elision.h: Likewise. * sysdeps/unix/sysv/linux/s390/lowlevellock.h: Likewise. --- Changes in v5: Per Carlos O'Donnell review. Improve documentation. Changes in v4: Per Carlos O'Donnell and Siddhesh Poyarekar reviews. Change SXID_IGNORE to SXID_ERASE. Remove libc secure check on elision. Improved documentation. Always build with elision support. Elision disabled by default at runtime. 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. INSTALL | 3 - config.h.in | 3 - configure.ac | 10 ---- elf/dl-tunables.list | 34 +++++++++++ manual/install.texi | 3 - manual/tunables.texi | 69 +++++++++++++++++++++++ nptl/Makefile | 4 ++ nptl/tst-mutex8.c | 12 +--- sysdeps/powerpc/nptl/elide.h | 9 --- sysdeps/powerpc/powerpc32/sysdep.h | 2 +- sysdeps/powerpc/powerpc64/sysdep.h | 2 +- sysdeps/powerpc/sysdep.h | 4 +- sysdeps/unix/sysv/linux/powerpc/elision-conf.c | 75 +++++++++++++++++++++++-- sysdeps/unix/sysv/linux/powerpc/force-elision.h | 2 - sysdeps/unix/sysv/linux/s390/elision-conf.c | 72 ++++++++++++++++++++++-- sysdeps/unix/sysv/linux/s390/elision-conf.h | 2 - sysdeps/unix/sysv/linux/s390/force-elision.h | 2 - sysdeps/unix/sysv/linux/s390/lowlevellock.h | 2 - sysdeps/unix/sysv/linux/x86/elision-conf.c | 74 +++++++++++++++++++++--- 19 files changed, 316 insertions(+), 68 deletions(-) diff --git a/INSTALL b/INSTALL index bc972b2..76cf167 100644 --- a/INSTALL +++ b/INSTALL @@ -115,9 +115,6 @@ will be used, and CFLAGS sets optimization options for the compiler. formats may change over time. Consult the 'timezone' subdirectory for more details. -'--enable-lock-elision=yes' - Enable lock elision for pthread mutexes by default. - '--enable-stack-protector' '--enable-stack-protector=strong' '--enable-stack-protector=all' diff --git a/config.h.in b/config.h.in index c140ff3..5622de8 100644 --- a/config.h.in +++ b/config.h.in @@ -134,9 +134,6 @@ /* Define if __stack_chk_guard canary should be randomized at program startup. */ #undef ENABLE_STACKGUARD_RANDOMIZE -/* Define if lock elision should be enabled by default. */ -#undef ENABLE_LOCK_ELISION - /* Package description. */ #undef PKGVERSION diff --git a/configure.ac b/configure.ac index 9f25c9f..fffc92a 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 - AC_ARG_ENABLE([hidden-plt], AC_HELP_STRING([--disable-hidden-plt], [do not hide internal function calls to avoid PLT]), diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index c188c6a..ec0fe20 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_ERASE + } + skip_lock_busy { + type: INT_32 + default: 3 + security_level: SXID_ERASE + } + skip_lock_internal_abort { + type: INT_32 + default: 3 + security_level: SXID_ERASE + } + skip_lock_after_retries { + type: INT_32 + default: 3 + security_level: SXID_ERASE + } + tries { + type: INT_32 + default: 3 + security_level: SXID_ERASE + } + skip_trylock_internal_abort { + type: INT_32 + default: 3 + security_level: SXID_ERASE + } + } } diff --git a/manual/install.texi b/manual/install.texi index 96b988e..8d91bb9 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -145,9 +145,6 @@ Note that you need to make sure the external tools are kept in sync with the versions that @theglibc{} expects as the data formats may change over time. Consult the @file{timezone} subdirectory for more details. -@item --enable-lock-elision=yes -Enable lock elision for pthread mutexes by default. - @item --enable-stack-protector @itemx --enable-stack-protector=strong @itemx --enable-stack-protector=all diff --git a/manual/tunables.texi b/manual/tunables.texi index f503dae..e851b95 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 * Hardware Capability Tunables:: Tunables that modify the hardware capabilities seen by @theglibc{} @end menu @@ -212,6 +213,74 @@ 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 +Contended locks are usually slow and can lead to performance and scalability +issues in multithread code. Lock elision will use memory transactions to under +certain conditions, to elide locks and improve performance. +Elision behavior can be modified by setting 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. + +Elision tunables are supported for 64-bit Intel, IBM POWER, and z System +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. + +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 +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. + +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 many times +to try to elide a lock with transactions, that only failed due to a different +thread's memory accesses, before falling back to regular lock. +Expressed in number of lock elision attempts. + +This tunable is supported only on IBM POWER, and z System architectures. + +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 to retry elision if there is +chance for the transaction to finish execution e.g., it wasn't +aborted due to the lock being already acquired. 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 the thread should avoid trying the lock if a transaction aborted due to +reasons other than a different thread's memory accesses. Expressed in number +of try lock attempts. + +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/nptl/Makefile b/nptl/Makefile index b0215e1..faf39a5 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -714,6 +714,10 @@ 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)),) .NOTPARALLEL: 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) { 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/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c index f631f0a..06361e6 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,52 @@ 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. It's not necessary to check + if __libc_enable_secure isn't enabled since elision_enable will be set + according to the default, which is disabled. */ + if (elision_enable == 1) + __pthread_force_elision = (GLRO (dl_hwcap2) + & PPC_FEATURE2_HAS_HTM) ? 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 /* Initialize elision. */ @@ -59,13 +109,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. */ } #ifdef SHARED 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.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c index cc0fdef..ab334cb 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,50 @@ 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. It's not necessary to check + if __libc_enable_secure isn't enabled since elision_enable will be set + according to the default, which is disabled. */ + if (elision_enable == 1) + __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 + /* Initialize elison. */ static void @@ -60,11 +109,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. */ } #ifdef SHARED 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 */ diff --git a/sysdeps/unix/sysv/linux/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c index 673b000..7e9fbf9 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,76 @@ 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. It's not necessary to check + if __libc_enable_secure isn't enabled since elision_enable will be set + according to the default, which is disabled. */ + if (elision_enable == 1) + __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. */ } #ifdef SHARED -- 2.7.4