public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Re: [RFC][PATCH] tunables: Add elision tunable
@ 2017-09-14 14:31 rcardoso
  2017-10-18 14:05 ` Tulio Magno Quites Machado Filho
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: rcardoso @ 2017-09-14 14:31 UTC (permalink / raw)
  To: libc-alpha; +Cc: stli

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

Hi Stefan,

 >Please also change the order of the lines (in all elision-conf.c >files):
 >+# include <elf/dl-tunables.h>
 >+# define TUNABLE_NAMESPACE elision
 >
 >If TUNABLE_NAMESPACE is not defined before dl-tunables.h is included,
 >there will be a build error as TUNABLE_GET will require 5 instead of 
 >three arguments.

That's odd I did not get a build error on my tests on POWER. But you're 
right. Fixed on v3.

Thanks,
Rogerio

[-- Attachment #2: 0001-Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 16066 bytes --]

From 3306d4c07dc673fde097420f0528d2e60c2a0d65 Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
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  <rcardoso@linux.vnet.ibm.com>,
	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* 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.
---
 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
+    }
+    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
+    }
+  }
 }
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
 * 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.
+
+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.
+
+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
+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 other threads' memory
+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.
+
+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.
+
+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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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;
+}
+
+/* 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.  */
 }
 
 #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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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)
+    __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 +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.  */
 }
 
 #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 <elision-conf.h>
 #include <unistd.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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)
+    __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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-14 14:31 [RFC][PATCH] tunables: Add elision tunable rcardoso
@ 2017-10-18 14:05 ` Tulio Magno Quites Machado Filho
  2017-10-18 18:24 ` Carlos O'Donell
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2017-10-18 14:05 UTC (permalink / raw)
  To: rcardoso, libc-alpha; +Cc: stli

rcardoso <rcardoso@linux.vnet.ibm.com> writes:

> 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  <rcardoso@linux.vnet.ibm.com>,
> 	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
>
> 	* 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.

Reviewed-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>

I'm going to push it later today.

Thanks!

-- 
Tulio Magno

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-14 14:31 [RFC][PATCH] tunables: Add elision tunable rcardoso
  2017-10-18 14:05 ` Tulio Magno Quites Machado Filho
@ 2017-10-18 18:24 ` Carlos O'Donell
  2017-10-23 15:43 ` Siddhesh Poyarekar
  2017-11-16 12:56 ` rcardoso
  3 siblings, 0 replies; 16+ messages in thread
From: Carlos O'Donell @ 2017-10-18 18:24 UTC (permalink / raw)
  To: rcardoso, libc-alpha; +Cc: stli

On 09/14/2017 07:30 AM, rcardoso wrote:
> From 3306d4c07dc673fde097420f0528d2e60c2a0d65 Mon Sep 17 00:00:00 2001
> From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
> 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  <rcardoso@linux.vnet.ibm.com>,
> 	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
> 
> 	* 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.

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 <htm.h>
 # include <elision-conf.h>
 
@@ -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 <bits/hwcap.h>
-#ifdef ENABLE_LOCK_ELISION
 #include <tls.h>
 #include <htm.h>
-#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
    <http://www.gnu.org/licenses/>.  */
 
-#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
    <http://www.gnu.org/licenses/>.  */
-#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
    <http://www.gnu.org/licenses/>.  */
 
-#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 <sysdeps/nptl/lowlevellock.h>
 
 /* 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 */


> ---
>  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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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 <elision-conf.h>
>  #include <unistd.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-14 14:31 [RFC][PATCH] tunables: Add elision tunable rcardoso
  2017-10-18 14:05 ` Tulio Magno Quites Machado Filho
  2017-10-18 18:24 ` Carlos O'Donell
@ 2017-10-23 15:43 ` Siddhesh Poyarekar
  2017-11-16 12:56 ` rcardoso
  3 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2017-10-23 15:43 UTC (permalink / raw)
  To: rcardoso, libc-alpha; +Cc: stli

On Thursday 14 September 2017 08:00 PM, rcardoso wrote:
> Hi Stefan,
> 
>>Please also change the order of the lines (in all elision-conf.c >files):
>>+# include <elf/dl-tunables.h>
>>+# define TUNABLE_NAMESPACE elision
>>
>>If TUNABLE_NAMESPACE is not defined before dl-tunables.h is included,
>>there will be a build error as TUNABLE_GET will require 5 instead of
>>three arguments.
> 
> That's odd I did not get a build error on my tests on POWER. But you're
> right. Fixed on v3.
> 
> Thanks,
> Rogerio

Sorry, here's a very late review since I noticed the patch only today.
There are a couple of points below, specifically concerning elision
behaviour for setxid binaries and their children.

>  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
> +    }
> +    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

SXID_IGNORE means that you don't want setxid binaries to read it but you
would like children of setxid binaries to read it.  Why is this use case
necessary to be supported?  We don't want to do this unless there is a
specific reason to allow it.  That is, set to SXID_ERASE unless there is
a very good reason to allow children of setxid process to read the
elision tunables.

> +    }
> +  }
>  }
> 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
>  * 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:

Should this have a brief description of what elision is?  Maybe just a
single line to give context to what we're controlling.

> +@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.
> +@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
> +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
> +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 other threads' memory
> +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.
> +
> +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.
> +
> +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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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)

If we set the tunable permissions correctly (i.e. the SXID_*) the
__libc_enable_secure check is unnecessary.  elision_enable will be set
according to the default, which is disabled.  You could replace the
check with a comment explaining why the check is unnecessary.

> +    __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 +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.  */
>  }
>  
>  #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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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)

Same comment about tunables protection applies here.
__libc_enable_secure check is unnecessary since elision_tunable will
only be set when !__libc_enable_secure.

> +    __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 +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.  */
>  }
>  
>  #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 <elision-conf.h>
>  #include <unistd.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +
>  /* 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)

Likewise, elision_enable is set only when !__libc_enable_secure, so the
check is unnecessary.

> +    __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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-14 14:31 [RFC][PATCH] tunables: Add elision tunable rcardoso
                   ` (2 preceding siblings ...)
  2017-10-23 15:43 ` Siddhesh Poyarekar
@ 2017-11-16 12:56 ` rcardoso
  2017-11-16 18:21   ` Carlos O'Donell
  3 siblings, 1 reply; 16+ messages in thread
From: rcardoso @ 2017-11-16 12:56 UTC (permalink / raw)
  To: libc-alpha; +Cc: Carlos O'Donell, siddhesh

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

Hi Carlos, Siddhesh,

Sorry for my late reply on that patch. I've implemented all the 
suggestion you made on patch v4.

About your comments Siddhesh:


 >SXID_IGNORE means that you don't want setxid binaries to read it but >you
 >would like children of setxid binaries to read it.  Why is this use >case
 >necessary to be supported?  We don't want to do this unless there is a
 >specific reason to allow it.  That is, set to SXID_ERASE unless there >is
 >a very good reason to allow children of setxid process to read the
 >elision tunables.

I've implemented that before read the documentation (I guess the 
original Paul Murphy's patch was using that (not sure). Changed to 
SXID_ERASE.

 >Should this have a brief description of what elision is?  Maybe just a
 >single line to give context to what we're controlling.

Yes. Added a line (two) explaining that elision is.

 >If we set the tunable permissions correctly (i.e. the SXID_*) the
 >__libc_enable_secure check is unnecessary.  elision_enable will be set
 >according to the default, which is disabled.  You could replace the
 >check with a comment explaining why the check is unnecessary.

Yes. Removed (also solves a boolean coercion problem with this variable)

About your questions Carlos:

 >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.

All you suggestions are implemented on patch v4. Thank you.

 >Awkward wording, suggest: (many of those)

Fixed. Using your suggestions.

 >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.


Yes. I fixed all boolean coercion's on elision. Also I've removed 
__libc_enable_secure according o Siddhesh suggestion above: 
elision_enable will be set according to the default, which is disabled. 
So the check is unnecessary.

I'll check the use for this variable and fix it on glibc on another 
patch thanks by that suggestion.

Regards,

Rogerio

[-- Attachment #2: 0001-Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 28691 bytes --]

From 8b5efb9bb4f7f5b8c7188c092cbaa3c0eeed293c Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
Date: Thu, 6 Jul 2017 13:21:08 -0500
Subject: [PATCH v4] 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  <rcardoso@linux.vnet.ibm.com>,
	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>,
	    Carlos O'Donnell <carlos@redhat.com>

	* 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.
-
 '--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..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
 * 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.
+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.
+
+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.
+
+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.
+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).  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 using trylocks if a transaction aborted due to
+reasons other than other threads' 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 <htm.h>
 # include <elision-conf.h>
 
@@ -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 <bits/hwcap.h>
-#ifdef ENABLE_LOCK_ELISION
 #include <tls.h>
 #include <htm.h>
-#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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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
    <http://www.gnu.org/licenses/>.  */
 
-#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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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
    <http://www.gnu.org/licenses/>.  */
-#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
    <http://www.gnu.org/licenses/>.  */
 
-#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 <sysdeps/nptl/lowlevellock.h>
 
 /* 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 <elision-conf.h>
 #include <unistd.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-11-16 12:56 ` rcardoso
@ 2017-11-16 18:21   ` Carlos O'Donell
  2017-11-16 18:44     ` Rogerio Alves
  0 siblings, 1 reply; 16+ messages in thread
From: Carlos O'Donell @ 2017-11-16 18:21 UTC (permalink / raw)
  To: rcardoso, libc-alpha; +Cc: siddhesh

On 11/16/2017 04:56 AM, rcardoso wrote:
> From 8b5efb9bb4f7f5b8c7188c092cbaa3c0eeed293c Mon Sep 17 00:00:00 2001
> From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
> 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  <rcardoso@linux.vnet.ibm.com>,
> 	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>,
> 	    Carlos O'Donnell <carlos@redhat.com>
> 
> 	* 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.

> +Elision behavior can be modified by setting any of the following tunables in

s/any of//g

> +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.

> +@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.

> +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.

> +
> +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.

> +
> +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 <htm.h>
>  # include <elision-conf.h>
>  
> @@ -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 <bits/hwcap.h>
> -#ifdef ENABLE_LOCK_ELISION
>  #include <tls.h>
>  #include <htm.h>
> -#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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +

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
>     <http://www.gnu.org/licenses/>.  */
>  
> -#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 <unistd.h>
>  #include <dl-procinfo.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +

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
>     <http://www.gnu.org/licenses/>.  */
> -#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
>     <http://www.gnu.org/licenses/>.  */
>  
> -#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 <sysdeps/nptl/lowlevellock.h>
>  
>  /* 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 <elision-conf.h>
>  #include <unistd.h>
>  
> +#if HAVE_TUNABLES
> +# define TUNABLE_NAMESPACE elision
> +#endif
> +#include <elf/dl-tunables.h>
> +

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


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-11-16 18:21   ` Carlos O'Donell
@ 2017-11-16 18:44     ` Rogerio Alves
  2017-11-21 12:56       ` Stefan Liebler
  0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2017-11-16 18:44 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha

[-- Attachment #1: Type: text/plain, Size: 33623 bytes --]

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 <rcardoso@linux.vnet.ibm.com>
>> 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  <rcardoso@linux.vnet.ibm.com>,
>> 	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>,
>> 	    Carlos O'Donnell <carlos@redhat.com>
>>
>> 	* 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 <htm.h>
>>   # include <elision-conf.h>
>>   
>> @@ -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 <bits/hwcap.h>
>> -#ifdef ENABLE_LOCK_ELISION
>>   #include <tls.h>
>>   #include <htm.h>
>> -#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 <unistd.h>
>>   #include <dl-procinfo.h>
>>   
>> +#if HAVE_TUNABLES
>> +# define TUNABLE_NAMESPACE elision
>> +#endif
>> +#include <elf/dl-tunables.h>
>> +
> 
> 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
>>      <http://www.gnu.org/licenses/>.  */
>>   
>> -#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 <unistd.h>
>>   #include <dl-procinfo.h>
>>   
>> +#if HAVE_TUNABLES
>> +# define TUNABLE_NAMESPACE elision
>> +#endif
>> +#include <elf/dl-tunables.h>
>> +
> 
> 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
>>      <http://www.gnu.org/licenses/>.  */
>> -#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
>>      <http://www.gnu.org/licenses/>.  */
>>   
>> -#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 <sysdeps/nptl/lowlevellock.h>
>>   
>>   /* 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 <elision-conf.h>
>>   #include <unistd.h>
>>   
>> +#if HAVE_TUNABLES
>> +# define TUNABLE_NAMESPACE elision
>> +#endif
>> +#include <elf/dl-tunables.h>
>> +
> 
> 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
> 
> 

[-- Attachment #2: 0001-Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 28811 bytes --]

From e0bbb587d9eec03dae9a3485856754e9b3254763 Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
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  <rcardoso@linux.vnet.ibm.com>,
	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>,
	    Carlos O'Donnell <carlos@redhat.com>

	* 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 <htm.h>
 # include <elision-conf.h>
 
@@ -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 <bits/hwcap.h>
-#ifdef ENABLE_LOCK_ELISION
 #include <tls.h>
 #include <htm.h>
-#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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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
    <http://www.gnu.org/licenses/>.  */
 
-#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 <unistd.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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
    <http://www.gnu.org/licenses/>.  */
-#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
    <http://www.gnu.org/licenses/>.  */
 
-#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 <sysdeps/nptl/lowlevellock.h>
 
 /* 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 <elision-conf.h>
 #include <unistd.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE elision
+#endif
+#include <elf/dl-tunables.h>
+
 /* 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-11-16 18:44     ` Rogerio Alves
@ 2017-11-21 12:56       ` Stefan Liebler
  2017-12-05 19:52         ` Tulio Magno Quites Machado Filho
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Liebler @ 2017-11-21 12:56 UTC (permalink / raw)
  To: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On 11/16/2017 07:44 PM, Rogerio Alves wrote:
> Hi Carlos,
> 
> All documentation suggestions was made on patch v5. Thanks for the review.
> 
> Regards
> 
> Rogerio.
> 

Hi Rogerio,

I've applied your patch in order to test it on s390.
Unfortunately building glibc is now failing.

Here are some notes:

git apply warned:
0001-Add-elision-tunables.patch:291: trailing whitespace.
# Disable elision for tst-mutex8 so it can verify error case for
warning: 1 line adds whitespace errors.

Changing configure.ac is not sufficient.
Please regenerate configure in order to reflect the changes in configure.ac.

There are also usages of enable_lock_elision and ENABLE_LOCK_ELISION in 
config.make.in, sysdeps/unix/sysv/linux/s390/Makefile, 
sysdeps/s390/nptl/bits/pthreadtypes-arch.h, sysdeps/s390/configure.ac.

Please have a look into attached diff which applies on top of your patch 
v5. With those changes I was able to build glibc on s390.

Afterwards I've run a small test program which determines if 
pthread_mutex_lock is using a transaction or not:

./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
Lock was elided via a transaction! (nesting-depth=1)

GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
Lock was a normal lock!

chmod +s prog_secure
GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
Lock was a normal lock!

Bye.
Stefan

[-- Attachment #2: 20171121_review_Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 4229 bytes --]

diff --git a/config.make.in b/config.make.in
index bd84a57..9da77d1 100644
--- a/config.make.in
+++ b/config.make.in
@@ -99,7 +99,6 @@ build-nscd = @build_nscd@
 use-nscd = @use_nscd@
 build-hardcoded-path-in-tests= @hardcoded_path_in_tests@
 build-pt-chown = @build_pt_chown@
-enable-lock-elision = @enable_lock_elision@
 have-tunables = @have_tunables@
 
 # Build tools.
diff --git a/configure b/configure
index d9d9243..47431bd 100755
--- a/configure
+++ b/configure
@@ -679,7 +679,6 @@ enable_werror
 all_warnings
 force_install
 bindnow
-enable_lock_elision
 hardcoded_path_in_tests
 enable_timezone_tools
 use_default_link
@@ -768,7 +767,6 @@ enable_profile
 enable_timezone_tools
 enable_hardcoded_path_in_tests
 enable_stackguard_randomization
-enable_lock_elision
 enable_hidden_plt
 enable_bind_now
 enable_stack_protector
@@ -1428,8 +1426,6 @@ Optional Features:
   --enable-stackguard-randomization
                           initialize __stack_chk_guard canary with a random
                           number at program start
-  --enable-lock-elision=yes/no
-                          Enable lock elision for pthread mutexes by default
   --disable-hidden-plt    do not hide internal function calls to avoid PLT
   --enable-bind-now       disable lazy relocations in DSOs
   --enable-stack-protector=[yes|no|all|strong]
@@ -3395,19 +3391,6 @@ if test "$enable_stackguard_randomize" = yes; then
 
 fi
 
-# Check whether --enable-lock-elision was given.
-if test "${enable_lock_elision+set}" = set; then :
-  enableval=$enable_lock_elision; enable_lock_elision=$enableval
-else
-  enable_lock_elision=no
-fi
-
-
-if test "$enable_lock_elision" = yes ; then
-  $as_echo "#define ENABLE_LOCK_ELISION 1" >>confdefs.h
-
-fi
-
 # Check whether --enable-hidden-plt was given.
 if test "${enable_hidden_plt+set}" = set; then :
   enableval=$enable_hidden_plt; hidden=$enableval
diff --git a/sysdeps/s390/configure b/sysdeps/s390/configure
index d4a4a3d..74b415f 100644
--- a/sysdeps/s390/configure
+++ b/sysdeps/s390/configure
@@ -35,7 +35,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_builtin_tbegin" >&5
 $as_echo "$libc_cv_gcc_builtin_tbegin" >&6; }
 
-if test "$enable_lock_elision" = yes && test "$libc_cv_gcc_builtin_tbegin" = no ; then
+if test "$libc_cv_gcc_builtin_tbegin" = no ; then
    critic_missing="$critic_missing The used GCC has no support for __builtin_tbegin, which is needed for lock-elision on target S390."
 fi
 
diff --git a/sysdeps/s390/configure.ac b/sysdeps/s390/configure.ac
index 7d0b5ce..1cdb021 100644
--- a/sysdeps/s390/configure.ac
+++ b/sysdeps/s390/configure.ac
@@ -26,7 +26,7 @@ else
 fi
 rm -f conftest* ])
 
-if test "$enable_lock_elision" = yes && test "$libc_cv_gcc_builtin_tbegin" = no ; then
+if test "$libc_cv_gcc_builtin_tbegin" = no ; then
    critic_missing="$critic_missing The used GCC has no support for __builtin_tbegin, which is needed for lock-elision on target S390."
 fi
 
diff --git a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
index 1ae2773..fd15931 100644
--- a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
+++ b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
@@ -40,11 +40,7 @@
 /* Definitions for internal mutex struct.  */
 #define __PTHREAD_COMPAT_PADDING_MID
 #define __PTHREAD_COMPAT_PADDING_END
-#ifdef ENABLE_LOCK_ELISION
 #define __PTHREAD_MUTEX_LOCK_ELISION    1
-#else
-#define __PTHREAD_MUTEX_LOCK_ELISION	0
-#endif
 #define __PTHREAD_MUTEX_NUSERS_AFTER_KIND  (__WORDSIZE != 64)
 #define __PTHREAD_MUTEX_USE_UNION          (__WORDSIZE != 64)
 
diff --git a/sysdeps/unix/sysv/linux/s390/Makefile b/sysdeps/unix/sysv/linux/s390/Makefile
index c5f544d..77f3852 100644
--- a/sysdeps/unix/sysv/linux/s390/Makefile
+++ b/sysdeps/unix/sysv/linux/s390/Makefile
@@ -16,7 +16,6 @@ sysdep_routines += dl-vdso
 endif
 
 ifeq ($(subdir),nptl)
-ifeq ($(enable-lock-elision),yes)
 libpthread-sysdep_routines += elision-lock elision-unlock elision-timed \
 			      elision-trylock
 
@@ -26,7 +25,6 @@ CFLAGS-elision-timed.c = $(elision-CFLAGS)
 CFLAGS-elision-trylock.c = $(elision-CFLAGS)
 CFLAGS-elision-unlock.c = $(elision-CFLAGS)
 endif
-endif
 
 ifeq ($(subdir),misc)
 tests += tst-ptrace-singleblock

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-11-21 12:56       ` Stefan Liebler
@ 2017-12-05 19:52         ` Tulio Magno Quites Machado Filho
  2018-01-09 15:54           ` Aurelien Jarno
  0 siblings, 1 reply; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2017-12-05 19:52 UTC (permalink / raw)
  To: Stefan Liebler, libc-alpha, Rogerio Alves Cardoso

Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> On 11/16/2017 07:44 PM, Rogerio Alves wrote:
>> All documentation suggestions was made on patch v5. Thanks for the review.
>> 
>> Regards
>> 
>> Rogerio.
>> 
>
> I've applied your patch in order to test it on s390.
> Unfortunately building glibc is now failing.
>
> Here are some notes:
>
> git apply warned:
> 0001-Add-elision-tunables.patch:291: trailing whitespace.
> # Disable elision for tst-mutex8 so it can verify error case for
> warning: 1 line adds whitespace errors.
>
> Changing configure.ac is not sufficient.
> Please regenerate configure in order to reflect the changes in configure.ac.
>
> There are also usages of enable_lock_elision and ENABLE_LOCK_ELISION in 
> config.make.in, sysdeps/unix/sysv/linux/s390/Makefile, 
> sysdeps/s390/nptl/bits/pthreadtypes-arch.h, sysdeps/s390/configure.ac.
>
> Please have a look into attached diff which applies on top of your patch 
> v5. With those changes I was able to build glibc on s390.
>
> Afterwards I've run a small test program which determines if 
> pthread_mutex_lock is using a transaction or not:
>
> ./prog
> Lock was a normal lock!
>
> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
> Lock was elided via a transaction! (nesting-depth=1)
>
> GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
> Lock was a normal lock!
>
> chmod +s prog_secure
> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
> Lock was a normal lock!

I applied this patch, updated the ChangeLog, run tests on ppc{|64|64le},
s390x and x86_64, and pushed the patch as 07ed18d26a342.

Thanks!

-- 
Tulio Magno

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-12-05 19:52         ` Tulio Magno Quites Machado Filho
@ 2018-01-09 15:54           ` Aurelien Jarno
  0 siblings, 0 replies; 16+ messages in thread
From: Aurelien Jarno @ 2018-01-09 15:54 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho
  Cc: Stefan Liebler, libc-alpha, Rogerio Alves Cardoso

On 2017-12-05 17:52, Tulio Magno Quites Machado Filho wrote:
> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
> 
> > On 11/16/2017 07:44 PM, Rogerio Alves wrote:
> >> All documentation suggestions was made on patch v5. Thanks for the review.
> >> 
> >> Regards
> >> 
> >> Rogerio.
> >> 
> >
> > I've applied your patch in order to test it on s390.
> > Unfortunately building glibc is now failing.
> >
> > Here are some notes:
> >
> > git apply warned:
> > 0001-Add-elision-tunables.patch:291: trailing whitespace.
> > # Disable elision for tst-mutex8 so it can verify error case for
> > warning: 1 line adds whitespace errors.
> >
> > Changing configure.ac is not sufficient.
> > Please regenerate configure in order to reflect the changes in configure.ac.
> >
> > There are also usages of enable_lock_elision and ENABLE_LOCK_ELISION in 
> > config.make.in, sysdeps/unix/sysv/linux/s390/Makefile, 
> > sysdeps/s390/nptl/bits/pthreadtypes-arch.h, sysdeps/s390/configure.ac.
> >
> > Please have a look into attached diff which applies on top of your patch 
> > v5. With those changes I was able to build glibc on s390.
> >
> > Afterwards I've run a small test program which determines if 
> > pthread_mutex_lock is using a transaction or not:
> >
> > ./prog
> > Lock was a normal lock!
> >
> > GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
> > Lock was elided via a transaction! (nesting-depth=1)
> >
> > GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
> > Lock was a normal lock!
> >
> > chmod +s prog_secure
> > GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
> > Lock was a normal lock!
> 
> I applied this patch, updated the ChangeLog, run tests on ppc{|64|64le},
> s390x and x86_64, and pushed the patch as 07ed18d26a342.

This commit cause the following tests to fail with the default
configuration on ppc{|64|64le}:
FAIL: elf/tst-env-setuid
FAIL: elf/tst-env-setuid-tunables
FAIL: stdlib/tst-secure-getenv

The problem is not new and it was possible to observe it with glibc-2.25
with --enable-tunables --enable-lock-elision=yes. Now --enable-tunables
is the default and following this commit lock elision code is always
there.

Please see https://sourceware.org/bugzilla/show_bug.cgi?id=22685 for
more details.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-13 19:28 rcardoso
@ 2017-09-14  6:32 ` Stefan Liebler
  0 siblings, 0 replies; 16+ messages in thread
From: Stefan Liebler @ 2017-09-14  6:32 UTC (permalink / raw)
  To: libc-alpha

Hi Rogerio,

On 09/13/2017 09:28 PM, rcardoso wrote:
> Hi Stefan,
> 
> Thanks for your review. I attached a v2 patch with you fix suggestions.
> 
>  >In addition to Paul's patches, here is a link to a newer patch from 
>  >Carlos:
>  >"RFC: Always-on elision with per-process opt-in using tunables."
>  >https://www.sourceware.org/ml/libc-alpha/2017-05/msg00335.html
>  >
>  >He also removes the --enable-lock-elision configure flag.
>  >Is this also required for your patch?
> 
> I'm aware of Carlos patch. In fact I talk to him before send this mail 
> since we are doing similar things. And we agree that I will send this 
> patch and he will word around with changes (like enable-lock-elision flag). >
Okay.
Please also change the order of the lines (in all elision-conf.c files):
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision

If TUNABLE_NAMESPACE is not defined before dl-tunables.h is included,
there will be a build error as TUNABLE_GET will require 5 instead of 
three arguments.


>  >I've also used a small s390 specific test program to check wether a 
> mutex was elided or not:
> 
> Pretty neat test. I see you already sent the test on this thread on 
> another reply. I will try to port it to power and x86 here.
>Testing the other variables is not as easy as en-/disabling lock-elision.

> Regards,
> Rogerio

Bye
Stefan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
@ 2017-09-13 19:28 rcardoso
  2017-09-14  6:32 ` Stefan Liebler
  0 siblings, 1 reply; 16+ messages in thread
From: rcardoso @ 2017-09-13 19:28 UTC (permalink / raw)
  To: libc-alpha; +Cc: stli

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

Hi Stefan,

Thanks for your review. I attached a v2 patch with you fix suggestions.

 >In addition to Paul's patches, here is a link to a newer patch from 
 >Carlos:
 >"RFC: Always-on elision with per-process opt-in using tunables."
 >https://www.sourceware.org/ml/libc-alpha/2017-05/msg00335.html
 >
 >He also removes the --enable-lock-elision configure flag.
 >Is this also required for your patch?

I'm aware of Carlos patch. In fact I talk to him before send this mail 
since we are doing similar things. And we agree that I will send this 
patch and he will word around with changes (like enable-lock-elision flag).

 >I've also used a small s390 specific test program to check wether a 
mutex was elided or not:

Pretty neat test. I see you already sent the test on this thread on 
another reply. I will try to port it to power and x86 here.

Regards,
Rogerio

[-- Attachment #2: 0001-Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 16049 bytes --]

From a3d8a372bba91de2dd72f998393b08bf5390b9cf Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
Date: Thu, 6 Jul 2017 13:21:08 -0500
Subject: [PATCH v2] 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  <rcardoso@linux.vnet.ibm.com>,
	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* 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.
---
 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 | 72 +++++++++++++++++++++++---
 sysdeps/unix/sysv/linux/s390/elision-conf.c    | 69 ++++++++++++++++++++++--
 sysdeps/unix/sysv/linux/x86/elision-conf.c     | 71 ++++++++++++++++++++++---
 5 files changed, 294 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
+    }
+    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
+    }
+  }
 }
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
 * 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.
+
+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.
+
+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
+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 other threads' memory
+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.
+
+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.
+
+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..9effe09 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
@@ -21,6 +21,10 @@
 #include <elision-conf.h>
 #include <unistd.h>
 #include <dl-procinfo.h>
+#if HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -50,7 +54,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;
+}
+
+/* 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 +106,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/s390/elision-conf.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c
index cc0fdef..c6da0b3 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -21,6 +21,10 @@
 #include <elision-conf.h>
 #include <unistd.h>
 #include <dl-procinfo.h>
+#if HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -53,6 +57,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)
+    __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 +106,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/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c
index 673b000..0c027cb 100644
--- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
@@ -21,6 +21,10 @@
 #include <init-arch.h>
 #include <elision-conf.h>
 #include <unistd.h>
+#if HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -48,21 +52,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)
+    __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.  */
+
+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);				\
+}
 
-/* Initialize elison.  */
+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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-13 13:43   ` Gabriel F. T. Gomes
@ 2017-09-13 14:14     ` Stefan Liebler
  0 siblings, 0 replies; 16+ messages in thread
From: Stefan Liebler @ 2017-09-13 14:14 UTC (permalink / raw)
  To: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On 09/13/2017 03:42 PM, Gabriel F. T. Gomes wrote:
> On 12 Sep 2017, Stefan Liebler wrote:
> 
>> I've also used a small s390 specific test program to check wether a
>> mutex was elided or not:
>> ./prog
>> Lock was a normal lock!
>>
>> GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
>> Lock was a normal lock!
>>
>> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
>> Lock was elided via a transaction! (nesting-depth=1)
>>
>> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
>> Lock was a normal lock!
> 
> Would you be willing to share this test program (and your build
> configurations)?  I understand that it is s390-specific, but it could help
> anyway (at least for my education, it will).
> 

Yes sure, please see the notes in the attached file.

Bye
Stefan

[-- Attachment #2: isintransaction-s390x.c --]
[-- Type: text/x-csrc, Size: 2912 bytes --]

//CFLAGS=-march=zEC12 -mzarch
/* We need at least zEC12 and zarch (on 31bit) for transaction instructions!  */
//LDFLAGS=-lpthread
#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <htmintrin.h> //for builtin_tx_nesting_depth
//https://gcc.gnu.org/onlinedocs/gcc/S_002f390-System-z-Built-in-Functions.html#S_002f390-System-z-Built-in-Functions
#include <sys/auxv.h>
#include <unistd.h>
#include <sys/types.h>

/* Note:
   You have to use configure flag --enable-lock-elision.
   Then transactions are always used (before the tunables patch).

   stli@xyz:# GLIBC_TUNABLES=glibc.elision.enable=1 ./isintransaction-s390x.new
   uid=12345; euid=12345
   Lock was elided via a transaction!

   Testing setuid:
   root@xyz:# cp isintransaction-s390x isintransaction-s390x_suid
   root@xyz:# chown root:root isintransaction-s390x_suid
   root@xyz:# chmod +s isintransaction-s390x_suid
   root@:# ls -lah
   -rwxrwxr-x  1 stli stli 142K Mar 31 09:33 isintransaction-s390x
   -rwsrwsr-x  1 root root 142K Mar 31 09:34 isintransaction-s390x_suid

   => Run isintransaction-s390x_suid as e.g. user stli and no transactions
   should be used even if you enabled it!
   stli@xyz:# GLIBC_TUNABLES=glibc.elision.enable=1 ./isintransaction-s390x_suid
   uid=12345; euid=0
   Lock was a normal lock!
*/

#define GETNESTINGDEPTH(var)					\
  ({								\
  __asm__ (".machine push \n\t"					\
	   ".machine \"all\"");					\
  var = __builtin_tx_nesting_depth ();				\
  __asm__ (".machine pop");					\
  })

int
main (void)
{
  uid_t uid = getuid ();
  uid_t euid = geteuid ();
  printf ("uid=%d; euid=%d\n", (int) uid, (int) euid);

  int have_te = (getauxval (AT_HWCAP) & HWCAP_S390_TE) ? 1 : 0;
  if (have_te == 0)
    puts ("This machine do not support transactional execution!\n"
	  "You need a >= zEC12 lpar or z/VM >= 6.4!\n");

  pthread_mutex_t testlock;
  if (pthread_mutex_init (&testlock, NULL) != 0)
    {
      printf("mutex init failed: %m \n");
      return EXIT_FAILURE;
    }

  int nestdepthInLock = -1;

  if (pthread_mutex_lock (&testlock))
    {
      printf ("LOCK failed: %m\n");
      return EXIT_FAILURE;
    }

  if (have_te)
    GETNESTINGDEPTH (nestdepthInLock);

  if (pthread_mutex_unlock (&testlock))
    {
      printf ("UNLOCK failed %m\n");
      return EXIT_FAILURE;
    }

  /* Note: a transaction can also be aborted even if lock-elision is available.
     If the lock should be elided but was not, then try again and set a debug
     the transaction fallback path.  */
  if (nestdepthInLock > 0)
    printf ("Lock was elided via a transaction! (nesting-depth=%d)\n",
	    nestdepthInLock);
  else if (nestdepthInLock < 0)
    puts ("Nesting-depth could not be determined!");
  else
    puts ("Lock was a normal lock!");

  if (pthread_mutex_destroy (&testlock))
    {
      printf("mutex destroy failed: %m \n");
      return EXIT_FAILURE;
    }

  return EXIT_SUCCESS;
}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-12  9:36 ` Stefan Liebler
@ 2017-09-13 13:43   ` Gabriel F. T. Gomes
  2017-09-13 14:14     ` Stefan Liebler
  0 siblings, 1 reply; 16+ messages in thread
From: Gabriel F. T. Gomes @ 2017-09-13 13:43 UTC (permalink / raw)
  To: Stefan Liebler; +Cc: libc-alpha

On 12 Sep 2017, Stefan Liebler wrote:

>I've also used a small s390 specific test program to check wether a 
>mutex was elided or not:
>./prog
>Lock was a normal lock!
>
>GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
>Lock was a normal lock!
>
>GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
>Lock was elided via a transaction! (nesting-depth=1)
>
>GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
>Lock was a normal lock!

Would you be willing to share this test program (and your build
configurations)?  I understand that it is s390-specific, but it could help
anyway (at least for my education, it will).

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][PATCH] tunables: Add elision tunable
  2017-09-11 13:25 rcardoso
@ 2017-09-12  9:36 ` Stefan Liebler
  2017-09-13 13:43   ` Gabriel F. T. Gomes
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Liebler @ 2017-09-12  9:36 UTC (permalink / raw)
  To: libc-alpha

On 09/11/2017 03:25 PM, rcardoso wrote:
> Hello,
> 
> Attached to this mail I have a patch which add elision to tunables 
> framework. This is a rebase of Paul Murphy's patch [1]. Also there's a 
> RFC related to this [2].
> 
> [1] https://patchwork.sourceware.org/patch/10342/
> [2] https://patchwork.sourceware.org/patch/20407/


Hi Rogerio,

Thanks for working on this topic.
In addition to Paul's patches, here is a link to a newer patch from Carlos:
"RFC: Always-on elision with per-process opt-in using tunables."
https://www.sourceware.org/ml/libc-alpha/2017-05/msg00335.html

He also removes the --enable-lock-elision configure flag.
Is this also required for your patch?

I've applied your patch to test it on s390x.
Some changes are needed in order to get it work:


diff --git a/manual/tunables.texi b/manual/tunables.texi
index 166624c..fb845b1 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -211,6 +211,7 @@ bounded, the user may set this tunable to limit the 
number of chunks
  that are scanned from the unsorted list while searching for chunks to
  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
diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.c 
b/sysdeps/unix/sysv/linux/s390/elision-conf.c
index 5f9c6d1..3dcf580 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -21,9 +21,9 @@
  #include <elision-conf.h>
  #include <unistd.h>
  #include <dl-procinfo.h>
-#ifdef HAVE_TUNABLES
-# include <elf/dl-tunables.h>
+#if HAVE_TUNABLES
  # define TUNABLE_NAMESPACE elision
+# include <elf/dl-tunables.h>
  #endif

  /* Reasonable initial tuning values, may be revised in the future.
@@ -57,7 +57,7 @@ struct elision_config __elision_aconf =

  int __pthread_force_elision attribute_hidden = 0;

-#ifdef HAVE_TUNABLES
+#if HAVE_TUNABLES
  static inline void
  __always_inline
  do_set_elision_enable (int32_t elision_enable)
@@ -106,7 +106,7 @@ elision_init (int argc __attribute__ ((unused)),
               char **argv  __attribute__ ((unused)),
               char **environ)
  {
-#ifdef HAVE_TUNABLES
+#if HAVE_TUNABLES
    /* Elision depends on tunables and must be explicitly turned on by 
setting
       the appropriate tunable on a supported platform.  */

Note: I haven't tested it on power or x86.


With these mentioned changes and the comments below,
I could build glibc and the testsuite runs without regressions.

I've also used a small s390 specific test program to check wether a 
mutex was elided or not:
./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
Lock was elided via a transaction! (nesting-depth=1)

GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=2 ./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=yes ./prog
Lock was a normal lock!

Note: I haven't tested setting the other variables.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC][PATCH] tunables: Add elision tunable
@ 2017-09-11 13:25 rcardoso
  2017-09-12  9:36 ` Stefan Liebler
  0 siblings, 1 reply; 16+ messages in thread
From: rcardoso @ 2017-09-11 13:25 UTC (permalink / raw)
  To: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

Hello,

Attached to this mail I have a patch which add elision to tunables 
framework. This is a rebase of Paul Murphy's patch [1]. Also there's a 
RFC related to this [2].

[1] https://patchwork.sourceware.org/patch/10342/
[2] https://patchwork.sourceware.org/patch/20407/

[-- Attachment #2: 0001-Add-elision-tunables.patch --]
[-- Type: text/x-patch, Size: 16897 bytes --]

From eb98c17fb08e7e5810af2108e820924cebf062d0 Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
Date: Thu, 6 Jul 2017 13:21:08 -0500
Subject: [PATCH v6] 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  <rcardoso@linux.vnet.ibm.com>,
	    Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* 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.
---
 Changes in v6: Based on Gabriel review. Fix small issues. Add
set_elision on CALLBACK macro for x86 s390.

 Changes in v5: Based on Gabriel review. Fix things that was uncompliant with 
 code standards. Fix others small things on manual that I was forgot on the 
 last review. 

 Changes in v4: Based on Gabriel review. Fix some small things on manual.

 Changes in v3: Based on Tulio review. Changed namespace from pthread to elision.
 Fix lots of extra spaces and typos. Fix some space/tab issues. Running 
 checkpath script to catch some code format issues. Reworked on commit message. 

 Changes in v2: Based on Raji review. Created manual entry for elision tunables. 
 Fix extra spaces on tunables list. Fix incorrect macro parameter on elision 
 enable for s390. Fix identation on defines. Include missing tunable namespace 
 on x86/elision-conf.c. Using #ifdef instead #if

 elf/dl-tunables.list                           | 34 ++++++++++++
 manual/tunables.texi                           | 64 +++++++++++++++++++++++
 sysdeps/unix/sysv/linux/powerpc/elision-conf.c | 72 +++++++++++++++++++++++---
 sysdeps/unix/sysv/linux/s390/elision-conf.c    | 69 ++++++++++++++++++++++--
 sysdeps/unix/sysv/linux/x86/elision-conf.c     | 71 ++++++++++++++++++++++---
 5 files changed, 293 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
+    }
+    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
+    }
+  }
 }
diff --git a/manual/tunables.texi b/manual/tunables.texi
index 3c19567..5b73f73 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
@@ -224,6 +225,69 @@ bounded, the user may set this tunable to limit the number of chunks
 that are scanned from the unsorted list while searching for chunks to
 pre-fill the per-thread cache with.  The default, or when set to zero,
 is no limit.
+
+@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.
+
+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.
+
+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
+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 other threads' memory
+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.
+
+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.
+
+The default value of this tunable is @samp{3}.
 @end deftp
 
 @node 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..07ebe37 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
@@ -21,6 +21,10 @@
 #include <elision-conf.h>
 #include <unistd.h>
 #include <dl-procinfo.h>
+#ifdef HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -50,7 +54,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;
+
+#ifdef 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;
+}
+
+/* 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 +106,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;
+#ifdef 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/s390/elision-conf.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c
index cc0fdef..5f9c6d1 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -21,6 +21,10 @@
 #include <elision-conf.h>
 #include <unistd.h>
 #include <dl-procinfo.h>
+#ifdef HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -53,6 +57,48 @@ struct elision_config __elision_aconf =
 
 int __pthread_force_elision attribute_hidden = 0;
 
+#ifdef 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_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 +106,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;
+#ifdef 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/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c
index 673b000..02e87dc 100644
--- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
@@ -21,6 +21,10 @@
 #include <init-arch.h>
 #include <elision-conf.h>
 #include <unistd.h>
+#ifdef HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+# define TUNABLE_NAMESPACE elision
+#endif
 
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
@@ -48,21 +52,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;
+
+#ifdef 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 = 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.  */
+
+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);				\
+}
 
-/* Initialize elison.  */
+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;
+#ifdef 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-01-09 15:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-14 14:31 [RFC][PATCH] tunables: Add elision tunable rcardoso
2017-10-18 14:05 ` Tulio Magno Quites Machado Filho
2017-10-18 18:24 ` Carlos O'Donell
2017-10-23 15:43 ` Siddhesh Poyarekar
2017-11-16 12:56 ` rcardoso
2017-11-16 18:21   ` Carlos O'Donell
2017-11-16 18:44     ` Rogerio Alves
2017-11-21 12:56       ` Stefan Liebler
2017-12-05 19:52         ` Tulio Magno Quites Machado Filho
2018-01-09 15:54           ` Aurelien Jarno
  -- strict thread matches above, loose matches on Subject: below --
2017-09-13 19:28 rcardoso
2017-09-14  6:32 ` Stefan Liebler
2017-09-11 13:25 rcardoso
2017-09-12  9:36 ` Stefan Liebler
2017-09-13 13:43   ` Gabriel F. T. Gomes
2017-09-13 14:14     ` Stefan Liebler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).