This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 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.
> 	prevent collision.
> 	(__tunable_set_val): New function.
> 	(TUNABLE_SET_VAL): Remove.
> 	* 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


> +----------------------------
> +
> +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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]