diff mbox

[2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable

Message ID 56941E13.4040306@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers

Commit Message

Paul E. Murphy Jan. 11, 2016, 9:26 p.m. UTC 
  I ran into a few minor issues while testing this, noted below.

On 01/11/2016 05:17 AM, Siddhesh Poyarekar wrote:
> Read tunables values from the users using the GLIBC_TUNABLES
> environment variable.  The value of this variable is a colon-separated
> list of name=value pairs.  So a typical string would look like this:
> 
> GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024
> 
> 	* tunables/tunables.c: Include sys/mman.h and libc-internals.h.
> 	(GLIBC_TUNABLES): New macro.
> 	(t_strdup): New function.
> 	(__tunables_init): Implement initializer.
> ---
>  tunables/tunables.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 98 insertions(+), 5 deletions(-)
> 
> diff --git a/tunables/tunables.c b/tunables/tunables.c
> index 2ae1050..ddd1934 100644
> --- a/tunables/tunables.c
> +++ b/tunables/tunables.c
> @@ -24,12 +24,16 @@
>  #include <stdbool.h>
>  #include <unistd.h>
>  #include <sys/param.h>
> +#include <sys/mman.h>
> +#include <libc-internal.h>
> 
>  extern char **__environ;
> 
>  #define TUNABLES_INTERNAL 1
>  #include "tunables.h"
> 
> +#define GLIBC_TUNABLES "GLIBC_TUNABLES"

Should this be "GLIBC_TUNABLES=" to prevent matching a bogus prefix?


> +
> +  while (get_next_env (&evp, &envname, &envnamelen, &envval))
> +    {
> +      if (!t_strncmp (envname, GLIBC_TUNABLES, sizeof (GLIBC_TUNABLES)))

"sizeof (GLIBC_TUNABLES) - 1"?


With those changes, I was able to use the dynamic TLE patch. Attached is my
updated dynamic TLE patch for those interested. It did not require much
change.

Paul

Comments

Siddhesh Poyarekar Jan. 12, 2016, 12:28 p.m. UTC | #1 
On Mon, Jan 11, 2016 at 03:26:43PM -0600, Paul E. Murphy wrote:
> > +#define GLIBC_TUNABLES "GLIBC_TUNABLES"
> 
> Should this be "GLIBC_TUNABLES=" to prevent matching a bogus prefix?
> 

Thanks for pointing out the problem.  I'm fixing it differently
though, by implementing the string comparison as strcmp instead, so
that it returns a mismatch if string lengths are different.  I chose
this approach because the other uses of t_strncmp also had the same
problem you pointed out.

Siddhesh
diff mbox

Patch

From 51967c99dbcea3e13bface4c6f13c1cfa3cb5709 Mon Sep 17 00:00:00 2001
From: "Paul E. Murphy" <murphyp@linux.vnet.ibm.com>
Date: Wed, 23 Sep 2015 15:16:41 -0500
Subject: [PATCH] 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.

The tunable names are slightly munged to remove architecture
specific terms.  Otherwise, the elision implementation is
roughly identical on all supported platforms.

2016-01-11  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* nptl/elision-tunables.c: New file.
	* sysdeps/unix/sysv/linux/powerpc/elision-conf.c:
	(ELISION_ENABLE): New macro.
	(ELISION_SKIP_LOCK_BUSY): Likewise.
	(ELISION_SKIP_LOCK_INTERNAL_ABORT): Likewise.
	(ELISION_SKIP_LOCK_AFTER_RETRIES): Likewise.
	(ELISION_TRIES): Likewise.
	(ELISION_TRYLOCK_INTERNAL_ABORT): Likewise.
	(ELISION_CAN_ENABLE): Likewise.
	(elision_init): Add hook to tunable function.
	* sysdeps/unix/sysv/linux/s390/elision-conf.c:
	Likewise.
	* sysdeps/unix/sysv/linux/x86/elision-conf.c:
	(ELISION_ENABLE): New macro.
	(ELISION_SKIP_LOCK_BUSY): Likewise.
	(ELISION_SKIP_LOCK_INTERNAL_ABORT): Likewise.
	(ELISION_TRIES): Likewise.
	(ELISION_TRYLOCK_INTERNAL_ABORT): Likewise.
	(ELISION_CAN_ENABLE): Likewise.
	(elision_init): Add hook to tunable function.
	* tunables/tunable-list.h (tunable_register): Regenerate.
	* tunables/tunable.list: Add elision parameters.
---
 nptl/elision-tunables.c                        | 82 ++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/powerpc/elision-conf.c | 18 +++++-
 sysdeps/unix/sysv/linux/s390/elision-conf.c    | 19 +++++-
 sysdeps/unix/sysv/linux/x86/elision-conf.c     | 12 ++++
 tunables/tunable-list.h                        | 30 +++++++---
 tunables/tunables.list                         |  8 +++
 6 files changed, 157 insertions(+), 12 deletions(-)
 create mode 100644 nptl/elision-tunables.c

diff --git a/nptl/elision-tunables.c b/nptl/elision-tunables.c
new file mode 100644
index 0000000..efaabdd
--- /dev/null
+++ b/nptl/elision-tunables.c
@@ -0,0 +1,82 @@ 
+/* Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#if BUILD_TUNABLES
+
+#define TUNABLE_NAMESPACE pthread
+#include <tunables/tunables.h>
+
+/* Define a function to parse an integral tunable value and
+   set the pointer this value.  */
+#define _ELISION_TUNABLE_INT_FUNC(tunable, ptr) \
+  void \
+  _elision_set_ ## tunable (const char * value) \
+  { \
+    *ptr = atoi (value); \
+  }
+
+/* Define some helper macros to register a tunable, and
+   define the setter function.  Note, this defines a
+   nested function, and the tunable macro likely resolves
+   to a function call.  */
+#define _ELISION_TUNABLE_REGISTER(name, mname) \
+  _ELISION_TUNABLE_INT_FUNC (name, mname); \
+  TUNABLE_REGISTER (pthread, elision_ ## name, \
+  			     &_elision_set_ ## name)
+
+/* Initialize the tunables based on what the platform has defined as
+   being available.  */
+static void __attribute__((unused))
+elision_init_tunables (char **envp)
+{
+  /* Don't attempt to do anything if elision isn't supported */
+  if (!ELISION_CAN_ENABLE || __libc_enable_secure)
+    return;
+
+  _ELISION_TUNABLE_REGISTER (enable, ELISION_ENABLE);
+
+#ifdef ELISION_SKIP_LOCK_BUSY
+  _ELISION_TUNABLE_REGISTER (skip_lock_busy, ELISION_SKIP_LOCK_BUSY);
+#endif
+
+#ifdef ELISION_SKIP_LOCK_INTERNAL_ABORT
+  _ELISION_TUNABLE_REGISTER (skip_lock_internal_abort,
+  			     ELISION_SKIP_LOCK_INTERNAL_ABORT);
+#endif
+
+#ifdef ELISION_SKIP_LOCK_AFTER_RETRIES
+  _ELISION_TUNABLE_REGISTER (skip_lock_after_retries,
+  			     ELISION_SKIP_LOCK_AFTER_RETRIES);
+#endif
+
+#ifdef ELISION_TRIES
+  _ELISION_TUNABLE_REGISTER (tries, ELISION_TRIES);
+#endif
+
+#ifdef ELISION_SKIP_TRYLOCK_INTERNAL_ABORT
+  _ELISION_TUNABLE_REGISTER (skip_trylock_internal_abort,
+  			     ELISION_SKIP_TRYLOCK_INTERNAL_ABORT);
+#endif
+
+  tunables_init (envp);
+}
+
+#else
+
+#define elision_init_tunables(x)
+
+#endif /* BUILD_TUNABLES */
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
index 21c3afd..b5249c3 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
@@ -22,6 +22,19 @@ 
 #include <unistd.h>
 #include <dl-procinfo.h>
 
+#define ELISION_ENABLE (&__pthread_force_elision)
+#define ELISION_SKIP_LOCK_BUSY (&__elision_aconf.skip_lock_busy)
+#define ELISION_SKIP_LOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_lock_internal_abort)
+#define ELISION_SKIP_LOCK_AFTER_RETRIES \
+  (&__elision_aconf.skip_lock_out_of_tbegin_retries)
+#define ELISION_TRIES (&__elision_aconf.try_tbegin)
+#define ELISION_SKIP_TRYLOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_trylock_internal_abort)
+#define ELISION_CAN_ENABLE ((GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM))
+
+#include "elision-tunables.c"
+
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
 
@@ -60,8 +73,9 @@  elision_init (int argc __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;
+  /* Note, elision must be explicitly turned on by setting the
+     appropriate tunable on a supported platform.  */
+  elision_init_tunables (environ);
 #endif
   if (!__pthread_force_elision)
     /* Disable elision on rwlocks.  */
diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.c b/sysdeps/unix/sysv/linux/s390/elision-conf.c
index 4441fd9..aa2b2de 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -22,6 +22,19 @@ 
 #include <unistd.h>
 #include <dl-procinfo.h>
 
+#define ELISION_ENABLE (&__pthread_force_elision)
+#define ELISION_SKIP_LOCK_BUSY (&__elision_aconf.skip_lock_busy)
+#define ELISION_SKIP_LOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_lock_internal_abort)
+#define ELISION_SKIP_LOCK_AFTER_RETRIES \
+  (&__elision_aconf.skip_lock_out_of_tbegin_retries)
+#define ELISION_TRIES (&__elision_aconf.try_tbegin)
+#define ELISION_SKIP_TRYLOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_trylock_internal_abort)
+#define ELISION_CAN_ENABLE (GLRO (dl_hwcap) & HWCAP_S390_TE)
+
+#include "elision-tunables.c"
+
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
 
@@ -62,9 +75,13 @@  elision_init (int argc __attribute__ ((unused)),
 {
   /* 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;
+  int elision_available = ELISION_SHOULD_ENABLE ? 1 : 0;
 
+  /* Enable elision, if available, by default.  */
   __pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
+
+  /* Update any tunables as desired.  */
+  elision_init_tunables (environ);
 }
 
 #ifdef SHARED
diff --git a/sysdeps/unix/sysv/linux/x86/elision-conf.c b/sysdeps/unix/sysv/linux/x86/elision-conf.c
index 0d98133..b307db8 100644
--- a/sysdeps/unix/sysv/linux/x86/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/x86/elision-conf.c
@@ -22,6 +22,17 @@ 
 #include <elision-conf.h>
 #include <unistd.h>
 
+#define ELISION_ENABLE (&__pthread_force_elision)
+#define ELISION_SKIP_LOCK_BUSY (&__elision_aconf.skip_lock_busy)
+#define ELISION_SKIP_LOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_lock_internal_abort)
+#define ELISION_TRIES (&__elision_aconf.retry_try_xbegin)
+#define ELISION_SKIP_TRYLOCK_INTERNAL_ABORT \
+  (&__elision_aconf.skip_trylock_internal_abort)
+#define ELISION_CAN_ENABLE (HAS_CPU_FEATURE (RTM))
+
+#include "elision-tunables.c"
+
 /* Reasonable initial tuning values, may be revised in the future.
    This is a conservative initial value.  */
 
@@ -65,6 +76,7 @@  elision_init (int argc __attribute__ ((unused)),
   __elision_available = HAS_CPU_FEATURE (RTM);
 #ifdef ENABLE_LOCK_ELISION
   __pthread_force_elision = __libc_enable_secure ? 0 : __elision_available;
+  elision_init_tunables (environ);
 #endif
   if (!HAS_CPU_FEATURE (RTM))
     __elision_aconf.retry_try_xbegin = 0; /* Disable elision on rwlocks */
diff --git a/tunables/tunable-list.h b/tunables/tunable-list.h
index 2b1caa4..08b07a6 100644
--- a/tunables/tunable-list.h
+++ b/tunables/tunable-list.h
@@ -8,25 +8,37 @@ 
 
 typedef enum
 {
-  TUNABLE_ENUM_NAME(glibc, malloc, check),
-  TUNABLE_ENUM_NAME(glibc, malloc, top_pad),
-  TUNABLE_ENUM_NAME(glibc, malloc, perturb),
-  TUNABLE_ENUM_NAME(glibc, malloc, mmap_threshold),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_tries),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_skip_trylock_internal_abort),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_enable),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_skip_lock_busy),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_skip_lock_internal_abort),
+  TUNABLE_ENUM_NAME(glibc, pthread, elision_skip_lock_after_retries),
   TUNABLE_ENUM_NAME(glibc, malloc, trim_threshold),
   TUNABLE_ENUM_NAME(glibc, malloc, mmap_max),
   TUNABLE_ENUM_NAME(glibc, malloc, arena_max),
   TUNABLE_ENUM_NAME(glibc, malloc, arena_test),
+  TUNABLE_ENUM_NAME(glibc, malloc, check),
+  TUNABLE_ENUM_NAME(glibc, malloc, top_pad),
+  TUNABLE_ENUM_NAME(glibc, malloc, perturb),
+  TUNABLE_ENUM_NAME(glibc, malloc, mmap_threshold),
 } tunable_id_t;
 
 #ifdef TUNABLES_INTERNAL
 static tunable_t tunable_list[] = {
-  {TUNABLE_NAME_S(glibc, malloc, check), NULL, NULL, false},
-  {TUNABLE_NAME_S(glibc, malloc, top_pad), NULL, NULL, false},
-  {TUNABLE_NAME_S(glibc, malloc, perturb), NULL, NULL, false},
-  {TUNABLE_NAME_S(glibc, malloc, mmap_threshold), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_tries), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_skip_trylock_internal_abort), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_enable), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_skip_lock_busy), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_skip_lock_internal_abort), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, pthread, elision_skip_lock_after_retries), NULL, NULL, false},
   {TUNABLE_NAME_S(glibc, malloc, trim_threshold), NULL, NULL, false},
   {TUNABLE_NAME_S(glibc, malloc, mmap_max), NULL, NULL, false},
   {TUNABLE_NAME_S(glibc, malloc, arena_max), NULL, NULL, false},
   {TUNABLE_NAME_S(glibc, malloc, arena_test), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, malloc, check), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, malloc, top_pad), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, malloc, perturb), NULL, NULL, false},
+  {TUNABLE_NAME_S(glibc, malloc, mmap_threshold), NULL, NULL, false},
 };
-#endif
+#endif
\ No newline at end of file
diff --git a/tunables/tunables.list b/tunables/tunables.list
index f1335cc..d7a677b 100644
--- a/tunables/tunables.list
+++ b/tunables/tunables.list
@@ -9,4 +9,12 @@  glibc {
     arena_max
     arena_test
   }
+  pthread {
+    elision_enable
+    elision_skip_lock_busy
+    elision_skip_lock_internal_abort
+    elision_skip_lock_after_retries
+    elision_tries
+    elision_skip_trylock_internal_abort
+  }
 }
-- 
1.8.3.1