This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/6] tunables: Clean up hooks to get and set tunables
On 01/06/2017 17:12, Siddhesh Poyarekar wrote:
> The TUNABLE_SET_VALUE and family of macros (and my later attempt to
> add a TUNABLE_GET) never quite went together very well because the
> overall interface was not clearly defined. This patch is an attempt
> to do just that.
>
> This patch consolidates the API to two simple sets of macros,
> TUNABLE_GET* and TUNABLE_SET*. If TUNABLE_NAMESPACE is defined,
> TUNABLE_GET takes just the tunable name, type and a (optionally NULL)
> callback function to get the value of the tunable. The callback
> function, if non-NULL, is called if the tunable was externally set
> (i.e. via GLIBC_TUNABLES or any future mechanism). For example:
>
> val = TUNABLE_GET (check, int32_t, check_callback)
>
> returns the value of the glibc.malloc.check tunable (assuming
> TUNABLE_NAMESPACE is set to malloc) as an int32_t into VAL after
> calling check_callback.
>
> Likewise, TUNABLE_SET can be used to set the value of the tunable,
> although this is currently possible only in the dynamic linker before
> it relocates itself. For example:
>
> TUNABLE_SET (check, int32_t, 2)
>
> will set glibc.malloc.check to 2. Of course, this is not possible
> since we set (or read) glibc.malloc.check long after it is relocated.
>
> To access or set a tunable outside of TUNABLE_NAMESPACE, use the
> TUNABLE_GET_FULL and TUNABLE_SET_FULL macros, which have the following
> prototype:
>
> TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
> TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, 0xffff)
>
> In future the tunable list may get split into mutable and immutable
> tunables where mutable tunables can be modified by the library and
> userspace after relocation as well and TUNABLE_SET will be more useful
> than it currently is. However whenever we actually do that split, we
> will have to ensure that the mutable tunables are protected with
> locks.
>
> * elf/Versions (__tunable_set_val): Rename to __tunable_get_val.
> * elf/dl-tunables.c: Likewise.
> (do_tunable_update_val): New function.
> (__tunable_set_val): New function.
> (__tunable_get_val): Call CB only if the tunable was externally
> initialized.
> (tunables_strtoul): Replace strval with initialized.
> * elf/dl-tunables.h (strval): Replace with a bool initialized.
> (TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
> prevent collision.
> (__tunable_set_val): New function.
> (TUNABLE_GET, TUNABLE_GET_FULL): New macros.
> (TUNABLE_SET, TUNABLE_SET_FULL): Likewise.
> (TUNABLE_SET_VAL): Remove.
> (TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
> * README.tunables: Document the new macros.
> * malloc/arena.c (ptmalloc_init): Adjust.
LGTM with just a clarification below.
> diff --git a/README.tunables b/README.tunables
> index 0e9b0d7..6fcfd2d 100644
> --- a/README.tunables
> +++ b/README.tunables
[...]
> +GETTING AND SETTING TUNABLES
> +----------------------------
> +
> +When the TUNABLE_NAMESPACE macro is defined, one may get tunables in that
> +module using the TUNABLE_GET macro as follows:
> +
> + val = TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (check_callback))
> +
> +where 'check' is the tunable name, 'int32_t' is the C type of the tunable and
> +'check_callback' is the function to call if the tunable got initialized to a
> +non-default value. The macro returns the value as type 'int32_t'.
> +
> +The callback function should be defined as follows:
> +
> + void
> + DL_TUNABLE_CALLBACK (check_callback) (int32_t *valp)
> + {
> + ...
> + }
Shouldn't it be just TUNABLE_CALLBACK? I noted that DL_TUNABLE_CALLBACK is just
define at malloc/arena.c.