Re: [PATCH v2 1/2] elf: Add a way to check if tunable is set (BZ 27069)

[DJ Delorie <dj@redhat.com>]

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> diff --git a/elf/Versions b/elf/Versions
> index 4614acea3e..1591031da9 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -77,6 +77,7 @@ ld {
>      _dl_signal_error;
>  
>      # Set value of a tunable.
> +    __tunable_is_initialized;
>      __tunable_get_val;
>    }
>  }

This is in GLIBC_PRIVATE so ok.

> diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
> index 62d6d9e629..a2048058fa 100644
> --- a/elf/dl-tunable-types.h
> +++ b/elf/dl-tunable-types.h
> @@ -61,6 +61,7 @@ struct _tunable
>  {
>    const char name[TUNABLE_NAME_MAX];	/* Internal name of the tunable.  */
>    tunable_type_t type;			/* Data type of the tunable.  */
> +  const tunable_val_t def;		/* The value.  */
>    tunable_val_t val;			/* The value.  */
>    bool initialized;			/* Flag to indicate that the tunable is
>  					   initialized.  */

Is this struct part of the ABI between ld.so and libc.so ?  It doesn't
look like it to me, so OK.

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 83265bc00b..644d21d1b0 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -145,6 +145,13 @@ tunable_initialize (tunable_t *cur, const char *strval)
>    do_tunable_update_val (cur, &val, NULL, NULL);
>  }
>  
> +bool
> +__tunable_is_initialized (tunable_id_t id)
> +{
> +  return tunable_list[id].initialized;
> +}
> +rtld_hidden_def (__tunable_is_initialized)
> +

Ok.

> @@ -333,6 +340,39 @@ __tunables_print (void)
>      }
>  }
>  
> +void
> +__tunable_get_default (tunable_id_t id, void *valp)
> +{
> +  tunable_t *cur = &tunable_list[id];
> +
> +  switch (cur->type.type_code)
> +    {
> +    case TUNABLE_TYPE_UINT_64:
> +	{
> +	  *((uint64_t *) valp) = (uint64_t) cur->def.numval;

Hmmm... doesn't this break the type aliasing rules?

But we do the same thing for __tunable_get_val so OK.

> +	  break;
> +	}
> +    case TUNABLE_TYPE_INT_32:
> +	{
> +	  *((int32_t *) valp) = (int32_t) cur->def.numval;
> +	  break;
> +	}
> +    case TUNABLE_TYPE_SIZE_T:
> +	{
> +	  *((size_t *) valp) = (size_t) cur->def.numval;
> +	  break;
> +	}
> +    case TUNABLE_TYPE_STRING:
> +	{
> +	  *((const char **)valp) = cur->def.strval;
> +	  break;
> +	}
> +    default:
> +      __builtin_unreachable ();
> +    }
> +}
> +rtld_hidden_def (__tunable_get_default)

We don't seem to have a flag that says "no default provided" but if we
assume the templates won't ever provide a default of zero, good enough?

Ok.

> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index 45c191e021..0df4dde24e 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -45,18 +45,26 @@ typedef void (*tunable_callback_t) (tunable_val_t *);
>  
>  extern void __tunables_init (char **);
>  extern void __tunables_print (void);
> +extern bool __tunable_is_initialized (tunable_id_t);

Ok.

> +extern void __tunable_get_default (tunable_id_t id, void *valp);

Ok.

> +rtld_hidden_proto (__tunable_is_initialized)
> +rtld_hidden_proto (__tunable_get_default)

Ok.

>  /* Define TUNABLE_GET and TUNABLE_SET in short form if TOP_NAMESPACE and
>     TUNABLE_NAMESPACE are defined.  This is useful shorthand to get and set
>     tunables within a module.  */
>  #if defined TOP_NAMESPACE && defined TUNABLE_NAMESPACE
> +# define TUNABLE_IS_INITIALIZED(__id) \
> +  TUNABLE_IS_INITIALIZED_FULL(TOP_NAMESPACE, TUNABLE_NAMESPACE, __id)
> +# define TUNABLE_GET_DEFAULT(__id, __type) \
> +  TUNABLE_GET_DEFAULT_FULL(TOP_NAMESPACE, TUNABLE_NAMESPACE,__id, __type)
>  # define TUNABLE_GET(__id, __type, __cb) \
>    TUNABLE_GET_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, __type, __cb)
>  # define TUNABLE_SET(__id, __val) \
> @@ -65,6 +73,10 @@ rtld_hidden_proto (__tunable_set_val)
>    TUNABLE_SET_WITH_BOUNDS_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, \
>  				__val, __min, __max)
>  #else
> +# define TUNABLE_IS_INITIALIZED(__top, __ns, __id) \
> +  TUNABLE_IS_INITIALIZED_FULL(__top, __ns, __id)
> +# define TUNABLE_GET_DEFAULT(__top, __ns, __type) \
> +  TUNABLE_GET_DEFAULT_FULL(__top, __ns, __id, __type)
>  # define TUNABLE_GET(__top, __ns, __id, __type, __cb) \
>    TUNABLE_GET_FULL (__top, __ns, __id, __type, __cb)
>  # define TUNABLE_SET(__top, __ns, __id, __val) \
> @@ -73,6 +85,22 @@ rtld_hidden_proto (__tunable_set_val)
>    TUNABLE_SET_WITH_BOUNDS_FULL (__top, __ns, __id, __val, __min, __max)
>  #endif

Ok.

> +/* Return whether the tunable was initialized by the environment variable.  */
> +#define TUNABLE_IS_INITIALIZED_FULL(__top, __ns, __id) \
> +({									      \
> +  tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id);		      \
> +  __tunable_is_initialized (id);					      \
> +})
> +
> +/* Return the default value of the tunable.  */
> +#define TUNABLE_GET_DEFAULT_FULL(__top, __ns, __id, __type) \
> +({									      \
> +  tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id);		      \
> +  __type __ret;								      \
> +  __tunable_get_default (id, &__ret);					      \
> +  __ret;								      \
> +})

Ok.

> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> index 720a8ac49c..1b23fc9473 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -20,6 +20,7 @@
>  # type: Defaults to STRING
>  # minval: Optional minimum acceptable value
>  # maxval: Optional maximum acceptable value
> +# default: Optional default value (if not specified it will be 0 or "")
>  # env_alias: An alias environment variable

Ok.

>  glibc {
> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index 1e9d6b534e..9f5336381e 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -163,8 +163,8 @@ END {
>      n = indices[2];
>      m = indices[3];
>      printf ("  {TUNABLE_NAME_S(%s, %s, %s)", t, n, m)
> -    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, false, %s},\n",
> -	    types[t,n,m], minvals[t,n,m], maxvals[t,n,m],
> +    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, %s},\n",
> +	    types[t,n,m], minvals[t,n,m], maxvals[t,n,m], default_val[t,n,m],
>  	    default_val[t,n,m], env_alias[t,n,m]);
>    }
>    print "};"

Ok.