| Message ID | 1467479562-11357-3-git-send-email-siddhesh@sourceware.org |
|---|---|
| State | Superseded |
| Headers | show |
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 >
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.
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
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
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];