* RFC: Always-on elision with per-process opt-in using tunables.
@ 2017-05-11 15:45 Carlos O'Donell
2017-05-12 11:14 ` Stefan Liebler
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Carlos O'Donell @ 2017-05-11 15:45 UTC (permalink / raw)
To: GNU C Library
I am going to propose the following:
* Always build with elision support.
* Elision disabled by default at runtime.
* Use tunables to allow processes to opt-in to transparent elision.
The benefit is that the elision support doesn't bit-rot, and we keep
it working, and that distributions can conservatively backport elision
and allow users to test enablement on a per-process basis.
The elision is enabled with:
GLIBC_TUNABLE=glibc.elision.enable=1
The obvious set of patches are:
(a) Split out some cleanups in this patch.
(b) Always build with elision and us tunables to opt-in.
(c) Extend tunables to allow modification of elision parameters
(useful for upcoming rwlock elision re-enablement).
I'm testing this on x86_64, ppc64le, and s390x.
Thoughts?
diff --git a/config.h.in b/config.h.in
index 2caa412..47b139e 100644
--- a/config.h.in
+++ b/config.h.in
@@ -147,9 +147,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 b/configure
index 422482f..d3b68c1 100755
--- a/configure
+++ b/configure
@@ -677,7 +677,6 @@ enable_werror
all_warnings
force_install
bindnow
-enable_lock_elision
hardcoded_path_in_tests
enable_timezone_tools
use_default_link
@@ -766,7 +765,6 @@ enable_profile
enable_timezone_tools
enable_hardcoded_path_in_tests
enable_stackguard_randomization
-enable_lock_elision
enable_add_ons
enable_hidden_plt
enable_bind_now
@@ -1427,8 +1425,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
--enable-add-ons[=DIRS...]
configure and build add-ons in DIR1,DIR2,... search
for add-ons if no parameter given
@@ -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-add-ons was given.
if test "${enable_add_ons+set}" = set; then :
enableval=$enable_add_ons;
diff --git a/configure.ac b/configure.ac
index 7f43042..a970296 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...@:>@],
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index f33adfb..3dad518 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -18,8 +18,8 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#ifndef _TUNABLES_H_
-#define _TUNABLES_H_
+#ifndef _DL_TUNABLES_H_
+#define _DL_TUNABLES_H_
#if !HAVE_TUNABLES
static inline void
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index b9f1488..97062f9 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -77,4 +77,12 @@ glibc {
security_level: SXID_IGNORE
}
}
+ elision {
+ enable {
+ type: INT_32
+ minval: 0
+ maxval: 1
+ security_level: SXID_IGNORE
+ }
+ }
}
diff --git a/nptl/Makefile b/nptl/Makefile
index 6d48c0c..a720612 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -704,6 +704,10 @@ $(objpfx)tst-oddstacklimit.out: $(objpfx)tst-oddstacklimit $(objpfx)tst-basic1
$(evaluate-test)
endif
+# 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/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index 601240a..6dba7e1 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -122,9 +122,9 @@ END {
}
print "/* AUTOGENERATED by gen-tunables.awk. */"
- print "#ifndef _TUNABLES_H_"
+ print "#ifndef _DL_TUNABLES_H_"
print "# error \"Do not include this file directly.\""
- print "# error \"Include tunables.h instead.\""
+ print "# error \"Include dl-tunables.h instead.\""
print "#endif"
# Now, the enum names
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 f92ab2c..0ed83d3 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -88,7 +88,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 db7c1d7..79a2697 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -272,7 +272,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/s390/nptl/bits/pthreadtypes.h b/sysdeps/s390/nptl/bits/pthreadtypes.h
index 48ffdb4..88d21f0 100644
--- a/sysdeps/s390/nptl/bits/pthreadtypes.h
+++ b/sysdeps/s390/nptl/bits/pthreadtypes.h
@@ -89,37 +89,25 @@ typedef union
binary compatibility with static initializers. */
int __kind;
#if __WORDSIZE == 64
-# ifdef ENABLE_LOCK_ELISION
short __spins;
short __elision;
/* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
-# define __PTHREAD_SPINS 0, 0
-# else
- int __spins;
- /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
-# define __PTHREAD_SPINS 0
-# endif
+# define __PTHREAD_SPINS 0, 0
__pthread_list_t __list;
# define __PTHREAD_MUTEX_HAVE_PREV 1
#else
unsigned int __nusers;
__extension__ union
{
-# ifdef ENABLE_LOCK_ELISION
struct
{
short __espins;
short __elision;
} _d;
-# define __spins _d.__espins
-# define __elision _d.__elision
- /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
-# define __PTHREAD_SPINS { 0, 0 }
-# else
- int __spins;
- /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
-# define __PTHREAD_SPINS 0
-# endif
+# define __spins _d.__espins
+# define __elision _d.__elision
+ /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
+# define __PTHREAD_SPINS { 0, 0 }
__pthread_slist_t __list;
};
#endif
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
index f631f0a..a767a0c 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
@@ -21,6 +21,7 @@
#include <elision-conf.h>
#include <unistd.h>
#include <dl-procinfo.h>
+#include <elf/dl-tunables.h>
/* Reasonable initial tuning values, may be revised in the future.
This is a conservative initial value. */
@@ -54,18 +55,47 @@ int __pthread_force_elision attribute_hidden;
/* Initialize elision. */
+static inline void
+do_set_elision_enable (bool elision_enable)
+{
+ /* If elision is not enabled, or we are in a secure mode,
+ make sure elision is never used... */
+ if (!elision_enable || __libc_enable_secure)
+ {
+ __pthread_force_elision = 0;
+ return;
+ }
+
+ /* ... otherwise enable elision based on hardare availability. */
+ __pthread_force_elision = (GLRO (dl_hwcap2)
+ & PPC_FEATURE2_HAS_HTM) ? 1 : 0;
+}
+
+#if HAVE_TUNABLES
+/* The elision->enable tunable is either 0 or 1 to indicate that
+ elision should be disabled or enabled respectively. Availability at
+ the hardware level will still dictate if the feature is used. */
+void
+DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
+{
+ int32_t elision_enable = (int32_t) valp->numval;
+ do_set_elision_enable (elision_enable ? true : false);
+}
+#endif
+
static void
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 to enable and tune elision at runtime. */
+# define TUNABLE_NAMESPACE elision
+ TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
+#else
+ /* Disable elision when built without tunables. */
+ do_set_elision_enable (false);
#endif
- if (!__pthread_force_elision)
- /* Disable elision on rwlocks. */
- __elision_aconf.try_tbegin = 0;
}
#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..c6d9962 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -21,6 +21,7 @@
#include <elision-conf.h>
#include <unistd.h>
#include <dl-procinfo.h>
+#include <elf/dl-tunables.h>
/* Reasonable initial tuning values, may be revised in the future.
This is a conservative initial value. */
@@ -55,16 +56,46 @@ int __pthread_force_elision attribute_hidden = 0;
/* Initialize elison. */
+static inline void
+do_set_elision_enable (bool elision_enable)
+{
+ /* If elision is not enabled, or we are in a secure mode,
+ make sure elision is never used... */
+ if (!elision_enable || __libc_enable_secure)
+ {
+ __pthread_force_elision = 0;
+ return;
+ }
+
+ /* ... otherwise enable elision based on hardare availability. */
+ __pthread_force_elision = (GLRO (dl_hwcap) & HWCAP_S390_TE) ? 1 : 0;
+}
+
+#if HAVE_TUNABLES
+/* The elision->enable tunable is either 0 or 1 to indicate that
+ elision should be disabled or enabled respectively. Availability at
+ the hardware level will still dictate if the feature is used. */
+void
+DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
+{
+ int32_t elision_enable = (int32_t) valp->numval;
+ do_set_elision_enable (elision_enable ? true : false);
+}
+#endif
+
static void
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;
-
- __pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
+#if HAVE_TUNABLES
+ /* Elision depends on tunables to enable and tune elision at runtime. */
+# define TUNABLE_NAMESPACE elision
+ TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
+#else
+ /* Disable elision when built without tunables. */
+ do_set_elision_enable (false);
+#endif
}
#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..7d3d8fd 100644
--- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
@@ -21,6 +21,7 @@
#include <init-arch.h>
#include <elision-conf.h>
#include <unistd.h>
+#include <elf/dl-tunables.h>
/* Reasonable initial tuning values, may be revised in the future.
This is a conservative initial value. */
@@ -52,17 +53,46 @@ int __pthread_force_elision attribute_hidden;
/* Initialize elison. */
+static inline void
+do_set_elision_enable (bool elision_enable)
+{
+ /* If elision is not enabled, or we are in a secure mode,
+ make sure elision is never used... */
+ if (!elision_enable || __libc_enable_secure)
+ {
+ __pthread_force_elision = 0;
+ return;
+ }
+
+ /* ... otherwise enable elision based on hardare availability. */
+ __pthread_force_elision = HAS_CPU_FEATURE (RTM);
+}
+
+#if HAVE_TUNABLES
+/* The elision->enable tunable is either 0 or 1 to indicate that
+ elision should be disabled or enabled respectively. Availability at
+ the hardware level will still dictate if the feature is used. */
+void
+DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp)
+{
+ int32_t elision_enable = (int32_t) valp->numval;
+ do_set_elision_enable (elision_enable ? true : false);
+}
+#endif
+
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 to enable and tune elision at runtime. */
+# define TUNABLE_NAMESPACE elision
+ TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
+#else
+ /* Disable elision when built without tunables. */
+ do_set_elision_enable (false);
#endif
- if (!elision_available)
- __elision_aconf.retry_try_xbegin = 0; /* Disable elision on rwlocks */
}
#ifdef SHARED
---
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFC: Always-on elision with per-process opt-in using tunables.
2017-05-11 15:45 RFC: Always-on elision with per-process opt-in using tunables Carlos O'Donell
@ 2017-05-12 11:14 ` Stefan Liebler
2017-06-21 7:02 ` Stefan Liebler
2017-05-15 19:59 ` Tulio Magno Quites Machado Filho
2017-05-30 6:46 ` Torvald Riegel
2 siblings, 1 reply; 5+ messages in thread
From: Stefan Liebler @ 2017-05-12 11:14 UTC (permalink / raw)
To: libc-alpha
On 05/11/2017 05:45 PM, Carlos O'Donell wrote:
> I am going to propose the following:
>
> * Always build with elision support.
>
> * Elision disabled by default at runtime.
>
> * Use tunables to allow processes to opt-in to transparent elision.
>
> The benefit is that the elision support doesn't bit-rot, and we keep
> it working, and that distributions can conservatively backport elision
> and allow users to test enablement on a per-process basis.
This is an improvement for users as they can simply compare performance
of their workload with or without lock elsion.
With (c) - see below - further tuning is possible.
>
> The elision is enabled with:
>
> GLIBC_TUNABLE=glibc.elision.enable=1
>
> The obvious set of patches are:
>
> (a) Split out some cleanups in this patch.
> (b) Always build with elision and us tunables to opt-in.
> (c) Extend tunables to allow modification of elision parameters
> (useful for upcoming rwlock elision re-enablement).
>
> I'm testing this on x86_64, ppc64le, and s390x.
>
I've applied this patch to test on s390x.
Some changes are needed in order to build it.
Please remove the line "enable-lock-elision = @enable_lock_elision"
in config.make.in. This variable is used in
sysdeps/unix/sysv/linux/s390/Makefile, which has to be adjusted, too:
ifeq ($(enable-lock-elision),yes)
...
endif
The s390 configure check to test the availability of __builtin_tbegin
has to be adjusted:
diff --git a/sysdeps/s390/configure.ac b/sysdeps/s390/configure.ac
index 8a782e7..62bcfb9 100644
--- a/sysdeps/s390/configure.ac
+++ b/sysdeps/s390/configure.ac
@@ -32,7 +32,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
Note: __builtin_tbegin is available on RHEL/SLES GCC 4.8.5.
Please remove '--enable-lock-elision=yes' in INSTALL, too.
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!
> Thoughts?
>
> diff --git a/config.h.in b/config.h.in
> index 2caa412..47b139e 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -147,9 +147,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 b/configure
> index 422482f..d3b68c1 100755
> --- a/configure
> +++ b/configure
> @@ -677,7 +677,6 @@ enable_werror
> all_warnings
> force_install
> bindnow
> -enable_lock_elision
> hardcoded_path_in_tests
> enable_timezone_tools
> use_default_link
> @@ -766,7 +765,6 @@ enable_profile
> enable_timezone_tools
> enable_hardcoded_path_in_tests
> enable_stackguard_randomization
> -enable_lock_elision
> enable_add_ons
> enable_hidden_plt
> enable_bind_now
> @@ -1427,8 +1425,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
> --enable-add-ons[=DIRS...]
> configure and build add-ons in DIR1,DIR2,... search
> for add-ons if no parameter given
> @@ -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-add-ons was given.
> if test "${enable_add_ons+set}" = set; then :
> enableval=$enable_add_ons;
> diff --git a/configure.ac b/configure.ac
> index 7f43042..a970296 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...@:>@],
> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index f33adfb..3dad518 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -18,8 +18,8 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#ifndef _TUNABLES_H_
> -#define _TUNABLES_H_
> +#ifndef _DL_TUNABLES_H_
> +#define _DL_TUNABLES_H_
>
> #if !HAVE_TUNABLES
> static inline void
> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> index b9f1488..97062f9 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -77,4 +77,12 @@ glibc {
> security_level: SXID_IGNORE
> }
> }
> + elision {
> + enable {
> + type: INT_32
> + minval: 0
> + maxval: 1
> + security_level: SXID_IGNORE
> + }
> + }
> }
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 6d48c0c..a720612 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -704,6 +704,10 @@ $(objpfx)tst-oddstacklimit.out: $(objpfx)tst-oddstacklimit $(objpfx)tst-basic1
> $(evaluate-test)
> endif
>
> +# 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/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index 601240a..6dba7e1 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -122,9 +122,9 @@ END {
> }
>
> print "/* AUTOGENERATED by gen-tunables.awk. */"
> - print "#ifndef _TUNABLES_H_"
> + print "#ifndef _DL_TUNABLES_H_"
> print "# error \"Do not include this file directly.\""
> - print "# error \"Include tunables.h instead.\""
> + print "# error \"Include dl-tunables.h instead.\""
> print "#endif"
>
> # Now, the enum names
> 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 f92ab2c..0ed83d3 100644
> --- a/sysdeps/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> @@ -88,7 +88,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 db7c1d7..79a2697 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -272,7 +272,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/s390/nptl/bits/pthreadtypes.h b/sysdeps/s390/nptl/bits/pthreadtypes.h
> index 48ffdb4..88d21f0 100644
> --- a/sysdeps/s390/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/s390/nptl/bits/pthreadtypes.h
> @@ -89,37 +89,25 @@ typedef union
> binary compatibility with static initializers. */
> int __kind;
> #if __WORDSIZE == 64
> -# ifdef ENABLE_LOCK_ELISION
> short __spins;
> short __elision;
> /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
> -# define __PTHREAD_SPINS 0, 0
> -# else
> - int __spins;
> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
> -# define __PTHREAD_SPINS 0
> -# endif
> +# define __PTHREAD_SPINS 0, 0
> __pthread_list_t __list;
> # define __PTHREAD_MUTEX_HAVE_PREV 1
> #else
> unsigned int __nusers;
> __extension__ union
> {
> -# ifdef ENABLE_LOCK_ELISION
> struct
> {
> short __espins;
> short __elision;
> } _d;
> -# define __spins _d.__espins
> -# define __elision _d.__elision
> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
> -# define __PTHREAD_SPINS { 0, 0 }
> -# else
> - int __spins;
> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
> -# define __PTHREAD_SPINS 0
> -# endif
> +# define __spins _d.__espins
> +# define __elision _d.__elision
> + /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
> +# define __PTHREAD_SPINS { 0, 0 }
> __pthread_slist_t __list;
> };
> #endif
Adhemerval's commit "Move shared pthread definitions to common headers"
consolidates the pthradtypes.h files.
Thus sysdeps/s390/nptl/bits/pthreadtypes.h does not exist anymore.
I've adjusted sysdeps/s390/nptl/bits/pthreadtypes-arch.h for my test:
diff --git a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
index 3a9ac57..db7195a 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 __LOCK_ALIGNMENT
#define __ONCE_ALIGNMENT
> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> index f631f0a..a767a0c 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> @@ -21,6 +21,7 @@
> #include <elision-conf.h>
> #include <unistd.h>
> #include <dl-procinfo.h>
> +#include <elf/dl-tunables.h>
>
> /* Reasonable initial tuning values, may be revised in the future.
> This is a conservative initial value. */
> @@ -54,18 +55,47 @@ int __pthread_force_elision attribute_hidden;
>
> /* Initialize elision. */
>
> +static inline void
> +do_set_elision_enable (bool elision_enable)
> +{
> + /* If elision is not enabled, or we are in a secure mode,
> + make sure elision is never used... */
> + if (!elision_enable || __libc_enable_secure)
> + {
> + __pthread_force_elision = 0;
> + return;
> + }
> +
> + /* ... otherwise enable elision based on hardare availability. */
> + __pthread_force_elision = (GLRO (dl_hwcap2)
> + & PPC_FEATURE2_HAS_HTM) ? 1 : 0;
> +}
> +
> +#if HAVE_TUNABLES
> +/* The elision->enable tunable is either 0 or 1 to indicate that
> + elision should be disabled or enabled respectively. Availability at
> + the hardware level will still dictate if the feature is used. */
> +void
> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
tunable_val_t *valp instead of tunable_val_t *elision_enable.
> +{
> + int32_t elision_enable = (int32_t) valp->numval;
> + do_set_elision_enable (elision_enable ? true : false);
> +}
> +#endif
> +
> static void
> 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 to enable and tune elision at runtime. */
> +# define TUNABLE_NAMESPACE elision
> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
> +#else
> + /* Disable elision when built without tunables. */
> + do_set_elision_enable (false);
> #endif
> - if (!__pthread_force_elision)
> - /* Disable elision on rwlocks. */
> - __elision_aconf.try_tbegin = 0;
> }
>
> #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..c6d9962 100644
> --- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
> +++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
> @@ -21,6 +21,7 @@
> #include <elision-conf.h>
> #include <unistd.h>
> #include <dl-procinfo.h>
> +#include <elf/dl-tunables.h>
>
> /* Reasonable initial tuning values, may be revised in the future.
> This is a conservative initial value. */
> @@ -55,16 +56,46 @@ int __pthread_force_elision attribute_hidden = 0;
>
> /* Initialize elison. */
>
> +static inline void
> +do_set_elision_enable (bool elision_enable)
> +{
> + /* If elision is not enabled, or we are in a secure mode,
> + make sure elision is never used... */
> + if (!elision_enable || __libc_enable_secure)
> + {
> + __pthread_force_elision = 0;
> + return;
> + }
> +
> + /* ... otherwise enable elision based on hardare availability. */
> + __pthread_force_elision = (GLRO (dl_hwcap) & HWCAP_S390_TE) ? 1 : 0;
> +}
> +
> +#if HAVE_TUNABLES
> +/* The elision->enable tunable is either 0 or 1 to indicate that
> + elision should be disabled or enabled respectively. Availability at
> + the hardware level will still dictate if the feature is used. */
> +void
> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
tunable_val_t *valp instead of tunable_val_t *elision_enable.
> +{
> + int32_t elision_enable = (int32_t) valp->numval;
> + do_set_elision_enable (elision_enable ? true : false);
> +}
> +#endif
> +
> static void
> 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;
> -
> - __pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
> +#if HAVE_TUNABLES
> + /* Elision depends on tunables to enable and tune elision at runtime. */
> +# define TUNABLE_NAMESPACE elision
> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
> +#else
> + /* Disable elision when built without tunables. */
> + do_set_elision_enable (false);
> +#endif
> }
>
> #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) \
Can you adjust the occurences of "# define" within the old #ifdef
ENABLE_LOCK_ELISON block to "# define".
> __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..7d3d8fd 100644
> --- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
> +++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
> @@ -21,6 +21,7 @@
> #include <init-arch.h>
> #include <elision-conf.h>
> #include <unistd.h>
> +#include <elf/dl-tunables.h>
>
> /* Reasonable initial tuning values, may be revised in the future.
> This is a conservative initial value. */
> @@ -52,17 +53,46 @@ int __pthread_force_elision attribute_hidden;
>
> /* Initialize elison. */
>
> +static inline void
> +do_set_elision_enable (bool elision_enable)
> +{
> + /* If elision is not enabled, or we are in a secure mode,
> + make sure elision is never used... */
> + if (!elision_enable || __libc_enable_secure)
> + {
> + __pthread_force_elision = 0;
> + return;
> + }
> +
> + /* ... otherwise enable elision based on hardare availability. */
> + __pthread_force_elision = HAS_CPU_FEATURE (RTM);
> +}
> +
> +#if HAVE_TUNABLES
> +/* The elision->enable tunable is either 0 or 1 to indicate that
> + elision should be disabled or enabled respectively. Availability at
> + the hardware level will still dictate if the feature is used. */
> +void
> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp)
> +{
> + int32_t elision_enable = (int32_t) valp->numval;
> + do_set_elision_enable (elision_enable ? true : false);
> +}
> +#endif
> +
> 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 to enable and tune elision at runtime. */
> +# define TUNABLE_NAMESPACE elision
> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
> +#else
> + /* Disable elision when built without tunables. */
> + do_set_elision_enable (false);
> #endif
> - if (!elision_available)
> - __elision_aconf.retry_try_xbegin = 0; /* Disable elision on rwlocks */
> }
>
> #ifdef SHARED
> ---
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFC: Always-on elision with per-process opt-in using tunables.
2017-05-11 15:45 RFC: Always-on elision with per-process opt-in using tunables Carlos O'Donell
2017-05-12 11:14 ` Stefan Liebler
@ 2017-05-15 19:59 ` Tulio Magno Quites Machado Filho
2017-05-30 6:46 ` Torvald Riegel
2 siblings, 0 replies; 5+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2017-05-15 19:59 UTC (permalink / raw)
To: Carlos O'Donell, GNU C Library
Carlos O'Donell <carlos@redhat.com> writes:
> I am going to propose the following:
>
> * Always build with elision support.
>
> * Elision disabled by default at runtime.
>
> * Use tunables to allow processes to opt-in to transparent elision.
>
> The benefit is that the elision support doesn't bit-rot, and we keep
> it working, and that distributions can conservatively backport elision
> and allow users to test enablement on a per-process basis.
>
> The elision is enabled with:
>
> GLIBC_TUNABLE=glibc.elision.enable=1
I like this proposal.
> The obvious set of patches are:
>
> (a) Split out some cleanups in this patch.
> (b) Always build with elision and us tunables to opt-in.
> (c) Extend tunables to allow modification of elision parameters
> (useful for upcoming rwlock elision re-enablement).
Paul Murphy had a patch to implement this step.
It could be useful, although it's outdated:
https://patchwork.sourceware.org/patch/10342/
Another patch to the set you suggested:
(d) Extend the testsuite to test elided locks too.
--
Tulio Magno
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFC: Always-on elision with per-process opt-in using tunables.
2017-05-11 15:45 RFC: Always-on elision with per-process opt-in using tunables Carlos O'Donell
2017-05-12 11:14 ` Stefan Liebler
2017-05-15 19:59 ` Tulio Magno Quites Machado Filho
@ 2017-05-30 6:46 ` Torvald Riegel
2 siblings, 0 replies; 5+ messages in thread
From: Torvald Riegel @ 2017-05-30 6:46 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: GNU C Library
On Thu, 2017-05-11 at 11:45 -0400, Carlos O'Donell wrote:
> I am going to propose the following:
>
> * Always build with elision support.
>
> * Elision disabled by default at runtime.
>
> * Use tunables to allow processes to opt-in to transparent elision.
>
> The benefit is that the elision support doesn't bit-rot, and we keep
> it working, and that distributions can conservatively backport elision
> and allow users to test enablement on a per-process basis.
>
> The elision is enabled with:
>
> GLIBC_TUNABLE=glibc.elision.enable=1
>
> The obvious set of patches are:
>
> (a) Split out some cleanups in this patch.
> (b) Always build with elision and us tunables to opt-in.
> (c) Extend tunables to allow modification of elision parameters
> (useful for upcoming rwlock elision re-enablement).
>
> I'm testing this on x86_64, ppc64le, and s390x.
>
> Thoughts?
I agree this would be useful.
Enabling elision by default for a specific target should require that we
have confidence that performance with elision is better overall (which
is affected by the default elision tuning parameters, for example). We
still need to find consensus on what "better overall" means, but I
believe it needs to be some mix of somewhat better performance overall
while not introducing cases in which elision can make things much worse
(so that we balance improvements in average performance against costs
caused by performance regressions (for the project and for users)).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFC: Always-on elision with per-process opt-in using tunables.
2017-05-12 11:14 ` Stefan Liebler
@ 2017-06-21 7:02 ` Stefan Liebler
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Liebler @ 2017-06-21 7:02 UTC (permalink / raw)
To: libc-alpha
Are there any news for this?
On 05/12/2017 01:13 PM, Stefan Liebler wrote:
> On 05/11/2017 05:45 PM, Carlos O'Donell wrote:
>> I am going to propose the following:
>>
>> * Always build with elision support.
>>
>> * Elision disabled by default at runtime.
>>
>> * Use tunables to allow processes to opt-in to transparent elision.
>>
>> The benefit is that the elision support doesn't bit-rot, and we keep
>> it working, and that distributions can conservatively backport elision
>> and allow users to test enablement on a per-process basis.
> This is an improvement for users as they can simply compare performance
> of their workload with or without lock elsion.
> With (c) - see below - further tuning is possible.
>
>>
>> The elision is enabled with:
>>
>> GLIBC_TUNABLE=glibc.elision.enable=1
>>
>> The obvious set of patches are:
>>
>> (a) Split out some cleanups in this patch.
>> (b) Always build with elision and us tunables to opt-in.
>> (c) Extend tunables to allow modification of elision parameters
>> (useful for upcoming rwlock elision re-enablement).
>>
>> I'm testing this on x86_64, ppc64le, and s390x.
>>
> I've applied this patch to test on s390x.
> Some changes are needed in order to build it.
>
> Please remove the line "enable-lock-elision = @enable_lock_elision"
> in config.make.in. This variable is used in
> sysdeps/unix/sysv/linux/s390/Makefile, which has to be adjusted, too:
> ifeq ($(enable-lock-elision),yes)
> ...
> endif
>
> The s390 configure check to test the availability of __builtin_tbegin
> has to be adjusted:
> diff --git a/sysdeps/s390/configure.ac b/sysdeps/s390/configure.ac
> index 8a782e7..62bcfb9 100644
> --- a/sysdeps/s390/configure.ac
> +++ b/sysdeps/s390/configure.ac
> @@ -32,7 +32,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
>
> Note: __builtin_tbegin is available on RHEL/SLES GCC 4.8.5.
>
>
> Please remove '--enable-lock-elision=yes' in INSTALL, too.
>
> 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!
>
>
>> Thoughts?
>>
>> diff --git a/config.h.in b/config.h.in
>> index 2caa412..47b139e 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -147,9 +147,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 b/configure
>> index 422482f..d3b68c1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -677,7 +677,6 @@ enable_werror
>> all_warnings
>> force_install
>> bindnow
>> -enable_lock_elision
>> hardcoded_path_in_tests
>> enable_timezone_tools
>> use_default_link
>> @@ -766,7 +765,6 @@ enable_profile
>> enable_timezone_tools
>> enable_hardcoded_path_in_tests
>> enable_stackguard_randomization
>> -enable_lock_elision
>> enable_add_ons
>> enable_hidden_plt
>> enable_bind_now
>> @@ -1427,8 +1425,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
>> --enable-add-ons[=DIRS...]
>> configure and build add-ons in
>> DIR1,DIR2,... search
>> for add-ons if no parameter given
>> @@ -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-add-ons was given.
>> if test "${enable_add_ons+set}" = set; then :
>> enableval=$enable_add_ons;
>> diff --git a/configure.ac b/configure.ac
>> index 7f43042..a970296 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...@:>@],
>> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
>> index f33adfb..3dad518 100644
>> --- a/elf/dl-tunables.h
>> +++ b/elf/dl-tunables.h
>> @@ -18,8 +18,8 @@
>> License along with the GNU C Library; if not, see
>> <http://www.gnu.org/licenses/>. */
>>
>> -#ifndef _TUNABLES_H_
>> -#define _TUNABLES_H_
>> +#ifndef _DL_TUNABLES_H_
>> +#define _DL_TUNABLES_H_
>>
>> #if !HAVE_TUNABLES
>> static inline void
>> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
>> index b9f1488..97062f9 100644
>> --- a/elf/dl-tunables.list
>> +++ b/elf/dl-tunables.list
>> @@ -77,4 +77,12 @@ glibc {
>> security_level: SXID_IGNORE
>> }
>> }
>> + elision {
>> + enable {
>> + type: INT_32
>> + minval: 0
>> + maxval: 1
>> + security_level: SXID_IGNORE
>> + }
>> + }
>> }
>> diff --git a/nptl/Makefile b/nptl/Makefile
>> index 6d48c0c..a720612 100644
>> --- a/nptl/Makefile
>> +++ b/nptl/Makefile
>> @@ -704,6 +704,10 @@ $(objpfx)tst-oddstacklimit.out:
>> $(objpfx)tst-oddstacklimit $(objpfx)tst-basic1
>> $(evaluate-test)
>> endif
>>
>> +# 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/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
>> index 601240a..6dba7e1 100644
>> --- a/scripts/gen-tunables.awk
>> +++ b/scripts/gen-tunables.awk
>> @@ -122,9 +122,9 @@ END {
>> }
>>
>> print "/* AUTOGENERATED by gen-tunables.awk. */"
>> - print "#ifndef _TUNABLES_H_"
>> + print "#ifndef _DL_TUNABLES_H_"
>> print "# error \"Do not include this file directly.\""
>> - print "# error \"Include tunables.h instead.\""
>> + print "# error \"Include dl-tunables.h instead.\""
>> print "#endif"
>>
>> # Now, the enum names
>> 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 f92ab2c..0ed83d3 100644
>> --- a/sysdeps/powerpc/powerpc32/sysdep.h
>> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
>> @@ -88,7 +88,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 db7c1d7..79a2697 100644
>> --- a/sysdeps/powerpc/powerpc64/sysdep.h
>> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
>> @@ -272,7 +272,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/s390/nptl/bits/pthreadtypes.h
>> b/sysdeps/s390/nptl/bits/pthreadtypes.h
>> index 48ffdb4..88d21f0 100644
>> --- a/sysdeps/s390/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/s390/nptl/bits/pthreadtypes.h
>> @@ -89,37 +89,25 @@ typedef union
>> binary compatibility with static initializers. */
>> int __kind;
>> #if __WORDSIZE == 64
>> -# ifdef ENABLE_LOCK_ELISION
>> short __spins;
>> short __elision;
>> /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
>> -# define __PTHREAD_SPINS 0, 0
>> -# else
>> - int __spins;
>> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
>> -# define __PTHREAD_SPINS 0
>> -# endif
>> +# define __PTHREAD_SPINS 0, 0
>> __pthread_list_t __list;
>> # define __PTHREAD_MUTEX_HAVE_PREV 1
>> #else
>> unsigned int __nusers;
>> __extension__ union
>> {
>> -# ifdef ENABLE_LOCK_ELISION
>> struct
>> {
>> short __espins;
>> short __elision;
>> } _d;
>> -# define __spins _d.__espins
>> -# define __elision _d.__elision
>> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
>> -# define __PTHREAD_SPINS { 0, 0 }
>> -# else
>> - int __spins;
>> - /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */
>> -# define __PTHREAD_SPINS 0
>> -# endif
>> +# define __spins _d.__espins
>> +# define __elision _d.__elision
>> + /* Mutex __spins initializer used by
>> PTHREAD_MUTEX_INITIALIZER. */
>> +# define __PTHREAD_SPINS { 0, 0 }
>> __pthread_slist_t __list;
>> };
>> #endif
> Adhemerval's commit "Move shared pthread definitions to common headers"
> consolidates the pthradtypes.h files.
> Thus sysdeps/s390/nptl/bits/pthreadtypes.h does not exist anymore.
> I've adjusted sysdeps/s390/nptl/bits/pthreadtypes-arch.h for my test:
> diff --git a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
> b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h
> index 3a9ac57..db7195a 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 __LOCK_ALIGNMENT
> #define __ONCE_ALIGNMENT
>
>
>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> index f631f0a..a767a0c 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> @@ -21,6 +21,7 @@
>> #include <elision-conf.h>
>> #include <unistd.h>
>> #include <dl-procinfo.h>
>> +#include <elf/dl-tunables.h>
>>
>> /* Reasonable initial tuning values, may be revised in the future.
>> This is a conservative initial value. */
>> @@ -54,18 +55,47 @@ int __pthread_force_elision attribute_hidden;
>>
>> /* Initialize elision. */
>>
>> +static inline void
>> +do_set_elision_enable (bool elision_enable)
>> +{
>> + /* If elision is not enabled, or we are in a secure mode,
>> + make sure elision is never used... */
>> + if (!elision_enable || __libc_enable_secure)
>> + {
>> + __pthread_force_elision = 0;
>> + return;
>> + }
>> +
>> + /* ... otherwise enable elision based on hardare availability. */
>> + __pthread_force_elision = (GLRO (dl_hwcap2)
>> + & PPC_FEATURE2_HAS_HTM) ? 1 : 0;
>> +}
>> +
>> +#if HAVE_TUNABLES
>> +/* The elision->enable tunable is either 0 or 1 to indicate that
>> + elision should be disabled or enabled respectively. Availability at
>> + the hardware level will still dictate if the feature is used. */
>> +void
>> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
> tunable_val_t *valp instead of tunable_val_t *elision_enable.
>
>> +{
>> + int32_t elision_enable = (int32_t) valp->numval;
>> + do_set_elision_enable (elision_enable ? true : false);
>> +}
>> +#endif
>> +
>> static void
>> 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 to enable and tune elision at
>> runtime. */
>> +# define TUNABLE_NAMESPACE elision
>> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
>> +#else
>> + /* Disable elision when built without tunables. */
>> + do_set_elision_enable (false);
>> #endif
>> - if (!__pthread_force_elision)
>> - /* Disable elision on rwlocks. */
>> - __elision_aconf.try_tbegin = 0;
>> }
>>
>> #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..c6d9962 100644
>> --- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
>> +++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
>> @@ -21,6 +21,7 @@
>> #include <elision-conf.h>
>> #include <unistd.h>
>> #include <dl-procinfo.h>
>> +#include <elf/dl-tunables.h>
>>
>> /* Reasonable initial tuning values, may be revised in the future.
>> This is a conservative initial value. */
>> @@ -55,16 +56,46 @@ int __pthread_force_elision attribute_hidden = 0;
>>
>> /* Initialize elison. */
>>
>> +static inline void
>> +do_set_elision_enable (bool elision_enable)
>> +{
>> + /* If elision is not enabled, or we are in a secure mode,
>> + make sure elision is never used... */
>> + if (!elision_enable || __libc_enable_secure)
>> + {
>> + __pthread_force_elision = 0;
>> + return;
>> + }
>> +
>> + /* ... otherwise enable elision based on hardare availability. */
>> + __pthread_force_elision = (GLRO (dl_hwcap) & HWCAP_S390_TE) ? 1 : 0;
>> +}
>> +
>> +#if HAVE_TUNABLES
>> +/* The elision->enable tunable is either 0 or 1 to indicate that
>> + elision should be disabled or enabled respectively. Availability at
>> + the hardware level will still dictate if the feature is used. */
>> +void
>> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *elision_enable)
> tunable_val_t *valp instead of tunable_val_t *elision_enable.
>
>> +{
>> + int32_t elision_enable = (int32_t) valp->numval;
>> + do_set_elision_enable (elision_enable ? true : false);
>> +}
>> +#endif
>> +
>> static void
>> 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;
>> -
>> - __pthread_force_elision = __libc_enable_secure ? 0 :
>> elision_available;
>> +#if HAVE_TUNABLES
>> + /* Elision depends on tunables to enable and tune elision at
>> runtime. */
>> +# define TUNABLE_NAMESPACE elision
>> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
>> +#else
>> + /* Disable elision when built without tunables. */
>> + do_set_elision_enable (false);
>> +#endif
>> }
>>
>> #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) \
> Can you adjust the occurences of "# define" within the old #ifdef
> ENABLE_LOCK_ELISON block to "# define".
>
>> __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..7d3d8fd 100644
>> --- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
>> +++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
>> @@ -21,6 +21,7 @@
>> #include <init-arch.h>
>> #include <elision-conf.h>
>> #include <unistd.h>
>> +#include <elf/dl-tunables.h>
>>
>> /* Reasonable initial tuning values, may be revised in the future.
>> This is a conservative initial value. */
>> @@ -52,17 +53,46 @@ int __pthread_force_elision attribute_hidden;
>>
>> /* Initialize elison. */
>>
>> +static inline void
>> +do_set_elision_enable (bool elision_enable)
>> +{
>> + /* If elision is not enabled, or we are in a secure mode,
>> + make sure elision is never used... */
>> + if (!elision_enable || __libc_enable_secure)
>> + {
>> + __pthread_force_elision = 0;
>> + return;
>> + }
>> +
>> + /* ... otherwise enable elision based on hardare availability. */
>> + __pthread_force_elision = HAS_CPU_FEATURE (RTM);
>> +}
>> +
>> +#if HAVE_TUNABLES
>> +/* The elision->enable tunable is either 0 or 1 to indicate that
>> + elision should be disabled or enabled respectively. Availability at
>> + the hardware level will still dictate if the feature is used. */
>> +void
>> +DL_TUNABLE_CALLBACK (set_elision_enable) (tunable_val_t *valp)
>> +{
>> + int32_t elision_enable = (int32_t) valp->numval;
>> + do_set_elision_enable (elision_enable ? true : false);
>> +}
>> +#endif
>> +
>> 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 to enable and tune elision at
>> runtime. */
>> +# define TUNABLE_NAMESPACE elision
>> + TUNABLE_SET_VAL_WITH_CALLBACK (enable, NULL, set_elision_enable);
>> +#else
>> + /* Disable elision when built without tunables. */
>> + do_set_elision_enable (false);
>> #endif
>> - if (!elision_available)
>> - __elision_aconf.retry_try_xbegin = 0; /* Disable elision on
>> rwlocks */
>> }
>>
>> #ifdef SHARED
>> ---
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-21 7:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 15:45 RFC: Always-on elision with per-process opt-in using tunables Carlos O'Donell
2017-05-12 11:14 ` Stefan Liebler
2017-06-21 7:02 ` Stefan Liebler
2017-05-15 19:59 ` Tulio Magno Quites Machado Filho
2017-05-30 6:46 ` Torvald Riegel
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).