[4/8] elf: Let environment aliases override overridable system-wide tunables
Checks
Commit Message
The environment-variable alias loop in __tunables_init skipped every tunable
whose "initialized" flag was set, which was originally meant only to give
the canonical GLIBC_TUNABLES form precedence over the legacy MALLOC_*
aliases.
Now that the cache also sets "initialized", a legacy alias could no longer
override an *overridable* cache default, even though the canonical
GLIBC_TUNABLES form still could -- an inconsistent and surprising asymmetry.
Track separately the tunables that were set from GLIBC_TUNABLES during this
call and skip only those in the alias loop
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
elf/Makefile | 7 ++++++-
elf/dl-tunables.c | 20 ++++++++++++++++++--
elf/tst-tunconf1.c | 24 ++++++++++++++++++++++++
elf/tst-tunconf1.root/etc/tunables.conf | 9 +++++++++
4 files changed, 57 insertions(+), 3 deletions(-)
Comments
LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> +/* Records tunables that were set from GLIBC_TUNABLES during this call, so
I wonder if it would make sense to have one variable that had the
"origin priority" of the set value, from TUNABLE_UNSET through
TUNABLE_LOCKED, so a single comparison tells us if a given source can
change the value and bump the priority, replacing initialized, locked,
and the new set_by_env?
TUNABLE_UNSET
TUNABLE_GLOBAL
TUNABLE_LEGACY
TUNABLE_ENVIRON
TUNABLE_LOCKED
(something for after the release, as it's purely internal)
> + that a legacy environment-variable alias does not override themi (the
> + canonical GLIBC_TUNABLES form takes precedence over the aliases).
> + A tunable that was set only from the system-wide cache is deliberately not
> + recorded here, so an alias may still override an overridable cache default;
> + a nonoverridable one remains protected by tunable_t::locked. */
> +static bool tunable_set_by_env[tunables_list_size];
Ok.
> if (!tunable_initialize (tunables[i].t, tunables[i].value,
> tunables[i].len))
> parse_tunable_print_error (&tunables[i]);
> + else
> + /* GLIBC_TUNABLES set this tunable; a legacy alias must not
> + override it. */
> + tunable_set_by_env[i] = true;
> }
> }
Ok.
> @@ -498,9 +510,13 @@ __tunables_init (char **envp, char **argv)
>
> for (int i = 0; i < tunable_num_env_alias; i++)
> {
> - /* Skip over tunables that have either been set or already initialized. */
> + /* Skip aliases whose tunable was already set through GLIBC_TUNABLES,
> + which takes precedence over the alias. A value coming only from the
> + system-wide cache does not block the alias here: an overridable cache
> + default may still be overridden, while a nonoverridable one is
> + protected by tunable_t::locked. */
> if (tunables_env_alias[i].t == NULL
> - || tunables_env_alias[i].t->initialized)
> + || tunable_set_by_env[tunable_env_alias_list[i]])
> continue;
>
Ok.
> diff --git a/elf/tst-tunconf1.c b/elf/tst-tunconf1.c
> + /* Interaction with legacy environment-variable aliases (MALLOC_*). */
> + int32_t mmap_max = TUNABLE_GET_FULL (glibc, malloc, mmap_max, int32_t, NULL);
> + size_t top_pad = TUNABLE_GET_FULL (glibc, malloc, top_pad, size_t, NULL);
> + size_t arena_max = TUNABLE_GET_FULL (glibc, malloc, arena_max, size_t, NULL);
> +
> + /* Overridable cache default (100); the MALLOC_MMAP_MAX_ alias overrides
> + it, just like GLIBC_TUNABLES would. */
> + printf("mmap_max is %d (should be 200, from MALLOC_MMAP_MAX_ alias)\n",
> + mmap_max);
> + TEST_COMPARE (mmap_max, 200);
> +
> + /* Nonoverridable cache default (100); the MALLOC_TOP_PAD_ alias must not
> + override it. */
> + printf("top_pad is %ld (should be 100, from /etc nonoverridable)\n",
> + (long)top_pad);
> + TEST_COMPARE ((long)top_pad, 100);
> +
> + /* Set both by GLIBC_TUNABLES (300) and by the MALLOC_ARENA_MAX alias
> + (400); the canonical GLIBC_TUNABLES form wins. */
> + printf("arena_max is %ld (should be 300, from GLIBC_TUNABLES)\n",
> + (long)arena_max);
> + TEST_COMPARE ((long)arena_max, 300);
Ok.
> diff --git a/elf/tst-tunconf1.root/etc/tunables.conf b/elf/tst-tunconf1.root/etc/tunables.conf
> +# Interaction with legacy environment-variable aliases (MALLOC_*), checked
> +# in the test case:
> +# - mmap_max: overridable cache default, must be overridable by the
> +# MALLOC_MMAP_MAX_ alias just like by GLIBC_TUNABLES.
> +# - top_pad: nonoverridable cache default, must NOT be overridable by the
> +# MALLOC_TOP_PAD_ alias.
> +glibc.malloc.mmap_max=100
> +-glibc.malloc.top_pad=100
Ok.
On 08/07/26 23:17, DJ Delorie wrote:
>
> LGTM
> Reviewed-by: DJ Delorie <dj@redhat.com>
>
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> +/* Records tunables that were set from GLIBC_TUNABLES during this call, so
>
> I wonder if it would make sense to have one variable that had the
> "origin priority" of the set value, from TUNABLE_UNSET through
> TUNABLE_LOCKED, so a single comparison tells us if a given source can
> change the value and bump the priority, replacing initialized, locked,
> and the new set_by_env?
>
> TUNABLE_UNSET
> TUNABLE_GLOBAL
> TUNABLE_LEGACY
> TUNABLE_ENVIRON
> TUNABLE_LOCKED
>
> (something for after the release, as it's purely internal)
It should be feasible to move the tunable_set_by_env to 'struct _tunable'
itself and replace it with this definition. It would require add a bit of more
logic to set the correct value, but I think we might also remove the
'struct _tunable::initialized' and use this new field instead.
>
>> + that a legacy environment-variable alias does not override themi (the
>> + canonical GLIBC_TUNABLES form takes precedence over the aliases).
>> + A tunable that was set only from the system-wide cache is deliberately not
>> + recorded here, so an alias may still override an overridable cache default;
>> + a nonoverridable one remains protected by tunable_t::locked. */
>> +static bool tunable_set_by_env[tunables_list_size];
>
> Ok.
>
>> if (!tunable_initialize (tunables[i].t, tunables[i].value,
>> tunables[i].len))
>> parse_tunable_print_error (&tunables[i]);
>> + else
>> + /* GLIBC_TUNABLES set this tunable; a legacy alias must not
>> + override it. */
>> + tunable_set_by_env[i] = true;
>> }
>> }
>
> Ok.
>
>> @@ -498,9 +510,13 @@ __tunables_init (char **envp, char **argv)
>>
>> for (int i = 0; i < tunable_num_env_alias; i++)
>> {
>> - /* Skip over tunables that have either been set or already initialized. */
>> + /* Skip aliases whose tunable was already set through GLIBC_TUNABLES,
>> + which takes precedence over the alias. A value coming only from the
>> + system-wide cache does not block the alias here: an overridable cache
>> + default may still be overridden, while a nonoverridable one is
>> + protected by tunable_t::locked. */
>> if (tunables_env_alias[i].t == NULL
>> - || tunables_env_alias[i].t->initialized)
>> + || tunable_set_by_env[tunable_env_alias_list[i]])
>> continue;
>>
>
> Ok.
>
>> diff --git a/elf/tst-tunconf1.c b/elf/tst-tunconf1.c
>
>> + /* Interaction with legacy environment-variable aliases (MALLOC_*). */
>> + int32_t mmap_max = TUNABLE_GET_FULL (glibc, malloc, mmap_max, int32_t, NULL);
>> + size_t top_pad = TUNABLE_GET_FULL (glibc, malloc, top_pad, size_t, NULL);
>> + size_t arena_max = TUNABLE_GET_FULL (glibc, malloc, arena_max, size_t, NULL);
>> +
>> + /* Overridable cache default (100); the MALLOC_MMAP_MAX_ alias overrides
>> + it, just like GLIBC_TUNABLES would. */
>> + printf("mmap_max is %d (should be 200, from MALLOC_MMAP_MAX_ alias)\n",
>> + mmap_max);
>> + TEST_COMPARE (mmap_max, 200);
>> +
>> + /* Nonoverridable cache default (100); the MALLOC_TOP_PAD_ alias must not
>> + override it. */
>> + printf("top_pad is %ld (should be 100, from /etc nonoverridable)\n",
>> + (long)top_pad);
>> + TEST_COMPARE ((long)top_pad, 100);
>> +
>> + /* Set both by GLIBC_TUNABLES (300) and by the MALLOC_ARENA_MAX alias
>> + (400); the canonical GLIBC_TUNABLES form wins. */
>> + printf("arena_max is %ld (should be 300, from GLIBC_TUNABLES)\n",
>> + (long)arena_max);
>> + TEST_COMPARE ((long)arena_max, 300);
>
> Ok.
>
>> diff --git a/elf/tst-tunconf1.root/etc/tunables.conf b/elf/tst-tunconf1.root/etc/tunables.conf
>
>> +# Interaction with legacy environment-variable aliases (MALLOC_*), checked
>> +# in the test case:
>> +# - mmap_max: overridable cache default, must be overridable by the
>> +# MALLOC_MMAP_MAX_ alias just like by GLIBC_TUNABLES.
>> +# - top_pad: nonoverridable cache default, must NOT be overridable by the
>> +# MALLOC_TOP_PAD_ alias.
>> +glibc.malloc.mmap_max=100
>> +-glibc.malloc.top_pad=100
>
> Ok.
>
@@ -351,7 +351,12 @@ tst-tunconf1-TUNABLES-only = \
glibc.malloc.tcache_count=5 \
:glibc.malloc.perturb=41 \
:glibc.malloc.mmap_threshold=10002 \
- :glibc.malloc.trim_threshold=10002
+ :glibc.malloc.trim_threshold=10002 \
+ :glibc.malloc.arena_max=300
+tst-tunconf1-ENV = \
+ MALLOC_MMAP_MAX_=200 \
+ MALLOC_TOP_PAD_=200 \
+ MALLOC_ARENA_MAX=400
static-dlopen-environment = \
LD_LIBRARY_PATH=$(ld-library-path):$(common-objpfx)dlfcn
@@ -179,6 +179,14 @@ struct tunable_toset_t
enum { tunables_list_size = array_length (tunable_list) };
+/* Records tunables that were set from GLIBC_TUNABLES during this call, so
+ that a legacy environment-variable alias does not override themi (the
+ canonical GLIBC_TUNABLES form takes precedence over the aliases).
+ A tunable that was set only from the system-wide cache is deliberately not
+ recorded here, so an alias may still override an overridable cache default;
+ a nonoverridable one remains protected by tunable_t::locked. */
+static bool tunable_set_by_env[tunables_list_size];
+
/* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
and their respective values. The VALSTRING is parsed in place, with the
tunable start and size recorded in TUNABLES.
@@ -288,6 +296,10 @@ parse_tunables (const char *valstring)
if (!tunable_initialize (tunables[i].t, tunables[i].value,
tunables[i].len))
parse_tunable_print_error (&tunables[i]);
+ else
+ /* GLIBC_TUNABLES set this tunable; a legacy alias must not
+ override it. */
+ tunable_set_by_env[i] = true;
}
}
@@ -498,9 +510,13 @@ __tunables_init (char **envp, char **argv)
for (int i = 0; i < tunable_num_env_alias; i++)
{
- /* Skip over tunables that have either been set or already initialized. */
+ /* Skip aliases whose tunable was already set through GLIBC_TUNABLES,
+ which takes precedence over the alias. A value coming only from the
+ system-wide cache does not block the alias here: an overridable cache
+ default may still be overridden, while a nonoverridable one is
+ protected by tunable_t::locked. */
if (tunables_env_alias[i].t == NULL
- || tunables_env_alias[i].t->initialized)
+ || tunable_set_by_env[tunable_env_alias_list[i]])
continue;
if (!tunable_initialize (tunables_env_alias[i].t,
@@ -16,6 +16,7 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <stdint.h>
#include <stdio.h>
#include <support/check.h>
@@ -50,6 +51,29 @@ do_test (void)
(long)trim_threshold);
TEST_COMPARE ((long)trim_threshold, 10001);
+ /* Interaction with legacy environment-variable aliases (MALLOC_*). */
+ int32_t mmap_max = TUNABLE_GET_FULL (glibc, malloc, mmap_max, int32_t, NULL);
+ size_t top_pad = TUNABLE_GET_FULL (glibc, malloc, top_pad, size_t, NULL);
+ size_t arena_max = TUNABLE_GET_FULL (glibc, malloc, arena_max, size_t, NULL);
+
+ /* Overridable cache default (100); the MALLOC_MMAP_MAX_ alias overrides
+ it, just like GLIBC_TUNABLES would. */
+ printf("mmap_max is %d (should be 200, from MALLOC_MMAP_MAX_ alias)\n",
+ mmap_max);
+ TEST_COMPARE (mmap_max, 200);
+
+ /* Nonoverridable cache default (100); the MALLOC_TOP_PAD_ alias must not
+ override it. */
+ printf("top_pad is %ld (should be 100, from /etc nonoverridable)\n",
+ (long)top_pad);
+ TEST_COMPARE ((long)top_pad, 100);
+
+ /* Set both by GLIBC_TUNABLES (300) and by the MALLOC_ARENA_MAX alias
+ (400); the canonical GLIBC_TUNABLES form wins. */
+ printf("arena_max is %ld (should be 300, from GLIBC_TUNABLES)\n",
+ (long)arena_max);
+ TEST_COMPARE ((long)arena_max, 300);
+
return 0;
}
@@ -13,6 +13,15 @@ $glibc.malloc.tcache_count=3
-glibc.malloc.mmap_threshold=10000
overridable glibc.malloc.trim_threshold=10000
+# Interaction with legacy environment-variable aliases (MALLOC_*), checked
+# in the test case:
+# - mmap_max: overridable cache default, must be overridable by the
+# MALLOC_MMAP_MAX_ alias just like by GLIBC_TUNABLES.
+# - top_pad: nonoverridable cache default, must NOT be overridable by the
+# MALLOC_TOP_PAD_ alias.
+glibc.malloc.mmap_max=100
+-glibc.malloc.top_pad=100
+
[proc:/bin/ls]
glibc.malloc.tcache_max=7