* [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable @ 2016-01-11 11:17 Siddhesh Poyarekar 2016-01-11 13:51 ` Andreas Schwab 2016-01-11 21:26 ` Paul E. Murphy 0 siblings, 2 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-01-11 11:17 UTC (permalink / raw) To: libc-alpha; +Cc: roland, carlos, Paul E. Murphy, Andi Kleen 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" + static int t_strncmp (const char *a, const char *b, size_t len) { @@ -46,6 +50,31 @@ t_strncmp (const char *a, const char *b, size_t len) return 0; } +static char * +t_strdup (const char *in) +{ + size_t len = 0; + + while (in[len] != '\0') + len++; + + /* Allocate enough number of pages. Given the number of tunables this should + not exceed a single page but we err on the conservative side and try to + allocate space as needed. */ + size_t alloclen = ALIGN_UP (len + 1, __getpagesize ()); + + char *out = __mmap (NULL, alloclen, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); + + if (__glibc_unlikely (out == MAP_FAILED)) + return NULL; + else + { + memcpy (out, in, len); + return out; + } +} + static bool get_next_env (char ***envp, char **name, size_t *namelen, char **val) { @@ -74,14 +103,78 @@ get_next_env (char ***envp, char **name, size_t *namelen, char **val) return false; } -/* This is where tunables will be read in from either an environment variable, - a set of environment variables or some other source and then initialized. - Caller should pass it the environment variable; __environ may not be - reliable if it is called earlier than libc.so initialization. */ +/* Initialize tunables from the GLIBC_TUNABLES environment variable. The + variable is set as colon separated name=value pairs. */ void __tunables_init (char **envp) { - /* Empty for now. */ + static bool initialized = false; + + if (__glibc_likely (initialized)) + return; + + char **evp = envp; + char *p = NULL; + + char *envname; + size_t envnamelen; + char *envval; + + while (get_next_env (&evp, &envname, &envnamelen, &envval)) + { + if (!t_strncmp (envname, GLIBC_TUNABLES, sizeof (GLIBC_TUNABLES))) + { + p = t_strdup (envval); + break; + } + } + + if (p == NULL || *p == '\0') + goto out; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + goto out; + + p[len] = '\0'; + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + if (t_strncmp (name, tunable_list[i].name, SIZE_MAX) == 0) + { + tunable_list[i].val = value; + break; + } + } + + if (end == ':') + p += len + 1; + else + goto out; + } +out: + initialized = true; } strong_alias (__tunables_init, tunables_init) -- 2.5.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-11 11:17 [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar @ 2016-01-11 13:51 ` Andreas Schwab 2016-01-11 14:45 ` Siddhesh Poyarekar 2016-01-11 21:26 ` Paul E. Murphy 1 sibling, 1 reply; 17+ messages in thread From: Andreas Schwab @ 2016-01-11 13:51 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, roland, carlos, Paul E. Murphy, Andi Kleen Siddhesh Poyarekar <sid@reserved-bit.com> writes: > void > __tunables_init (char **envp) > { > - /* Empty for now. */ > + static bool initialized = false; > + > + if (__glibc_likely (initialized)) > + return; Is this supposed to be thread-safe? Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-11 13:51 ` Andreas Schwab @ 2016-01-11 14:45 ` Siddhesh Poyarekar 2016-01-13 2:44 ` Carlos O'Donell 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-01-11 14:45 UTC (permalink / raw) To: Andreas Schwab; +Cc: libc-alpha, roland, carlos, Paul E. Murphy, Andi Kleen On Mon, Jan 11, 2016 at 02:51:36PM +0100, Andreas Schwab wrote: > Siddhesh Poyarekar <sid@reserved-bit.com> writes: > > > void > > __tunables_init (char **envp) > > { > > - /* Empty for now. */ > > + static bool initialized = false; > > + > > + if (__glibc_likely (initialized)) > > + return; > > Is this supposed to be thread-safe? This is called only from the libc.so and libpthread.so constructors. So the first run will always happen exclusively in the main thread through either library constructor. Even in case the constructors do get called in parallel in different threads, they should get synchronized by the dynamic linker load lock, so you'd never have concurrent calls to __tunables_init that race on the value of initialized. Siddhesh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-11 14:45 ` Siddhesh Poyarekar @ 2016-01-13 2:44 ` Carlos O'Donell 2016-01-13 3:27 ` Siddhesh Poyarekar 0 siblings, 1 reply; 17+ messages in thread From: Carlos O'Donell @ 2016-01-13 2:44 UTC (permalink / raw) To: Siddhesh Poyarekar, Andreas Schwab Cc: libc-alpha, roland, Paul E. Murphy, Andi Kleen On 01/11/2016 09:45 AM, Siddhesh Poyarekar wrote: > On Mon, Jan 11, 2016 at 02:51:36PM +0100, Andreas Schwab wrote: >> Siddhesh Poyarekar <sid@reserved-bit.com> writes: >> >>> void >>> __tunables_init (char **envp) >>> { >>> - /* Empty for now. */ >>> + static bool initialized = false; >>> + >>> + if (__glibc_likely (initialized)) >>> + return; >> >> Is this supposed to be thread-safe? > > This is called only from the libc.so and libpthread.so constructors. > So the first run will always happen exclusively in the main thread > through either library constructor. Not true if you link statically and dlopen, at which point you could have multiple threads, and you call dlopen which loads libc.so/libpthread.so's constructors? You can write a simple static test case that creates N threads and calls dlopen on some DSO to test this. > Even in case the constructors do get called in parallel in different > threads, they should get synchronized by the dynamic linker load lock, > so you'd never have concurrent calls to __tunables_init that race on > the value of initialized. Please include a concurrency note there then, that this is protected by the load lock? Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-13 2:44 ` Carlos O'Donell @ 2016-01-13 3:27 ` Siddhesh Poyarekar 2016-01-13 8:38 ` Andreas Schwab 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-01-13 3:27 UTC (permalink / raw) To: Carlos O'Donell Cc: Andreas Schwab, libc-alpha, roland, Paul E. Murphy, Andi Kleen On Tue, Jan 12, 2016 at 09:44:51PM -0500, Carlos O'Donell wrote: > Not true if you link statically and dlopen, at which point you could > have multiple threads, and you call dlopen which loads > libc.so/libpthread.so's constructors? A single load (and constructor call) does not matter since there's nothing to race against. The case of concurrent constructor calls I mentioned in the following paragraph; the load lock is sufficient to protect it. > > Even in case the constructors do get called in parallel in different > > threads, they should get synchronized by the dynamic linker load lock, > > so you'd never have concurrent calls to __tunables_init that race on > > the value of initialized. > > Please include a concurrency note there then, that this is protected > by the load lock? OK. Thanks, Siddhesh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-13 3:27 ` Siddhesh Poyarekar @ 2016-01-13 8:38 ` Andreas Schwab 2016-01-13 15:37 ` Carlos O'Donell 0 siblings, 1 reply; 17+ messages in thread From: Andreas Schwab @ 2016-01-13 8:38 UTC (permalink / raw) To: Siddhesh Poyarekar Cc: Carlos O'Donell, libc-alpha, roland, Paul E. Murphy, Andi Kleen Siddhesh Poyarekar <sid@reserved-bit.com> writes: > A single load (and constructor call) does not matter since there's > nothing to race against. The case of concurrent constructor calls I > mentioned in the following paragraph; the load lock is sufficient to > protect it. PR19448 is about removing that lock. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-13 8:38 ` Andreas Schwab @ 2016-01-13 15:37 ` Carlos O'Donell 0 siblings, 0 replies; 17+ messages in thread From: Carlos O'Donell @ 2016-01-13 15:37 UTC (permalink / raw) To: Andreas Schwab, Siddhesh Poyarekar Cc: libc-alpha, roland, Paul E. Murphy, Andi Kleen On 01/13/2016 03:38 AM, Andreas Schwab wrote: > Siddhesh Poyarekar <sid@reserved-bit.com> writes: > >> A single load (and constructor call) does not matter since there's >> nothing to race against. The case of concurrent constructor calls I >> mentioned in the following paragraph; the load lock is sufficient to >> protect it. > > PR19448 is about removing that lock. If the code has comments that it needs protection from load lock then such code will be reviewed as we fix BZ19448. Which is why I suggested comments to that extent. The removal of the load lock won't be easy and will require quite a bit of review. Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-11 11:17 [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-01-11 13:51 ` Andreas Schwab @ 2016-01-11 21:26 ` Paul E. Murphy 2016-01-12 12:28 ` Siddhesh Poyarekar 1 sibling, 1 reply; 17+ messages in thread From: Paul E. Murphy @ 2016-01-11 21:26 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: roland, carlos, Andi Kleen [-- Attachment #1: Type: text/plain, Size: 1580 bytes --] 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 [-- Attachment #2: 0001-Add-elision-tunables.patch --] [-- Type: text/x-patch, Size: 12007 bytes --] 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-01-11 21:26 ` Paul E. Murphy @ 2016-01-12 12:28 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-01-12 12:28 UTC (permalink / raw) To: Paul E. Murphy; +Cc: libc-alpha, roland, carlos, Andi Kleen 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/2] Tunables for glibc @ 2016-07-01 18:31 Siddhesh Poyarekar 2016-07-01 18:31 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-01 18:31 UTC (permalink / raw) To: libc-alpha Hi, Here's another swipe at the tunables. As usual, I'll start with an apology for the very slow turnaround time and especially my tendency to come to this around freeze time. This version takes a completely different approach for a couple of reasons and ends up making the tunables interface simpler to use. Firstly, the tunables are now much richer as Florian requested. A tunable now has a data type associated with it, with the ability to attach attributes like type, minimum and maximum values to it as well as calling a callback function. Additionally, I got rid of the compatibility interface and included that in the tunable structure since that is much easier to deal with. Finally, I've moved the tunables out of the tunables directory and into elf/. This is because I had overlooked the fact that if we had to control ifuncs using tunables, they would have to be initialized much earlier. They are now initialized along with the LD_* environment variables. In fact, it might not be a bad idea to move the LD_* variables under the tunables mechanism as well. Like last time, the first patch adds a framework for tunables, moving the old environment variables into the tunables infrastructure and reads them all in early. The module (malloc is the example in this case) only has to get the initialized value (if it has been initialized that is) and optionally call a function to do additional work based on whether the value has been set via the envvar. Patch 2 then introduces the GLIBC_TUNABLES environment variable, which can be set to a colon-separated list of name=value pairs of tunables. This is the contentious bit because there isn't agreement yet on what the final interface should look like. Siddhesh Poyarekar (2): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable INSTALL | 6 ++ Makeconfig | 16 ++++ README.tunables | 74 +++++++++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 ++++ configure.ac | 10 ++ elf/Makefile | 5 + elf/Versions | 3 + elf/dl-tunable-types.h | 45 +++++++++ elf/dl-tunables.c | 241 +++++++++++++++++++++++++++++++++++++++++++++++ elf/dl-tunables.h | 76 +++++++++++++++ elf/dl-tunables.list | 50 ++++++++++ elf/rtld.c | 8 ++ malloc/arena.c | 28 ++++++ manual/install.texi | 5 + scripts/gen-tunables.awk | 157 ++++++++++++++++++++++++++++++ 17 files changed, 744 insertions(+) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 scripts/gen-tunables.awk -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-01 18:31 [PATCH 0/2] Tunables for glibc Siddhesh Poyarekar @ 2016-07-01 18:31 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-01 18:31 UTC (permalink / raw) To: libc-alpha 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 * elf/dl-tunables.c: Include string.h. (parse_tunables): New function. (GLIBC_TUNABLES): New macro. (__tunables_init): Use it. --- elf/dl-tunables.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 8a26dfc..04d1d52 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -22,10 +22,13 @@ #include <stdbool.h> #include <unistd.h> #include <stdlib.h> +#include <string.h> #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" +#define GLIBC_TUNABLES "GLIBC_TUNABLES" + /* Compare environment names, bounded by the name hardcoded in glibc. */ static bool is_name (const char *orig, const char *envname) @@ -104,6 +107,66 @@ tunable_initialize (tunable_t *cur) } } +static void +parse_tunables (char *tunestr) +{ + if (tunestr == NULL || *tunestr == '\0') + return; + + char *p = tunestr; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + return; + + /* We did not find a valid name-value pair before encountering the + colon. */ + if (p[len]== ':') + { + p += len + 1; + continue; + } + + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + if (is_name (tunable_list[i].name, name)) + { + /* Tunable values take precedence over the env_alias. */ + tunable_list[i].strval = value; + tunable_initialize (&tunable_list[i]); + break; + } + } + + if (end == ':') + p += len + 1; + else + return; + } +} + /* Initialize the tunables list from the environment. For now we only use the ENV_ALIAS to find values. Later we will also use the tunable names to find values. */ @@ -116,6 +179,12 @@ __tunables_init (char **envp) while (get_next_env (&envp, &envname, &len, &envval)) { + if (is_name (GLIBC_TUNABLES, envname)) + { + parse_tunables (strdup (envval)); + continue; + } + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/2] tunables for glibc @ 2016-07-02 17:13 Siddhesh Poyarekar 2016-07-02 17:13 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-02 17:13 UTC (permalink / raw) To: libc-alpha Hi, Here's another stab at the tunables patchset. This patch now gets tunables working with static binaries as well. The tunables framework aims to provide a simple and consistent interface for the library to expose tuning switches in a much more coherent manner to userspace. In its initial form, the framework will allow users to tweak certain parts of the library using an environment variable (or set of environment variables, depending on where the consensus sends us). In future, this could be extended to per-user or even systemwide tuning switches. The first patch implements a bare framework that moves the environment variables in the malloc module to the tunables framework without adding any new user-visible functionality. The environment variables that were implemented earlier should work as is. The second patch adds support for a GLIBC_TUNABLES environment variable, which users can use to set up tunables instead of using the old environment variables. The alternative approach to this is to expose namespace-consistent environment variables for each tunable for userspace to take advantage of instead of the earlier environment variables. Siddhesh Poyarekar (2): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable INSTALL | 6 + Makeconfig | 16 +++ README.tunables | 74 ++++++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 +++ configure.ac | 10 ++ csu/init-first.c | 7 ++ elf/Makefile | 5 + elf/Versions | 3 + elf/dl-tunable-types.h | 45 +++++++ elf/dl-tunables.c | 241 ++++++++++++++++++++++++++++++++++++++ elf/dl-tunables.h | 76 ++++++++++++ elf/dl-tunables.list | 50 ++++++++ elf/rtld.c | 8 ++ malloc/Makefile | 3 + malloc/arena.c | 35 ++++++ malloc/tst-malloc-usable-static.c | 1 + manual/install.texi | 5 + scripts/gen-tunables.awk | 157 +++++++++++++++++++++++++ 20 files changed, 762 insertions(+) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 scripts/gen-tunables.awk -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-02 17:13 [PATCH 0/2] tunables for glibc Siddhesh Poyarekar @ 2016-07-02 17:13 ` Siddhesh Poyarekar 2016-07-03 14:39 ` H.J. Lu 2016-07-03 15:53 ` H.J. Lu 0 siblings, 2 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-02 17:13 UTC (permalink / raw) To: libc-alpha 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 * elf/dl-tunables.c: Include string.h. (parse_tunables): New function. (GLIBC_TUNABLES): New macro. (__tunables_init): Use it. --- elf/dl-tunables.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 8a26dfc..04d1d52 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -22,10 +22,13 @@ #include <stdbool.h> #include <unistd.h> #include <stdlib.h> +#include <string.h> #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" +#define GLIBC_TUNABLES "GLIBC_TUNABLES" + /* Compare environment names, bounded by the name hardcoded in glibc. */ static bool is_name (const char *orig, const char *envname) @@ -104,6 +107,66 @@ tunable_initialize (tunable_t *cur) } } +static void +parse_tunables (char *tunestr) +{ + if (tunestr == NULL || *tunestr == '\0') + return; + + char *p = tunestr; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + return; + + /* We did not find a valid name-value pair before encountering the + colon. */ + if (p[len]== ':') + { + p += len + 1; + continue; + } + + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + if (is_name (tunable_list[i].name, name)) + { + /* Tunable values take precedence over the env_alias. */ + tunable_list[i].strval = value; + tunable_initialize (&tunable_list[i]); + break; + } + } + + if (end == ':') + p += len + 1; + else + return; + } +} + /* Initialize the tunables list from the environment. For now we only use the ENV_ALIAS to find values. Later we will also use the tunable names to find values. */ @@ -116,6 +179,12 @@ __tunables_init (char **envp) while (get_next_env (&envp, &envname, &len, &envval)) { + if (is_name (GLIBC_TUNABLES, envname)) + { + parse_tunables (strdup (envval)); + continue; + } + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-02 17:13 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar @ 2016-07-03 14:39 ` H.J. Lu 2016-07-03 17:15 ` Siddhesh Poyarekar 2016-07-03 15:53 ` H.J. Lu 1 sibling, 1 reply; 17+ messages in thread From: H.J. Lu @ 2016-07-03 14:39 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: GNU C Library On Sat, Jul 2, 2016 at 10:12 AM, Siddhesh Poyarekar <siddhesh@sourceware.org> 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 > > * elf/dl-tunables.c: Include string.h. > (parse_tunables): New function. > (GLIBC_TUNABLES): New macro. > (__tunables_init): Use it. > --- > elf/dl-tunables.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 8a26dfc..04d1d52 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -22,10 +22,13 @@ > #include <stdbool.h> > #include <unistd.h> > #include <stdlib.h> > +#include <string.h> > > #define TUNABLES_INTERNAL 1 > #include "dl-tunables.h" > > +#define GLIBC_TUNABLES "GLIBC_TUNABLES" > + > /* Compare environment names, bounded by the name hardcoded in glibc. */ > static bool > is_name (const char *orig, const char *envname) > @@ -104,6 +107,66 @@ tunable_initialize (tunable_t *cur) > } > } > > +static void > +parse_tunables (char *tunestr) > +{ > + if (tunestr == NULL || *tunestr == '\0') > + return; > + > + char *p = tunestr; > + > + while (true) > + { > + char *name = p; > + size_t len = 0; > + > + /* First, find where the name ends. */ > + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') > + len++; > + > + /* If we reach the end of the string before getting a valid name-value > + pair, bail out. */ > + if (p[len] == '\0') > + return; > + > + /* We did not find a valid name-value pair before encountering the > + colon. */ > + if (p[len]== ':') > + { > + p += len + 1; > + continue; > + } > + > + p += len + 1; > + > + char *value = p; > + len = 0; > + > + while (p[len] != ':' && p[len] != '\0') > + len++; > + > + char end = p[len]; > + p[len] = '\0'; > + > + /* Add the tunable if it exists. */ > + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) > + { > + if (is_name (tunable_list[i].name, name)) Can you add the length of name to tunable_list field and check if the length matches first? > + { > + /* Tunable values take precedence over the env_alias. */ > + tunable_list[i].strval = value; > + tunable_initialize (&tunable_list[i]); > + break; > + } > + } > + > + if (end == ':') > + p += len + 1; > + else > + return; > + } > +} > + > /* Initialize the tunables list from the environment. For now we only use the > ENV_ALIAS to find values. Later we will also use the tunable names to find > values. */ > @@ -116,6 +179,12 @@ __tunables_init (char **envp) > > while (get_next_env (&envp, &envname, &len, &envval)) > { > + if (is_name (GLIBC_TUNABLES, envname)) > + { > + parse_tunables (strdup (envval)); > + continue; > + } > + > for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) > { > tunable_t *cur = &tunable_list[i]; > -- > 2.5.5 > -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-03 14:39 ` H.J. Lu @ 2016-07-03 17:15 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-03 17:15 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library On Sun, Jul 03, 2016 at 07:38:54AM -0700, H.J. Lu wrote: > Can you add the length of name to tunable_list field and check > if the length matches first? Similar to passing the length in is_name, can we do this as an addon if we see a significant performance impact? Siddhesh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-02 17:13 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-07-03 14:39 ` H.J. Lu @ 2016-07-03 15:53 ` H.J. Lu 2016-07-03 17:18 ` Siddhesh Poyarekar 1 sibling, 1 reply; 17+ messages in thread From: H.J. Lu @ 2016-07-03 15:53 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: GNU C Library On Sat, Jul 2, 2016 at 10:12 AM, Siddhesh Poyarekar <siddhesh@sourceware.org> 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 > > * elf/dl-tunables.c: Include string.h. > (parse_tunables): New function. > (GLIBC_TUNABLES): New macro. > (__tunables_init): Use it. I didn't see any tests for GLIBC_TUNABLES=glibc.malloc and I don't think it works for static executable: gdb) bt #0 __tunable_set_val (id=glibc_malloc_check, valp=0x6b67d0 <check_action>, callback=0x414460 <_dl_tunable_set_mallopt_check>) at dl-tunables.c:221 #1 0x0000000000413cda in ptmalloc_init () at arena.c:299 #2 0x0000000000414dfe in ptmalloc_init () at hooks.c:29 #3 malloc_hook_ini (sz=66, caller=<optimized out>) at hooks.c:31 #4 0x0000000000418b2a in __strdup ( s=0x7fffffffef66 "glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024") at strdup.c:42 #5 0x0000000000437e01 in __tunables_init (envp=0x7fffffffe088, envp@entry=0x7fffffffdeb8) at dl-tunables.c:184 #6 0x000000000043908b in __libc_init_first (argc=argc@entry=1, argv=argv@entry=0x7fffffffdea8, envp=0x7fffffffdeb8) at ../csu/init-first.c:81 #7 0x00000000004012d5 in generic_start_main (main=0x400550 <main>, argc=1, argv=argv@entry=0x7fffffffdea8, init=init@entry=0x401b20 <__libc_csu_init>, fini=0x401bb0 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0x7fffffffde98) at ../csu/libc-start.c:225 #8 0x00000000004015c7 in __libc_start_main (main=<optimized out>, argc=<optimized out>, argv=0x7fffffffdea8, init=0x401b20 <__libc_csu_init>, fini=0x401bb0 <__libc_csu_fini>, rtld_fini=<optimized out>, stack_end=0x7fffffffde98) at ../sysdeps/x86/libc-start.c:38 ---Type <return> to continue, or q <return> to quit--- #9 0x0000000000400e1a in _start () at ../sysdeps/x86_64/start.S:120 if (is_name (GLIBC_TUNABLES, envname)) { parse_tunables (strdup (envval)); ^^^^^^^^^ Which calls malloc and other library functions. continue; } Since tunable code is called very early, it can't use any C library functions. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-03 15:53 ` H.J. Lu @ 2016-07-03 17:18 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-03 17:18 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library On Sun, Jul 03, 2016 at 08:53:12AM -0700, H.J. Lu wrote: > I didn't see any tests for GLIBC_TUNABLES=glibc.malloc and I don't think > it works for static executable: Hmm, right, I need to fix this. I had manually run the tst-malloc-usable patch with GLIBC_TUNABLES but didn't do it for static case again. I'll fix this up, write those tests and repost. Thanks, Siddhesh ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv3 0/2] tunables for glibc @ 2016-07-09 18:49 Siddhesh Poyarekar 2016-07-09 18:49 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-09 18:49 UTC (permalink / raw) To: libc-alpha Hi, Here is another iteration of the tunables patch with some more recommendations from H. J. Lu incorporated. I have also added the TUNABLE_TYPE_STRING type to allow adding of string types easily. Major changes from the previous version are: - New type TUNABLE_TYPE_STRING - Callback now accepts a pointer which will be used to pass the tunable value - Eliminate the strdup call completely since it won't work for static binaries. Siddhesh Poyarekar (2): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable INSTALL | 6 + Makeconfig | 16 ++ README.tunables | 74 ++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 ++ configure.ac | 10 ++ csu/init-first.c | 7 + elf/Makefile | 5 + elf/Versions | 3 + elf/dl-tunable-types.h | 46 +++++ elf/dl-tunables.c | 275 +++++++++++++++++++++++++++++ elf/dl-tunables.h | 76 ++++++++ elf/dl-tunables.list | 50 ++++++ elf/rtld.c | 8 + malloc/Makefile | 7 +- malloc/arena.c | 35 ++++ malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-static.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + manual/install.texi | 5 + scripts/gen-tunables.awk | 157 ++++++++++++++++ 22 files changed, 802 insertions(+), 1 deletion(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 malloc/tst-malloc-usable-tunables.c create mode 100644 scripts/gen-tunables.awk -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-07-09 18:49 [PATCHv3 0/2] tunables for glibc Siddhesh Poyarekar @ 2016-07-09 18:49 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-07-09 18:49 UTC (permalink / raw) To: libc-alpha 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 * elf/dl-tunables.c: Include mman.h and libc-internals.h. (tunables_strdup): New function. (parse_tunables): New function. (GLIBC_TUNABLES): New macro. (__tunables_init): Use the new functions and macro. * malloc/tst-malloc-usable-tunables.c: New test case. * malloc/tst-malloc-usable-static-tunables.c: New test case. * malloc/Makefile (tests, tests-static): Add tests. --- elf/dl-tunables.c | 93 ++++++++++++++++++++++++++++++ malloc/Makefile | 6 +- malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + 4 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-tunables.c diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index ee8c2f0..8ed66c8 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -22,10 +22,14 @@ #include <stdbool.h> #include <unistd.h> #include <stdlib.h> +#include <sys/mman.h> +#include <libc-internal.h> #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" +#define GLIBC_TUNABLES "GLIBC_TUNABLES" + /* Compare environment names, bounded by the name hardcoded in glibc. */ static bool is_name (const char *orig, const char *envname) @@ -41,6 +45,27 @@ is_name (const char *orig, const char *envname) return false; } +static char *tunables_strdup (const char *in) +{ + size_t i = 0; + + while (in[i++]); + + char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()), + PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, + 0); + + if (out == MAP_FAILED) + return NULL; + + i--; + + while (i-- > 0) + out[i] = in[i]; + + return out; +} + static bool get_next_env (char ***envp, char **name, size_t *namelen, char **val) { @@ -109,6 +134,66 @@ tunable_initialize (tunable_t *cur) } } +static void +parse_tunables (char *tunestr) +{ + if (tunestr == NULL || *tunestr == '\0') + return; + + char *p = tunestr; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + return; + + /* We did not find a valid name-value pair before encountering the + colon. */ + if (p[len]== ':') + { + p += len + 1; + continue; + } + + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + if (is_name (tunable_list[i].name, name)) + { + /* Tunable values take precedence over the env_alias. */ + tunable_list[i].strval = value; + tunable_initialize (&tunable_list[i]); + break; + } + } + + if (end == ':') + p += len + 1; + else + return; + } +} + /* Initialize the tunables list from the environment. For now we only use the ENV_ALIAS to find values. Later we will also use the tunable names to find values. */ @@ -121,6 +206,14 @@ __tunables_init (char **envp) while (get_next_env (&envp, &envname, &len, &envval)) { + if (is_name (GLIBC_TUNABLES, envname)) + { + char *val = tunables_strdup (envval); + if (val != NULL) + parse_tunables (val); + continue; + } + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; diff --git a/malloc/Makefile b/malloc/Makefile index 23ab2f4..ba34da1 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -30,8 +30,8 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-pvalloc tst-memalign tst-mallopt tst-scratch_buffer \ tst-malloc-backtrace tst-malloc-thread-exit \ tst-malloc-thread-fail tst-malloc-fork-deadlock \ - tst-mallocfork2 -tests-static := tst-malloc-usable-static + tst-mallocfork2 tst-malloc-usable-tunables +tests-static := tst-malloc-usable-static tst-malloc-usable-static-tunables tests += $(tests-static) test-srcs = tst-mtrace @@ -141,6 +141,8 @@ endif tst-mcheck-ENV = MALLOC_CHECK_=3 tst-malloc-usable-ENV = MALLOC_CHECK_=3 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) +tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 +tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV) # Uncomment this for test releases. For public releases it is too expensive. #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1 diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-static-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv4 0/2] tunables for glibc @ 2016-08-15 20:05 Siddhesh Poyarekar 2016-08-15 20:05 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 0 siblings, 1 reply; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-08-15 20:05 UTC (permalink / raw) To: libc-alpha; +Cc: carlos, fweimer Hi, After another episode of disappearance, here is an updated iteration of the tunables patch with suggestions from Florian, Carlos and H. J. incorporated. Changes since the last version: - Fixed mallopt behaviour using a set of intermediate functions that the callback and __libc_mallopt use. I did not make them into direct callbacks because the callbacks are built only when tunables is enabled. - Loads of comment and documentation fixes based on Carlos' and Florian's suggestions - Call __init_tunables even earlier to ensure that tunables are initialized before get_cpu_features is called. Siddhesh Poyarekar (2): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable INSTALL | 5 + Makeconfig | 16 ++ README.tunables | 84 +++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 ++ configure.ac | 10 + csu/init-first.c | 7 + elf/Makefile | 5 + elf/Versions | 3 + elf/dl-sysdep.c | 8 + elf/dl-tunable-types.h | 46 +++++ elf/dl-tunables.c | 284 +++++++++++++++++++++++++++++ elf/dl-tunables.h | 78 ++++++++ elf/dl-tunables.list | 69 +++++++ malloc/Makefile | 7 +- malloc/arena.c | 54 ++++++ malloc/malloc.c | 126 +++++++++---- malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-static.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + manual/install.texi | 5 + scripts/gen-tunables.awk | 157 ++++++++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 8 + sysdeps/mach/hurd/i386/init-first.c | 8 + 25 files changed, 968 insertions(+), 35 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 malloc/tst-malloc-usable-tunables.c create mode 100644 scripts/gen-tunables.awk -- 2.7.4 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-08-15 20:05 [PATCHv4 0/2] tunables for glibc Siddhesh Poyarekar @ 2016-08-15 20:05 ` Siddhesh Poyarekar 0 siblings, 0 replies; 17+ messages in thread From: Siddhesh Poyarekar @ 2016-08-15 20:05 UTC (permalink / raw) To: libc-alpha; +Cc: carlos, fweimer 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 * elf/dl-tunables.c: Include mman.h and libc-internals.h. (tunables_strdup): New function. (parse_tunables): New function. (GLIBC_TUNABLES): New macro. (__tunables_init): Use the new functions and macro. * malloc/tst-malloc-usable-tunables.c: New test case. * malloc/tst-malloc-usable-static-tunables.c: New test case. * malloc/Makefile (tests, tests-static): Add tests. --- elf/dl-tunables.c | 99 ++++++++++++++++++++++++++++++ malloc/Makefile | 6 +- malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + 4 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-tunables.c diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index b8f8fc0..ff71e9d 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -22,10 +22,14 @@ #include <stdbool.h> #include <unistd.h> #include <stdlib.h> +#include <sys/mman.h> +#include <libc-internal.h> #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" +#define GLIBC_TUNABLES "GLIBC_TUNABLES" + /* Compare environment names, bounded by the name hardcoded in glibc. */ static bool is_name (const char *orig, const char *envname) @@ -41,6 +45,27 @@ is_name (const char *orig, const char *envname) return false; } +static char *tunables_strdup (const char *in) +{ + size_t i = 0; + + while (in[i++]); + + char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()), + PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, + 0); + + if (out == MAP_FAILED) + return NULL; + + i--; + + while (i-- > 0) + out[i] = in[i]; + + return out; +} + static char ** get_next_env (char **envp, char **name, size_t *namelen, char **val) { @@ -109,6 +134,72 @@ tunable_initialize (tunable_t *cur, const char *strval) } } +static void +parse_tunables (char *tunestr) +{ + if (tunestr == NULL || *tunestr == '\0') + return; + + char *p = tunestr; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + return; + + /* We did not find a valid name-value pair before encountering the + colon. */ + if (p[len]== ':') + { + p += len + 1; + continue; + } + + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + tunable_t *cur = &tunable_list[i]; + + /* If we are in a secure context (AT_SECURE) then ignore the tunable + unless it is explicitly marked as secure. Tunable values take + precendence over their envvar aliases. */ + if (__libc_enable_secure && !cur->is_secure) + continue; + + if (is_name (cur->name, name)) + { + tunable_initialize (cur, value); + break; + } + } + + if (end == ':') + p += len + 1; + else + return; + } +} + /* Initialize the tunables list from the environment. For now we only use the ENV_ALIAS to find values. Later we will also use the tunable names to find values. */ @@ -121,6 +212,14 @@ __tunables_init (char **envp) while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) { + if (is_name (GLIBC_TUNABLES, envname)) + { + char *val = tunables_strdup (envval); + if (val != NULL) + parse_tunables (val); + continue; + } + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; diff --git a/malloc/Makefile b/malloc/Makefile index de9aafe..599ae9d 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -30,8 +30,8 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-pvalloc tst-memalign tst-mallopt tst-scratch_buffer \ tst-malloc-backtrace tst-malloc-thread-exit \ tst-malloc-thread-fail tst-malloc-fork-deadlock \ - tst-mallocfork2 -tests-static := tst-malloc-usable-static + tst-mallocfork2 tst-malloc-usable-tunables +tests-static := tst-malloc-usable-static tst-malloc-usable-static-tunables tests += $(tests-static) test-srcs = tst-mtrace @@ -141,6 +141,8 @@ endif tst-mcheck-ENV = MALLOC_CHECK_=3 tst-malloc-usable-ENV = MALLOC_CHECK_=3 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) +tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 +tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV) # Uncomment this for test releases. For public releases it is too expensive. #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1 diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-static-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> -- 2.7.4 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-08-15 20:05 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-11 11:17 [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-01-11 13:51 ` Andreas Schwab 2016-01-11 14:45 ` Siddhesh Poyarekar 2016-01-13 2:44 ` Carlos O'Donell 2016-01-13 3:27 ` Siddhesh Poyarekar 2016-01-13 8:38 ` Andreas Schwab 2016-01-13 15:37 ` Carlos O'Donell 2016-01-11 21:26 ` Paul E. Murphy 2016-01-12 12:28 ` Siddhesh Poyarekar 2016-07-01 18:31 [PATCH 0/2] Tunables for glibc Siddhesh Poyarekar 2016-07-01 18:31 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-07-02 17:13 [PATCH 0/2] tunables for glibc Siddhesh Poyarekar 2016-07-02 17:13 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-07-03 14:39 ` H.J. Lu 2016-07-03 17:15 ` Siddhesh Poyarekar 2016-07-03 15:53 ` H.J. Lu 2016-07-03 17:18 ` Siddhesh Poyarekar 2016-07-09 18:49 [PATCHv3 0/2] tunables for glibc Siddhesh Poyarekar 2016-07-09 18:49 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-08-15 20:05 [PATCHv4 0/2] tunables for glibc Siddhesh Poyarekar 2016-08-15 20:05 ` [PATCH 2/2] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar
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).