tunables: Clean up hooks to get and set tunables

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.

6 files changed, 168 insertions, 70 deletions

diff --git a/ChangeLog b/ChangeLog
index b42bdb8565..b85c6182ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2017-06-07  Siddhesh Poyarekar  <siddhesh@sourceware.org>
+
+	* 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.
+
 2017-06-06  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* sysdeps/x86_64/multiarch/Makefile (sysdep_routines): Add
diff --git a/README.tunables b/README.tunables
index 0e9b0d7a47..3967679f43 100644
--- a/README.tunables
+++ b/README.tunables
@@ -20,14 +20,12 @@ namespace by overriding the TOP_NAMESPACE macro for that tunable.  Downstream
 implementations are discouraged from using the 'glibc' top namespace for
 tunables they don't already have consensus to push upstream.
 
-There are two steps to adding a tunable:
+There are three steps to adding a tunable:
 
-1. Add a tunable ID:
+1. Add a tunable to the list and fully specify its properties:
 
-Modules that wish to use the tunables interface must define the
-TUNABLE_NAMESPACE macro.  Following this, for each tunable you want to
-add, make an entry in elf/dl-tunables.list.  The format of the file is as
-follows:
+For each tunable you want to add, make an entry in elf/dl-tunables.list.  The
+format of the file is as follows:
 
 TOP_NAMESPACE {
   NAMESPACE1 {
@@ -69,11 +67,60 @@ The list of allowed attributes are:
 				     non-AT_SECURE subprocesses.
 			NONE: Read all the time.
 
-2. Call either the TUNABLE_SET_VALUE and pass into it the tunable name and a
-   pointer to the variable that should be set with the tunable value.
-   If additional work needs to be done after setting the value, use the
-   TUNABLE_SET_VALUE_WITH_CALLBACK instead and additionally pass a pointer to
-   the function that should be called if the tunable value has been set.
+2. Use TUNABLE_GET/TUNABLE_SET to get and set tunables.
+
+3. OPTIONAL: If tunables in a namespace are being used multiple times within a
+   specific module, set the TUNABLE_NAMESPACE macro to reduce the amount of
+   typing.
+
+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
+  TUNABLE_CALLBACK (check_callback) (int32_t *valp)
+  {
+  ...
+  }
+
+where it can expect the tunable value to be passed in VALP.
+
+Tunables in the module can be updated using:
+
+  TUNABLE_SET (check, int32_t, val)
+
+where 'check' is the tunable name, 'int32_t' is the C type of the tunable and
+'val' is a value of same type.
+
+To get and set tunables in a different namespace from that module, use the full
+form of the macros as follows:
+
+  val = TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
+
+  TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, val)
+
+where 'glibc' is the top namespace, 'tune' is the tunable namespace and the
+remaining arguments are the same as the short form macros.
+
+When TUNABLE_NAMESPACE is not defined in a module, TUNABLE_GET is equivalent to
+TUNABLE_GET_FULL, so you will need to provide full namespace information for
+both macros.  Likewise for TUNABLE_SET and TUNABLE_SET_FULL.
+
+** IMPORTANT NOTE **
+
+The tunable list is set as read-only after the dynamic linker relocates itself,
+so setting tunable values must be limited only to tunables within the dynamic
+linker, that too before relocation.
 
 FUTURE WORK
 -----------
diff --git a/elf/Versions b/elf/Versions
index 6abe9db8bd..c59facdbd7 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -72,6 +72,6 @@ ld {
     _dl_signal_error; _dl_catch_error;
 
     # Set value of a tunable.
-    __tunable_set_val;
+    __tunable_get_val;
   }
 }
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index b6e6b3da52..76e8c5cae1 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -184,19 +184,17 @@ tunables_strtoul (const char *nptr)
   if ((__type) (__val) >= min && (__type) (val) <= max)			      \
     {									      \
       (__cur)->val.numval = val;					      \
-      (__cur)->strval = strval;						      \
+      (__cur)->initialized = true;					      \
     }									      \
 })
 
-/* Validate range of the input value and initialize the tunable CUR if it looks
-   good.  */
 static void
-tunable_initialize (tunable_t *cur, const char *strval)
+do_tunable_update_val (tunable_t *cur, const void *valp)
 {
   uint64_t val;
 
   if (cur->type.type_code != TUNABLE_TYPE_STRING)
-    val = tunables_strtoul (strval);
+    val = *((int64_t *) valp);
 
   switch (cur->type.type_code)
     {
@@ -217,7 +215,7 @@ tunable_initialize (tunable_t *cur, const char *strval)
 	}
     case TUNABLE_TYPE_STRING:
 	{
-	  cur->val.strval = cur->strval = strval;
+	  cur->val.strval = valp;
 	  break;
 	}
     default:
@@ -225,6 +223,35 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+/* Validate range of the input value and initialize the tunable CUR if it looks
+   good.  */
+static void
+tunable_initialize (tunable_t *cur, const char *strval)
+{
+  uint64_t val;
+  const void *valp;
+
+  if (cur->type.type_code != TUNABLE_TYPE_STRING)
+    {
+      val = tunables_strtoul (strval);
+      valp = &val;
+    }
+  else
+    {
+      cur->initialized = true;
+      valp = strval;
+    }
+  do_tunable_update_val (cur, valp);
+}
+
+void
+__tunable_set_val (tunable_id_t id, void *valp)
+{
+  tunable_t *cur = &tunable_list[id];
+
+  do_tunable_update_val (cur, valp);
+}
+
 #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
 /* Parse the tunable string TUNESTR and adjust it to drop any tunables that may
    be unsafe for AT_SECURE processes so that it can be used as the new
@@ -375,7 +402,7 @@ __tunables_init (char **envp)
 
 	  /* Skip over tunables that have either been set already or should be
 	     skipped.  */
-	  if (cur->strval != NULL || cur->env_alias == NULL)
+	  if (cur->initialized || cur->env_alias == NULL)
 	    continue;
 
 	  const char *name = cur->env_alias;
@@ -426,20 +453,10 @@ __tunables_init (char **envp)
 /* Set the tunable value.  This is called by the module that the tunable exists
    in. */
 void
-__tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
+__tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
 {
   tunable_t *cur = &tunable_list[id];
 
-  /* Don't do anything if our tunable was not set during initialization or if
-     it failed validation.  */
-  if (cur->strval == NULL)
-    return;
-
-  /* Caller does not need the value, just call the callback with our tunable
-     value.  */
-  if (valp == NULL)
-    goto cb;
-
   switch (cur->type.type_code)
     {
     case TUNABLE_TYPE_UINT_64:
@@ -466,9 +483,8 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
       __builtin_unreachable ();
     }
 
-cb:
-  if (callback)
+  if (cur->initialized && callback != NULL)
     callback (&cur->val);
 }
 
-rtld_hidden_def (__tunable_set_val)
+rtld_hidden_def (__tunable_get_val)
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 20ee51254e..6c49dcbf47 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -39,8 +39,8 @@ struct _tunable
   const char *name;			/* Internal name of the tunable.  */
   tunable_type_t type;			/* Data type of the tunable.  */
   tunable_val_t val;			/* The value.  */
-  const char *strval;			/* The string containing the value,
-					   points into envp.  */
+  bool initialized;			/* Flag to indicate that the tunable is
+					   initialized.  */
   tunable_seclevel_t security_level;	/* Specify the security level for the
 					   tunable with respect to AT_SECURE
 					   programs.  See description of
@@ -61,37 +61,52 @@ typedef struct _tunable tunable_t;
 /* Full name for a tunable is top_ns.tunable_ns.id.  */
 # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id
 
-# define TUNABLE_ENUM_NAME(top,ns,id) TUNABLE_ENUM_NAME1 (top,ns,id)
-# define TUNABLE_ENUM_NAME1(top,ns,id) top ## _ ## ns ## _ ## id
+# define TUNABLE_ENUM_NAME(__top,__ns,__id) TUNABLE_ENUM_NAME1 (__top,__ns,__id)
+# define TUNABLE_ENUM_NAME1(__top,__ns,__id) __top ## _ ## __ns ## _ ## __id
 
 # include "dl-tunable-list.h"
 
 extern void __tunables_init (char **);
-extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
-
+extern void __tunable_get_val (tunable_id_t, void *, tunable_callback_t);
+extern void __tunable_set_val (tunable_id_t, void *);
 rtld_hidden_proto (__tunables_init)
-rtld_hidden_proto (__tunable_set_val)
+rtld_hidden_proto (__tunable_get_val)
+
+/* 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_GET(__id, __type, __cb) \
+  TUNABLE_GET_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, __type, __cb)
+# define TUNABLE_SET(__id, __type, __val) \
+  TUNABLE_SET_FULL (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id, __type, __val)
+#else
+# define TUNABLE_GET(__top, __ns, __id, __type, __cb) \
+  TUNABLE_GET_FULL (__top, __ns, __id, __type, __cb)
+# define TUNABLE_SET(__top, __ns, __id, __type, __val) \
+  TUNABLE_SET_FULL (__top, __ns, __id, __type, __val)
+#endif
 
-/* Check if the tunable has been set to a non-default value and if it is, copy
-   it over into __VAL.  */
-# define TUNABLE_SET_VAL(__id,__val) \
+/* Get and return a tunable value.  If the tunable was set externally and __CB
+   is defined then call __CB before returning the value.  */
+# define TUNABLE_GET_FULL(__top, __ns, __id, __type, __cb) \
 ({									      \
-  __tunable_set_val							      \
-   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
-    NULL);								      \
+  tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id);		      \
+  __type ret;								      \
+  __tunable_get_val (id, &ret, __cb);					      \
+  ret;									      \
 })
 
-/* Same as TUNABLE_SET_VAL, but also call the callback function __CB.  */
-# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
+/* Set a tunable value.  */
+# define TUNABLE_SET_FULL(__top, __ns, __id, __type, __val) \
 ({									      \
-  __tunable_set_val							      \
-   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
-    DL_TUNABLE_CALLBACK (__cb));					      \
+  __tunable_set_val (TUNABLE_ENUM_NAME (__top, __ns, __id),		      \
+			& (__type) {__val});				      \
 })
 
 /* Namespace sanity for callback functions.  Use this macro to keep the
    namespace of the modules clean.  */
-# define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
+# define TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
 
 # define TUNABLES_FRONTEND_valstring 1
 /* The default value for TUNABLES_FRONTEND.  */
diff --git a/malloc/arena.c b/malloc/arena.c
index d49e4a21c8..660d638c93 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -212,7 +212,7 @@ __malloc_fork_unlock_child (void)
 #if HAVE_TUNABLES
 static inline int do_set_mallopt_check (int32_t value);
 void
-DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
+TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
 {
   int32_t value = (int32_t) valp->numval;
   do_set_mallopt_check (value);
@@ -220,22 +220,22 @@ DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
     __malloc_check_init ();
 }
 
-# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \
+# define TUNABLE_CALLBACK_FNDECL(__name, __type) \
 static inline int do_ ## __name (__type value);				      \
 void									      \
-DL_TUNABLE_CALLBACK (__name) (tunable_val_t *valp)			      \
+TUNABLE_CALLBACK (__name) (tunable_val_t *valp)				      \
 {									      \
   __type value = (__type) (valp)->numval;				      \
   do_ ## __name (value);						      \
 }
 
-DL_TUNABLE_CALLBACK_FNDECL (set_mmap_threshold, size_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_mmaps_max, int32_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_top_pad, size_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_perturb_byte, int32_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_trim_threshold, size_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_arena_max, size_t)
-DL_TUNABLE_CALLBACK_FNDECL (set_arena_test, size_t)
+TUNABLE_CALLBACK_FNDECL (set_mmap_threshold, size_t)
+TUNABLE_CALLBACK_FNDECL (set_mmaps_max, int32_t)
+TUNABLE_CALLBACK_FNDECL (set_top_pad, size_t)
+TUNABLE_CALLBACK_FNDECL (set_perturb_byte, int32_t)
+TUNABLE_CALLBACK_FNDECL (set_trim_threshold, size_t)
+TUNABLE_CALLBACK_FNDECL (set_arena_max, size_t)
+TUNABLE_CALLBACK_FNDECL (set_arena_test, size_t)
 #else
 /* Initialization routine. */
 #include <string.h>
@@ -314,14 +314,14 @@ ptmalloc_init (void)
   __libc_lock_lock (main_arena.mutex);
   malloc_consolidate (&main_arena);
 
-  TUNABLE_SET_VAL_WITH_CALLBACK (check, NULL, set_mallopt_check);
-  TUNABLE_SET_VAL_WITH_CALLBACK (top_pad, NULL, set_top_pad);
-  TUNABLE_SET_VAL_WITH_CALLBACK (perturb, NULL, set_perturb_byte);
-  TUNABLE_SET_VAL_WITH_CALLBACK (mmap_threshold, NULL, set_mmap_threshold);
-  TUNABLE_SET_VAL_WITH_CALLBACK (trim_threshold, NULL, set_trim_threshold);
-  TUNABLE_SET_VAL_WITH_CALLBACK (mmap_max, NULL, set_mmaps_max);
-  TUNABLE_SET_VAL_WITH_CALLBACK (arena_max, NULL, set_arena_max);
-  TUNABLE_SET_VAL_WITH_CALLBACK (arena_test, NULL, set_arena_test);
+  TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
+  TUNABLE_GET (top_pad, size_t, TUNABLE_CALLBACK (set_top_pad));
+  TUNABLE_GET (perturb, int32_t, TUNABLE_CALLBACK (set_perturb_byte));
+  TUNABLE_GET (mmap_threshold, size_t, TUNABLE_CALLBACK (set_mmap_threshold));
+  TUNABLE_GET (trim_threshold, size_t, TUNABLE_CALLBACK (set_trim_threshold));
+  TUNABLE_GET (mmap_max, int32_t, TUNABLE_CALLBACK (set_mmaps_max));
+  TUNABLE_GET (arena_max, size_t, TUNABLE_CALLBACK (set_arena_max));
+  TUNABLE_GET (arena_test, size_t, TUNABLE_CALLBACK (set_arena_test));
   __libc_lock_unlock (main_arena.mutex);
 #else
   const char *s = NULL;