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

Adhemerval Zanella Netto adhemerval.zanella@linaro.org
Wed Nov 29 12:29:24 GMT 2023



On 28/11/23 18:38, DJ Delorie wrote:
> 
> 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.

I don't think so, libc.so will issue the macros TUNABLES_* that will maps to
tunable_id_t.  The tunable_t is used only on dl-tunables.c.

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

Indeed, I will send a patch to fix it.

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

Right, but I think it would be easier to add once we actually require it.

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


More information about the Libc-alpha mailing list