[PATCH v5 0/6] glibc tunables

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 0/6] glibc tunables

Siddhesh Poyarekar-9
Hi,

... and I'm back!

Here is another updated patch set with suggestions from multiple people
incorporated, including results from discussions at the GNU Tools Cauldron last
month.

 - I had inadvertently opened a hole with MALLOC_CHECK_ where it could be read
   in setuid binaries even if /etc/suid-debug was not present.  This is now
   fixed.

 - __tunables_init is now called *really* early so that tunables are set up
   before apply_irel is called.  H. J. will now have to split his patch such
   that his IFUNC resolver looks at both, the tunables and the result of cpuid
   instead of overriding cpuid using tunables.  I think this is a good thing
   since it keeps the cpuid information honest and only masks it for the
   specific purpose of IFUNC.

 - The very early initialization meant that I needed a new version of __access
   that does not set errno.  That's another patch to the patchset.  This is an
   internal function, so there is no ABI event here.

 - Enhanced the --enable-tunables option (in a separate patch) to accept string
   values other than 'yes' and 'no'.  As discussed at Cauldron, this would
   allow us to experiment with different frontends without committing to one
   yet.  Right now there is only one frontend, i.e. 'valstring' that allows
   setting tunables using the single GLIBC_TUNABLES environment variable.

 - Wrote a manual node for tunables.

As usual, the branch siddhesh/tunables has these patches and any patches they
may depend on (like the doc changes for malloc).

Siddhesh Poyarekar (6):
  Static inline functions for mallopt helpers
  New internal function __access_noerrno
  Add framework for tunables
  Initialize tunable list with the GLIBC_TUNABLES environment variable
  Enhance --enable-tunables to select tunables frontend at build time
  User manual documentation for tunables

 INSTALL                                    |  18 ++
 Makeconfig                                 |  16 ++
 README.tunables                            |  84 ++++++
 config.h.in                                |   3 +
 config.make.in                             |   1 +
 configure                                  |  17 ++
 configure.ac                               |  10 +
 csu/init-first.c                           |   2 -
 csu/libc-start.c                           |  11 +
 elf/Makefile                               |   7 +
 elf/Versions                               |   3 +
 elf/dl-support.c                           |   2 +
 elf/dl-sysdep.c                            |   8 +
 elf/dl-tunable-types.h                     |  46 +++
 elf/dl-tunables.c                          | 435 +++++++++++++++++++++++++++++
 elf/dl-tunables.h                          |  82 ++++++
 elf/dl-tunables.list                       |  69 +++++
 elf/rtld.c                                 |   2 +
 include/unistd.h                           |   6 +
 io/Makefile                                |   1 +
 io/access.c                                |  10 +-
 io/access_noerrno.c                        |  21 ++
 malloc/Makefile                            |   6 +
 malloc/arena.c                             |  54 ++++
 malloc/malloc.c                            | 126 ++++++---
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-static.c          |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 manual/Makefile                            |   3 +-
 manual/install.texi                        |  21 ++
 manual/probes.texi                         |   2 +-
 manual/tunables.texi                       | 184 ++++++++++++
 scripts/gen-tunables.awk                   | 157 +++++++++++
 sysdeps/mach/hurd/access.c                 |  20 +-
 sysdeps/mach/hurd/dl-sysdep.c              |   8 +
 sysdeps/nacl/access.c                      |  16 +-
 sysdeps/nacl/nacl-interfaces.h             |   4 +
 sysdeps/unix/access_noerrno.c              |  38 +++
 sysdeps/unix/sysv/linux/generic/access.c   |  19 +-
 39 files changed, 1469 insertions(+), 46 deletions(-)
 create mode 100644 README.tunables
 create mode 100644 elf/dl-tunable-types.h
 create mode 100644 elf/dl-tunables.c
 create mode 100644 elf/dl-tunables.h
 create mode 100644 elf/dl-tunables.list
 create mode 100644 io/access_noerrno.c
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-static.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c
 create mode 100644 manual/tunables.texi
 create mode 100644 scripts/gen-tunables.awk
 create mode 100644 sysdeps/unix/access_noerrno.c

--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/6] Static inline functions for mallopt helpers

Siddhesh Poyarekar-9
Make mallopt helper functions for each mallopt parameter so that it
can be called consistently in other areas, like setting tunables.

        * malloc/malloc.c (do_set_mallopt_check): New function.
        (do_set_mmap_threshold): Likewise.
        (do_set_mmaps_max): Likewise.
        (do_set_top_pad): Likewise.
        (do_set_perturb_byte): Likewise.
        (do_set_trim_threshold): Likewise.
        (do_set_arena_max): Likewise.
        (do_set_arena_test): Likewise.
        (__libc_mallopt): Use them.
---
 malloc/malloc.c | 126 +++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 92 insertions(+), 34 deletions(-)

diff --git a/malloc/malloc.c b/malloc/malloc.c
index a849901..0011a6d 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -4723,6 +4723,90 @@ __malloc_stats (void)
 /*
    ------------------------------ mallopt ------------------------------
  */
+static inline int
+__always_inline
+do_set_mallopt_check (int32_t value)
+{
+  LIBC_PROBE (memory_mallopt_check_action, 2, value, check_action);
+  check_action = value;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_mmap_threshold (size_t value)
+{
+  if (value <= HEAP_MAX_SIZE / 2)
+    {
+      LIBC_PROBE (memory_mallopt_mmap_threshold, 3, value, mp_.mmap_threshold,
+  mp_.no_dyn_threshold);
+      mp_.mmap_threshold = value;
+      mp_.no_dyn_threshold = 1;
+      return 1;
+    }
+  return 0;
+}
+
+static inline int
+__always_inline
+do_set_mmaps_max (int32_t value)
+{
+  LIBC_PROBE (memory_mallopt_mmap_max, 3, value, mp_.n_mmaps_max,
+      mp_.no_dyn_threshold);
+  mp_.n_mmaps_max = value;
+  mp_.no_dyn_threshold = 1;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_top_pad (size_t value)
+{
+  LIBC_PROBE (memory_mallopt_top_pad, 3, value, mp_.top_pad,
+      mp_.no_dyn_threshold);
+  mp_.top_pad = value;
+  mp_.no_dyn_threshold = 1;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_perturb_byte (int32_t value)
+{
+  LIBC_PROBE (memory_mallopt_perturb, 2, value, perturb_byte);
+  perturb_byte = value;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_trim_threshold (size_t value)
+{
+  LIBC_PROBE (memory_mallopt_trim_threshold, 3, value, mp_.trim_threshold,
+      mp_.no_dyn_threshold);
+  mp_.trim_threshold = value;
+  mp_.no_dyn_threshold = 1;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_arena_max (size_t value)
+{
+  LIBC_PROBE (memory_mallopt_arena_max, 2, value, mp_.arena_max);
+  mp_.arena_max = value;
+  return 1;
+}
+
+static inline int
+__always_inline
+do_set_arena_test (size_t value)
+{
+  LIBC_PROBE (memory_mallopt_arena_test, 2, value, mp_.arena_test);
+  mp_.arena_test = value;
+  return 1;
+}
+
 
 int
 __libc_mallopt (int param_number, int value)
@@ -4751,63 +4835,37 @@ __libc_mallopt (int param_number, int value)
       break;
 
     case M_TRIM_THRESHOLD:
-      LIBC_PROBE (memory_mallopt_trim_threshold, 3, value,
-                  mp_.trim_threshold, mp_.no_dyn_threshold);
-      mp_.trim_threshold = value;
-      mp_.no_dyn_threshold = 1;
+      do_set_trim_threshold (value);
       break;
 
     case M_TOP_PAD:
-      LIBC_PROBE (memory_mallopt_top_pad, 3, value,
-                  mp_.top_pad, mp_.no_dyn_threshold);
-      mp_.top_pad = value;
-      mp_.no_dyn_threshold = 1;
+      do_set_top_pad (value);
       break;
 
     case M_MMAP_THRESHOLD:
-      /* Forbid setting the threshold too high. */
-      if ((unsigned long) value > HEAP_MAX_SIZE / 2)
-        res = 0;
-      else
-        {
-          LIBC_PROBE (memory_mallopt_mmap_threshold, 3, value,
-                      mp_.mmap_threshold, mp_.no_dyn_threshold);
-          mp_.mmap_threshold = value;
-          mp_.no_dyn_threshold = 1;
-        }
+      res = do_set_mmap_threshold (value);
       break;
 
     case M_MMAP_MAX:
-      LIBC_PROBE (memory_mallopt_mmap_max, 3, value,
-                  mp_.n_mmaps_max, mp_.no_dyn_threshold);
-      mp_.n_mmaps_max = value;
-      mp_.no_dyn_threshold = 1;
+      do_set_mmaps_max (value);
       break;
 
     case M_CHECK_ACTION:
-      LIBC_PROBE (memory_mallopt_check_action, 2, value, check_action);
-      check_action = value;
+      do_set_mallopt_check (value);
       break;
 
     case M_PERTURB:
-      LIBC_PROBE (memory_mallopt_perturb, 2, value, perturb_byte);
-      perturb_byte = value;
+      do_set_perturb_byte (value);
       break;
 
     case M_ARENA_TEST:
       if (value > 0)
-        {
-          LIBC_PROBE (memory_mallopt_arena_test, 2, value, mp_.arena_test);
-          mp_.arena_test = value;
-        }
+ do_set_arena_test (value);
       break;
 
     case M_ARENA_MAX:
       if (value > 0)
-        {
-          LIBC_PROBE (memory_mallopt_arena_max, 2, value, mp_.arena_max);
-          mp_.arena_max = value;
-        }
+ do_set_arena_test (value);
       break;
     }
   __libc_lock_unlock (av->mutex);
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/6] Initialize tunable list with the GLIBC_TUNABLES environment variable

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
Read tunables values from the users using the GLIBC_TUNABLES
environment variable.  The value of this variable is a colon-separated
list of name=value pairs.  So a typical string would look like this:

GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024

        * elf/dl-tunables.c: Include mman.h and libc-internals.h.
        (GLIBC_TUNABLES): New macro.
        (tunables_strdup): New function.
        (parse_tunables): New function.
        (__tunables_init): Use the new functions and macro.
        (disable_tunable): Disable tunable from GLIBC_TUNABLES.
        * malloc/tst-malloc-usable-tunables.c: New test case.
        * malloc/tst-malloc-usable-static-tunables.c: New test case.
        * malloc/Makefile (tests, tests-static): Add tests.
---
 elf/dl-tunables.c                          | 115 +++++++++++++++++++++++++++++
 malloc/Makefile                            |   6 +-
 malloc/tst-malloc-usable-static-tunables.c |   1 +
 malloc/tst-malloc-usable-tunables.c        |   1 +
 4 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 malloc/tst-malloc-usable-static-tunables.c
 create mode 100644 malloc/tst-malloc-usable-tunables.c

diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 91340b6..6612885 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -22,6 +22,7 @@
 #include <stdbool.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <sys/mman.h>
 #include <libc-internal.h>
 #include <sysdep.h>
 #include <fcntl.h>
@@ -29,6 +30,8 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
+#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+
 /* Compare environment names, bounded by the name hardcoded in glibc.  */
 static bool
 is_name (const char *orig, const char *envname)
@@ -44,6 +47,27 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+static char *tunables_strdup (const char *in)
+{
+  size_t i = 0;
+
+  while (in[i++]);
+
+  char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()),
+      PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
+      0);
+
+  if (out == MAP_FAILED)
+    return NULL;
+
+  i--;
+
+  while (i-- > 0)
+    out[i] = in[i];
+
+  return out;
+}
+
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
 {
@@ -208,6 +232,72 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+static void
+parse_tunables (char *tunestr)
+{
+  if (tunestr == NULL || *tunestr == '\0')
+    return;
+
+  char *p = tunestr;
+
+  while (true)
+    {
+      char *name = p;
+      size_t len = 0;
+
+      /* First, find where the name ends.  */
+      while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
+ len++;
+
+      /* If we reach the end of the string before getting a valid name-value
+ pair, bail out.  */
+      if (p[len] == '\0')
+ return;
+
+      /* We did not find a valid name-value pair before encountering the
+ colon.  */
+      if (p[len]== ':')
+ {
+  p += len + 1;
+  continue;
+ }
+
+      p += len + 1;
+
+      char *value = p;
+      len = 0;
+
+      while (p[len] != ':' && p[len] != '\0')
+ len++;
+
+      char end = p[len];
+      p[len] = '\0';
+
+      /* Add the tunable if it exists.  */
+      for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+ {
+  tunable_t *cur = &tunable_list[i];
+
+  /* If we are in a secure context (AT_SECURE) then ignore the tunable
+     unless it is explicitly marked as secure.  Tunable values take
+     precendence over their envvar aliases.  */
+  if (__libc_enable_secure && !cur->is_secure)
+    continue;
+
+  if (is_name (cur->name, name))
+    {
+      tunable_initialize (cur, value);
+      break;
+    }
+ }
+
+      if (end == ':')
+ p += len + 1;
+      else
+ return;
+    }
+}
+
 /* Disable a tunable if it is set.  */
 static void
 disable_tunable (tunable_id_t id, char **envp)
@@ -216,6 +306,23 @@ disable_tunable (tunable_id_t id, char **envp)
 
   if (env_alias)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
+
+  char *tunable = getenv (GLIBC_TUNABLES);
+  const char *cmp = tunable_list[id].name;
+  const size_t len = strlen (cmp);
+
+  while (tunable && *tunable != '\0' && *tunable != ':')
+    {
+      if (strncmp (tunable, cmp, len) == 0)
+ {
+  tunable += len;
+  /* Overwrite the = and the value with colons.  */
+  while (*tunable != '\0' && *tunable != ':')
+    *tunable++ = ':';
+  break;
+ }
+      tunable++;
+    }
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -246,6 +353,14 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+      if (is_name (GLIBC_TUNABLES, envname))
+ {
+  char *val = tunables_strdup (envval);
+  if (val != NULL)
+    parse_tunables (val);
+  continue;
+ }
+
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
  {
   tunable_t *cur = &tunable_list[i];
diff --git a/malloc/Makefile b/malloc/Makefile
index e7c2cb7..4e4898c 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -33,11 +33,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
  tst-mallocfork2 \
  tst-interpose-nothread \
  tst-interpose-thread \
+ tst-malloc-usable-tunables
 
 tests-static := \
  tst-interpose-static-nothread \
  tst-interpose-static-thread \
- tst-malloc-usable-static
+ tst-malloc-usable-static \
+ tst-malloc-usable-static-tunables
 
 tests += $(tests-static)
 test-srcs = tst-mtrace
@@ -157,6 +159,8 @@ endif
 tst-mcheck-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV)
+tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3
+tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV)
 
 # Uncomment this for test releases.  For public releases it is too expensive.
 #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1
diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-static-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-tunables.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/6] New internal function __access_noerrno

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
Implement an internal version of __access called __access_noerrno that
avoids setting errno.  This is useful to check accessibility of files
very early on in process startup i.e. before TLS setup.  This allows
tunables to replace MALLOC_CHECK_ safely (i.e. check existence of
/etc/suid-debug to enable/disable MALLOC_CHECK) and at the same time
initialize very early so that it can override IFUNCs.

        * include/unistd.h [IS_IN (rtld) || !defined SHARED]: Declare
        __access_noerrno.
        * io/Makefile (routines): Add access_noerrno.
        * io/access.c (__ACCESS)[!__ACCESS]: Define as __access.
        (__access): Rename to __ACCESS.
        [!NOERRNO]: Retain default __access logic.
        * io/access_noerrno.c: New file.
        * sysdeps/mach/hurd/access.c (__ACCESS)[!__ACCESS]: Define as
        __access.
        (__HURD_FAIL): New macro.
        (__access): Rename to __ACCESS.  Use __HURD_FAIL instead of
        __hurd_fail.
        [!NOERRNO]: Set weak alias to access.
        * sysdeps/nacl/access.c (__ACCESS)[!__ACCESS]: Define as
        __access.
        (DO_NACL_CALL): New macro.
        (__access): Rename to __ACCESS.  Use DO_NACL_CALL instead of
        NACL_CALL.
        [!NOERRNO]: Set weak alias to access.
        * sysdeps/nacl/nacl-interfaces.h (NACL_CALL_NOERRNO): New
        macro.
        * sysdeps/unix/access_noerrno.c: New file.
        * sysdeps/unix/sysv/linux/generic/access.c: Include sysdep.h.
        (__ACCESS)[!__ACCESS]: Define as __access.
        (__access): Rename to __ACCESS.
        [NOERRNO]: Call faccessat syscall without setting errno.
---
 include/unistd.h                         |  6 +++++
 io/Makefile                              |  1 +
 io/access.c                              | 10 ++++++++-
 io/access_noerrno.c                      | 21 ++++++++++++++++++
 sysdeps/mach/hurd/access.c               | 20 +++++++++++++----
 sysdeps/nacl/access.c                    | 16 ++++++++++++--
 sysdeps/nacl/nacl-interfaces.h           |  4 ++++
 sysdeps/unix/access_noerrno.c            | 38 ++++++++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/generic/access.c | 19 +++++++++++++++-
 9 files changed, 127 insertions(+), 8 deletions(-)
 create mode 100644 io/access_noerrno.c
 create mode 100644 sysdeps/unix/access_noerrno.c

diff --git a/include/unistd.h b/include/unistd.h
index d2802b2..6144f41 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -181,6 +181,12 @@ extern int __getlogin_r_loginuid (char *name, size_t namesize)
 #   include <dl-unistd.h>
 #  endif
 
+#  if IS_IN (rtld) || !defined SHARED
+/* __access variant that does not set errno.  Used in very early initialization
+   code in libc.a and ld.so.  */
+extern __typeof (__access) __access_noerrno attribute_hidden;
+#  endif
+
 __END_DECLS
 # endif
 
diff --git a/io/Makefile b/io/Makefile
index e5493b3..1ebb0d0 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -40,6 +40,7 @@ routines := \
  mkdir mkdirat \
  open open_2 open64 open64_2 openat openat_2 openat64 openat64_2 \
  read write lseek lseek64 access euidaccess faccessat \
+ access_noerrno \
  fcntl flock lockf lockf64 \
  close dup dup2 dup3 pipe pipe2 \
  creat creat64 \
diff --git a/io/access.c b/io/access.c
index 4534704..80cfa98 100644
--- a/io/access.c
+++ b/io/access.c
@@ -19,10 +19,15 @@
 #include <stddef.h>
 #include <unistd.h>
 
+#ifndef __ACCESS
+# define __ACCESS __access
+#endif
+
 /* Test for access to FILE.  */
 int
-__access (const char *file, int type)
+__ACCESS (const char *file, int type)
 {
+#ifndef NOERRNO
   if (file == NULL || (type & ~(R_OK|W_OK|X_OK|F_OK)) != 0)
     {
       __set_errno (EINVAL);
@@ -30,8 +35,11 @@ __access (const char *file, int type)
     }
 
   __set_errno (ENOSYS);
+#endif
   return -1;
 }
+#ifndef NOERRNO
 stub_warning (access)
 
 weak_alias (__access, access)
+#endif
diff --git a/io/access_noerrno.c b/io/access_noerrno.c
new file mode 100644
index 0000000..4bd531f
--- /dev/null
+++ b/io/access_noerrno.c
@@ -0,0 +1,21 @@
+/* Test for access to a file but do not set errno on error.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define NOERRNO 1
+#define __ACCESS __access_noerrno
+#include <access.c>
diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c
index c308340..189fefe 100644
--- a/sysdeps/mach/hurd/access.c
+++ b/sysdeps/mach/hurd/access.c
@@ -22,9 +22,19 @@
 #include <hurd/lookup.h>
 #include <fcntl.h>
 
+#ifndef __ACCESS
+# define __ACCESS __access
+#endif
+
+#ifndef NOERRNO
+# define __HURD_FAIL(e) __hurd_fail (e)
+#else
+# define __HURD_FAIL(e) (e)
+#endif
+
 /* Test for access to FILE by our real user and group IDs.  */
 int
-__access (const char *file, int type)
+__ACCESS (const char *file, int type)
 {
   error_t err;
   file_t rcrdir, rcwdir, io;
@@ -120,13 +130,13 @@ __access (const char *file, int type)
   if (rcwdir != MACH_PORT_NULL)
     __mach_port_deallocate (__mach_task_self (), rcwdir);
   if (err)
-    return __hurd_fail (err);
+    return __HURD_FAIL (err);
 
   /* Find out what types of access we are allowed to this file.  */
   err = __file_check_access (io, &allowed);
   __mach_port_deallocate (__mach_task_self (), io);
   if (err)
-    return __hurd_fail (err);
+    return __HURD_FAIL (err);
 
   flags = 0;
   if (type & R_OK)
@@ -138,9 +148,11 @@ __access (const char *file, int type)
 
   if (flags & ~allowed)
     /* We are not allowed all the requested types of access.  */
-    return __hurd_fail (EACCES);
+    return __HURD_FAIL (EACCES);
 
   return 0;
 }
 
+#ifndef NOERRNO
 weak_alias (__access, access)
+#endif
diff --git a/sysdeps/nacl/access.c b/sysdeps/nacl/access.c
index 95a0fb7..b7339e7 100644
--- a/sysdeps/nacl/access.c
+++ b/sysdeps/nacl/access.c
@@ -19,10 +19,22 @@
 #include <unistd.h>
 #include <nacl-interfaces.h>
 
+#ifndef __ACCESS
+# define __ACCESS __access
+#endif
+
+#ifndef NOERRNO
+# define DO_NACL_CALL NACL_CALL
+#else
+# define DO_NACL_CALL NACL_CALL_NOERRNO
+#endif
+
 /* Test for access to FILE.  */
 int
-__access (const char *file, int type)
+__ACCESS (const char *file, int type)
 {
-  return NACL_CALL (__nacl_irt_dev_filename.access (file, type), 0);
+  return DO_NACL_CALL (__nacl_irt_dev_filename.access (file, type), 0);
 }
+#ifndef NOERRNO
 weak_alias (__access, access)
+#endif
diff --git a/sysdeps/nacl/nacl-interfaces.h b/sysdeps/nacl/nacl-interfaces.h
index b7b45bb..fd8d580 100644
--- a/sysdeps/nacl/nacl-interfaces.h
+++ b/sysdeps/nacl/nacl-interfaces.h
@@ -113,4 +113,8 @@ __nacl_fail (int err)
 #define NACL_CALL(err, val) \
   ({ int _err = (err); _err ? __nacl_fail (_err) : (val); })
 
+/* Don't set errno.  */
+#define NACL_CALL_NOERRNO(err, val) \
+  ({ int _err = (err); _err ? _err : (val); })
+
 #endif  /* nacl-interfaces.h */
diff --git a/sysdeps/unix/access_noerrno.c b/sysdeps/unix/access_noerrno.c
new file mode 100644
index 0000000..1ff90a2
--- /dev/null
+++ b/sysdeps/unix/access_noerrno.c
@@ -0,0 +1,38 @@
+/* Test for access to a file but do not set errno on error.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Chris Metcalf <[hidden email]>, 2011.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <stddef.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sysdep-cancel.h>
+#include <sysdep.h>
+
+/* Test for access to FILE.  */
+int
+__access_noerrno (const char *file, int type)
+{
+  INTERNAL_SYSCALL_DECL (err);
+  int res;
+  res = INTERNAL_SYSCALL (access, err, 2, file, type);
+  if (INTERNAL_SYSCALL_ERROR_P (res, err))
+    return INTERNAL_SYSCALL_ERRNO (res, err);
+  else
+    return 0;
+}
diff --git a/sysdeps/unix/sysv/linux/generic/access.c b/sysdeps/unix/sysv/linux/generic/access.c
index 586aa93..5bafc06 100644
--- a/sysdeps/unix/sysv/linux/generic/access.c
+++ b/sysdeps/unix/sysv/linux/generic/access.c
@@ -21,11 +21,28 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <sysdep-cancel.h>
+#include <sysdep.h>
+
+#ifndef __ACCESS
+# define __ACCESS __access
+#endif
 
 /* Test for access to FILE.  */
 int
-__access (const char *file, int type)
+__ACCESS (const char *file, int type)
 {
+#ifndef NOERRNO
   return INLINE_SYSCALL (faccessat, 3, AT_FDCWD, file, type);
+#else
+  INTERNAL_SYSCALL_DECL (err);
+  int res;
+  res = INTERNAL_SYSCALL (faccessat, err, 3, AT_FDCWD, file, type);
+  if (INTERNAL_SYSCALL_ERROR_P (res, err))
+    return INTERNAL_SYSCALL_ERRNO (res, err);
+  else
+    return 0;
+#endif
 }
+#ifndef NOERRNO
 weak_alias (__access, access)
+#endif
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 6/6] User manual documentation for tunables

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
Create a new node for tunables documentation and add notes for the
malloc tunables.

        * manual/tunables.texi: New chapter.
        * manual/Makefile (chapters): Add it.
        * manual/probes.texi (@node): Point to the Tunables chapter.
---
 manual/Makefile      |   3 +-
 manual/probes.texi   |   2 +-
 manual/tunables.texi | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 2 deletions(-)
 create mode 100644 manual/tunables.texi

diff --git a/manual/Makefile b/manual/Makefile
index f2f694f..ecc2bf6 100644
--- a/manual/Makefile
+++ b/manual/Makefile
@@ -38,7 +38,8 @@ chapters = $(addsuffix .texi, \
        message search pattern io stdio llio filesys \
        pipe socket terminal syslog math arith time \
        resource setjmp signal startup process ipc job \
-       nss users sysinfo conf crypt debug threads probes)
+       nss users sysinfo conf crypt debug threads \
+       probes tunables)
 add-chapters = $(wildcard $(foreach d, $(add-ons), ../$d/$d.texi))
 appendices = lang.texi header.texi install.texi maint.texi platform.texi \
      contrib.texi
diff --git a/manual/probes.texi b/manual/probes.texi
index 237a918..eb91c62 100644
--- a/manual/probes.texi
+++ b/manual/probes.texi
@@ -1,5 +1,5 @@
 @node Internal Probes
-@c @node Internal Probes, , POSIX Threads, Top
+@c @node Internal Probes, Tunables, POSIX Threads, Top
 @c %MENU% Probes to monitor libc internal behavior
 @chapter Internal probes
 
diff --git a/manual/tunables.texi b/manual/tunables.texi
new file mode 100644
index 0000000..b0ad6897
--- /dev/null
+++ b/manual/tunables.texi
@@ -0,0 +1,184 @@
+@node Tunables
+@c @node Tunables, , Internal Probes, Top
+@c %MENU% Tunable switches to alter libc internal behavior
+@chapter Tunables
+
+Tunables is a feature in @theglibc{} that allows application authors and
+distribution maintainers to alter the runtime library behaviour to match
+their workload. These are implemented as a set of switches that may be
+modified in different ways. The current default method to do this is via
+the @code{GLIBC_TUNABLES} environment variable by setting it to a string
+of colon-separated @code{name=value} pairs.  For example, the following
+environment variable export enables malloc checking and sets the malloc
+trim threshold to 128 bytes.
+
+@code{export GLIBC_TUNABLES=glibc.malloc.trim_threshold=128:glibc.malloc.check=3}
+
+Tunables are not part of the @glibcadj{} stable ABI, and they are
+subject to change or removal across releases.  Additionally, the method to
+modify tunable values may change between releases and across distributions.
+It is possible to implement multiple 'frontends' for the tunables allowing
+distributions to choose their preferred method at build time.
+
+Finally, the set of tunables available may vary between distributions as
+the tunables feature allows distributions to add their own tunables under
+their own namespace.
+
+@menu
+* Tunable names::  The structure of a tunable name
+* Memory Allocation Tunables::  Tunables in the memory allocation subsystem
+@end menu
+
+@node Tunable names
+@section Tunable names
+
+A tunable name is split into three components, a top namespace, a tunable
+namespace and the tunable name. The top namespace for tunables
+implemented in @theglibc{} project is @code{glibc}. Distributions that
+choose to add custom tunables in their maintained versions of @theglibc{}
+may choose to do so under their own top namespace.
+
+The tunable namespace is a logical grouping of tunables in a single
+module. This currently holds no special significance, although that may
+change in future.
+
+The tunable name is the actual name of the tunable. It is possible that
+different tunable namespaces may have tunables within them that have the
+same name, likewise for top namespaces. Hence, we only support
+identification of tunables by their full name, i.e. with the top
+namespace, tunable namespace and tunable name, separated by periods.
+
+@node Memory Allocation Tunables
+@section Memory Allocation Tunables
+
+@deftp Namespace glibc.malloc
+Memory allocation behaviour can be modified by setting any of the
+following tunables in the @code{malloc} namespace.
+@end deftp
+
+@deftp Tunable glibc.malloc.check
+This tunable supersedes the @code{MALLOC_CHECK_} environment variable and is
+identical in features.
+
+Setting this tunable enables a special (less efficient) memory allocator for
+the malloc family of functions that is designed to be tolerant against simple
+errors such as double calls of free with the same argument, or overruns of a
+single byte (off-by-one bugs). Not all such errors can be protected against,
+however, and memory leaks can result.  The following list describes the values
+that this tunable can take and the effect they have on malloc functionality.
+
+@itemize @bullet
+@item @var{0} Disable all error reporting.  The alternate allocator is selected
+and heap corruption detection is in place, but any such errors detected are
+ignored.  This is currently a supported use, but is not recommended.
+@item @var{1} Report errors.  The alternate allocator is selected and heap
+corruption, if detected, is reported as diagnostic messages to @var{stderr} and
+the program continues execution.
+@item @var{2} Abort on errors.  The alternate allocator is selected and if heap
+corruption is detected, the program is ended immediately by calling
+@code{abort}.
+@item @var{3} Fully enabled.  The alternate allocator is selected and is fully
+functional.  That is, if heap corruption is detected, a verbose diagnostic
+message is printed to @var{stderr} and the program is ended by calling
+@code{abort}.
+@end itemize
+
+Like @code{MALLOC_CHECK_}, @code{glibc.malloc.check} has a problem in that it
+diverges from normal program behavior by writing to @var{stderr}, which could
+by exploited in SUID and SGID binaries.  Therefore, @code{glibc.malloc.check}
+is disabled by default for SUID and SGID binaries.  This can be enabled again
+by the system administrator by adding a file @var{/etc/suid-debug}; the content
+of the file could be anything or even empty.
+@end deftp
+
+@deftp Tunable glibc.malloc.top_pad
+This tunable supersedes the @code{MALLOC_TOP_PAD_} environment variable and is
+identical in features.
+
+This tunable determines the amount of extra memory to obtain from the system
+when any of the arenas need to be extended.  It also specifies the number of
+bytes to retain when shrinking any of the arenas.  This provides the necessary
+hysteresis in heap size such that excessive amounts of system calls can be
+avoided.
+
+The default value of this tunable is @samp{0}.
+@end deftp
+
+@deftp Tunable glibc.malloc.perturb
+This tunable supersedes the @code{MALLOC_PERTURB_} environment variable and is
+identical in features.
+
+If set to a non-zero value, memory blocks are initialized with values depending
+on some low order bits of this tunable when they are allocated (except when
+allocated by calloc) and freed.  This can be used to debug the use of
+uninitialized or freed heap memory. Note that this option does not guarantee
+that the freed block will have any specific values. It only guarantees that the
+content the block had before it was freed will be overwritten.
+
+The default value of this tunable is @samp{0}.
+@end deftp
+
+@deftp Tunable glibc.malloc.mmap_threshold
+This tunable supersedes the @code{MALLOC_MMAP_THRESHOLD_} environment variable
+and is identical in features.
+
+When this tunable is set, all chunks larger than this value are allocated
+outside the normal heap, using the @code{mmap} system call. This way it is
+guaranteed that the memory for these chunks can be returned to the system on
+@code{free}. Note that requests smaller than this threshold might still be
+allocated via @code{mmap}.
+
+If this tunable is not set, the default value is set as 128 KB and the
+threshold is adjusted dynamically to suit the allocation patterns of the
+program.  If the tunable is set, the dynamic adjustment is disabled and the
+value is set as static.
+@end deftp
+
+@deftp Tunable glibc.malloc.trim_threshold
+This tunable supersedes the @code{MALLOC_TRIM_THRESHOLD_} environment variable
+and is identical in features.
+
+The value of this tunable is the minimum size (in bytes) of the top-most,
+releasable chunk in an arena that will trigger a system call in order to return
+memory to the system from that arena.
+
+If this tunable is not set, the default value is set as 128 KB and the
+threshold is adjusted dynamically to suit the allocation patterns of the
+program.  If the tunable is set, the dynamic adjustment is disabled and the
+value is set as static.
+@end deftp
+
+@deftp Tunable glibc.malloc.mmap_max
+This tunable supersedes the @code{MALLOC_MMAP_MAX_} environment variable and is
+identical in features.
+
+The value of this tunable is maximum number of chunks to allocate with
+@code{mmap}.  Setting this to zero disables all use of @code{mmap}.
+
+The default value of this tunable is @samp{65536}.
+@end deftp
+
+@deftp Tunable glibc.malloc.arena_test
+This tunable supersedes the @code{MALLOC_ARENA_TEST} environment variable and is
+identical in features.
+
+The @code{glibc.malloc.arena_test} tunable specifies the number of arenas that
+can be created before the test on the limit to the number of arenas is
+conducted.  The value is ignored if @code{glibc.malloc.arena_max} is set.
+
+The default value of this tunable is 2 for 32-bit systems and 8 for 64-bit
+systems.
+@end deftp
+
+@deftp Tunable glibc.malloc.arena_max
+This tunable supersedes the @code{MALLOC_ARENA_MAX} environment variable and is
+identical in features.
+
+This tunable sets the number of arenas to use in a process regardless of the
+number of cores in the system.
+
+The default value of this tunable is @code{0}, meaning that the limit on the
+number of arenas is determined by the number of CPU cores online.  For 32-bit
+systems the limit is twice the number of cores online and on 64-bit systems, it
+is 8 times the number of cores online.
+@end deftp
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/6] Add framework for tunables

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
The tunables framework allows us to uniformly manage and expose global
variables inside glibc as switches to users.  tunables/README has
instructions for glibc developers to add new tunables.

Tunables support can be enabled by passing the --enable-tunables
configure flag to the configure script.  This patch only adds a
framework and does not pose any limitations on how tunable values are
read from the user.  It also adds environment variables used in malloc
behaviour tweaking to the tunables framework as a PoC of the
compatibility interface.

        * manual/install.texi: Add --enable-tunables option.
        * INSTALL: Regenerate.
        * README.tunables: New file.
        * Makeconfig (CPPFLAGS): Define TOP_NAMESPACE.
        (before-compile): Generate dl-tunable-list.h early.
        * config.h.in: Add HAVE_TUNABLES.
        * config.make.in: Add have-tunables.
        * configure.ac: Add --enable-tunables option.
        * configure: Regenerate.
        * csu/init-first.c (__libc_init_first): Move
        __libc_init_secure earlier...
        * csu/init-first.c (LIBC_START_MAIN):... to here.
        [HAVE_TUNABLES]: Include dl-tunables.h.
        (LIBC_START_MAIN) [!SHARED]: Initialize tunables for static
        binaries.
        * elf/Makefile (dl-routines): Add dl-tunables.
        * elf/Versions (ld): Add __tunable_set_val to GLIBC_PRIVATE
        namespace.
        * elf/dl-support (_dl_nondynamic_init): Unset MALLOC_CHECK_
        only when !HAVE_TUNABLES.
        * elf/rtld.c (process_envvars): Likewise.
        * elf/dl-sysdep.c [HAVE_TUNABLES]: Include dl-tunables.h
        (_dl_sysdep_start): Call __tunables_init.
        * elf/dl-tunable-types.h: New file.
        * elf/dl-tunables.c: New file.
        * elf/dl-tunables.h: New file.
        * elf/dl-tunables.list: New file.
        * malloc/tst-malloc-usable-static.c: New test case.
        * malloc/Makefile (tests-static): Add it.
        * malloc/arena.c [HAVE_TUNABLES]: Include dl-tunables.h.
        Define TUNABLE_NAMESPACE.
        (DL_TUNABLE_CALLBACK (set_mallopt_check)): New function.
        (DL_TUNABLE_CALLBACK_FNDECL): New macro.  Use it to define
        callback functions.
        (ptmalloc_init): Set tunable values.
        * scripts/gen-tunables.awk: New file.
        * sysdeps/mach/hurd/dl-sysdep.c [HAVE_TUNABLES]: Include
        dl-tunables.h.
        (_dl_sysdep_start): Call __tunables_init.
---
 INSTALL                           |   5 +
 Makeconfig                        |  16 ++
 README.tunables                   |  84 +++++++++++
 config.h.in                       |   3 +
 config.make.in                    |   1 +
 configure                         |  16 ++
 configure.ac                      |  10 ++
 csu/init-first.c                  |   2 -
 csu/libc-start.c                  |  11 ++
 elf/Makefile                      |   5 +
 elf/Versions                      |   3 +
 elf/dl-support.c                  |   2 +
 elf/dl-sysdep.c                   |   8 +
 elf/dl-tunable-types.h            |  46 ++++++
 elf/dl-tunables.c                 | 310 ++++++++++++++++++++++++++++++++++++++
 elf/dl-tunables.h                 |  78 ++++++++++
 elf/dl-tunables.list              |  69 +++++++++
 elf/rtld.c                        |   2 +
 malloc/Makefile                   |   2 +
 malloc/arena.c                    |  54 +++++++
 malloc/tst-malloc-usable-static.c |   1 +
 manual/install.texi               |   5 +
 scripts/gen-tunables.awk          | 157 +++++++++++++++++++
 sysdeps/mach/hurd/dl-sysdep.c     |   8 +
 24 files changed, 896 insertions(+), 2 deletions(-)
 create mode 100644 README.tunables
 create mode 100644 elf/dl-tunable-types.h
 create mode 100644 elf/dl-tunables.c
 create mode 100644 elf/dl-tunables.h
 create mode 100644 elf/dl-tunables.list
 create mode 100644 malloc/tst-malloc-usable-static.c
 create mode 100644 scripts/gen-tunables.awk

diff --git a/INSTALL b/INSTALL
index b5acedc..ccacccc 100644
--- a/INSTALL
+++ b/INSTALL
@@ -158,6 +158,11 @@ will be used, and CFLAGS sets optimization options for the compiler.
      By default for x86_64, the GNU C Library is built with the vector
      math library.  Use this option to disable the vector math library.
 
+'--enable-tunables'
+     Tunables support allows additional library parameters to be
+     customized at runtime.  This is an experimental feature and affects
+     startup time and is thus disabled by default.
+
 '--build=BUILD-SYSTEM'
 '--host=HOST-SYSTEM'
      These options are for cross-compiling.  If you specify both options
diff --git a/Makeconfig b/Makeconfig
index a785860..7f5d645 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -883,6 +883,11 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \
    $(foreach lib,$(libof-$(basename $(@F))) \
  $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \
    $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F)))
+
+ifeq (yes,$(have-tunables))
+CPPFLAGS += -DTOP_NAMESPACE=glibc
+endif
+
 override CFLAGS = -std=gnu11 -fgnu89-inline $(config-extra-cflags) \
   $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \
   $(sysdep-CFLAGS) $(CFLAGS-$(suffix $@)) $(CFLAGS-$(<F)) \
@@ -1057,6 +1062,17 @@ $(common-objpfx)libc-modules.stmp: $(..)scripts/gen-libc-modules.awk \
 
 endif
 
+# Build the tunables list header early since it could be used by any module in
+# glibc.
+ifeq (yes,$(have-tunables))
+before-compile += $(common-objpfx)dl-tunable-list.h
+
+$(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \
+   $(..)elf/dl-tunables.list
+ $(AWK) -f $^ > $@.tmp
+ mv $@{.tmp,}
+endif
+
 common-generated += libc-modules.h libc-modules.stmp
 
 # The name under which the run-time dynamic linker is installed.
diff --git a/README.tunables b/README.tunables
new file mode 100644
index 0000000..3d9c86a
--- /dev/null
+++ b/README.tunables
@@ -0,0 +1,84 @@
+ TUNABLE FRAMEWORK
+ =================
+
+Tunables is a feature in the GNU C Library that allows application authors and
+distribution maintainers to alter the runtime library behaviour to match their
+workload.
+
+The tunable framework allows modules within glibc to register variables that
+may be tweaked through an environment variable or an API call.  It aims to
+enforce a strict namespace rule to bring consistency to naming of these tunable
+environment variables across the project.  This document is a guide for glibc
+developers to add tunables to the framework.
+
+ADDING A NEW TUNABLE
+--------------------
+
+The TOP_NAMESPACE is defined by default as 'glibc' and it may be overridden in
+distributions for specific tunables if they want to add their own tunables.
+Downstream implementations are discouraged from using the 'glibc' namespace for
+tunables they don't already have consensus to push upstream.
+
+There are two steps to adding a tunable:
+
+1. Add a tunable ID:
+
+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:
+
+TOP_NAMESPACE {
+  NAMESPACE1 {
+    TUNABLE1 {
+      # tunable attributes, one per line
+    }
+    # A tunable with default attributes, i.e. string variable.
+    TUNABLE2
+    TUNABLE3 {
+      # its attributes
+    }
+  }
+  NAMESPACE2 {
+    ...
+  }
+}
+
+The list of allowed attributes are:
+
+- type: Data type.  Defaults to STRING.  Allowed types are:
+ INT_32, SIZE_T and STRING.
+
+- minval: Optional minimum acceptable value.  For a string type
+ this is the minimum length of the value.
+
+- maxval: Optional maximum acceptable value.  For a string type
+ this is the maximum length of the value.
+
+- env_alias: An alias environment variable
+
+- is_secure: Specify whether the tunable should be read for setuid
+ binaries.  True allows the tunable to be read for
+ setuid binaries while false disables it.  Note that
+ even if this is set as true and the value is read, it
+ may not be used if it does not validate against the
+ acceptable values or is not considered safe by the
+ module.
+
+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.
+
+FUTURE WORK
+-----------
+
+The framework currently only allows a one-time initialization of variables
+through environment variables and in some cases, modification of variables via
+an API call.  A future goals for this project include:
+
+- Setting system-wide and user-wide defaults for tunables through some
+  mechanism like a configuration file.
+
+- Allow tweaking of some tunables at runtime
diff --git a/config.h.in b/config.h.in
index 33757bd..fd93bb3 100644
--- a/config.h.in
+++ b/config.h.in
@@ -246,4 +246,7 @@
 /* PowerPC32 uses fctidz for floating point to long long conversions.  */
 #define HAVE_PPC_FCTIDZ 0
 
+/* Build glibc with tunables support.  */
+#define HAVE_TUNABLES 0
+
 #endif
diff --git a/config.make.in b/config.make.in
index 04a8b3e..6d0b607 100644
--- a/config.make.in
+++ b/config.make.in
@@ -93,6 +93,7 @@ use-nscd = @use_nscd@
 build-hardcoded-path-in-tests= @hardcoded_path_in_tests@
 build-pt-chown = @build_pt_chown@
 enable-lock-elision = @enable_lock_elision@
+have-tunables = @have_tunables@
 
 # Build tools.
 CC = @CC@
diff --git a/configure b/configure
index e80e0ad..f3526f4 100755
--- a/configure
+++ b/configure
@@ -662,6 +662,7 @@ multi_arch
 base_machine
 add_on_subdirs
 add_ons
+have_tunables
 build_pt_chown
 build_nscd
 link_obsolete_rpc
@@ -776,6 +777,7 @@ enable_systemtap
 enable_build_nscd
 enable_nscd
 enable_pt_chown
+enable_tunables
 enable_mathvec
 with_cpu
 '
@@ -1443,6 +1445,7 @@ Optional Features:
   --disable-build-nscd    disable building and installing the nscd daemon
   --disable-nscd          library functions will not contact the nscd daemon
   --enable-pt_chown       Enable building and installing pt_chown
+  --enable-tunables       Enable tunables support
   --enable-mathvec        Enable building and installing mathvec [default
                           depends on architecture]
 
@@ -3649,6 +3652,19 @@ if test "$build_pt_chown" = yes; then
 
 fi
 
+# Check whether --enable-tunables was given.
+if test "${enable_tunables+set}" = set; then :
+  enableval=$enable_tunables; have_tunables=$enableval
+else
+  have_tunables=no
+fi
+
+
+if test "$have_tunables" = yes; then
+  $as_echo "#define HAVE_TUNABLES 1" >>confdefs.h
+
+fi
+
 # The abi-tags file uses a fairly simplistic model for name recognition that
 # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu.  So we mutate a
 # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell.
diff --git a/configure.ac b/configure.ac
index a64aeb9..1255b08 100644
--- a/configure.ac
+++ b/configure.ac
@@ -395,6 +395,16 @@ if test "$build_pt_chown" = yes; then
   AC_DEFINE(HAVE_PT_CHOWN)
 fi
 
+AC_ARG_ENABLE([tunables],
+      [AS_HELP_STRING([--enable-tunables],
+       [Enable tunables support])],
+      [have_tunables=$enableval],
+      [have_tunables=no])
+AC_SUBST(have_tunables)
+if test "$have_tunables" = yes; then
+  AC_DEFINE(HAVE_TUNABLES)
+fi
+
 # The abi-tags file uses a fairly simplistic model for name recognition that
 # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu.  So we mutate a
 # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell.
diff --git a/csu/init-first.c b/csu/init-first.c
index 77c6e1c..465f25b 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -72,8 +72,6 @@ _init (int argc, char **argv, char **envp)
   __environ = envp;
 
 #ifndef SHARED
-  __libc_init_secure ();
-
   /* First the initialization which normally would be done by the
      dynamic linker.  */
   _dl_non_dynamic_init ();
diff --git a/csu/libc-start.c b/csu/libc-start.c
index 99c040a..b38039a 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -22,6 +22,10 @@
 #include <ldsodefs.h>
 #include <exit-thread.h>
 
+#if HAVE_TUNABLES
+# include <elf/dl-tunables.h>
+#endif
+
 extern void __libc_init_first (int argc, char **argv, char **envp);
 
 extern int __libc_multiple_libcs;
@@ -184,6 +188,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
     }
 # endif
 
+  /* Initialize very early so that tunables can use it.  */
+  __libc_init_secure ();
+
+#if HAVE_TUNABLES
+  __tunables_init (__environ);
+#endif
+
   /* Perform IREL{,A} relocations.  */
   apply_irel ();
 
diff --git a/elf/Makefile b/elf/Makefile
index caffd92..8d3f4ff 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -35,6 +35,11 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps hwcaps \
 ifeq (yes,$(use-ldconfig))
 dl-routines += dl-cache
 endif
+
+ifeq (yes,$(have-tunables))
+dl-routines += dl-tunables
+endif
+
 all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
 # But they are absent from the shared libc, because that code is in ld.so.
 elide-routines.os = $(all-dl-routines) dl-support enbl-secure dl-origin \
diff --git a/elf/Versions b/elf/Versions
index 23deda9..1734697 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -64,5 +64,8 @@ ld {
 
     # Pointer protection.
     __pointer_chk_guard;
+
+    # Set value of a tunable.
+    __tunable_set_val;
   }
 }
diff --git a/elf/dl-support.c b/elf/dl-support.c
index c30194c..d350d6d 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -354,8 +354,10 @@ _dl_non_dynamic_init (void)
   cp = (const char *) __rawmemchr (cp, '\0') + 1;
  }
 
+#if !HAVE_TUNABLES
       if (__access ("/etc/suid-debug", F_OK) != 0)
  __unsetenv ("MALLOC_CHECK_");
+#endif
     }
 
 #ifdef DL_PLATFORM_INIT
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index eaa7155..68f30f2 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -44,6 +44,10 @@
 #include <hp-timing.h>
 #include <tls.h>
 
+#if HAVE_TUNABLES
+# include <dl-tunables.h>
+#endif
+
 extern char **_environ attribute_hidden;
 extern char _end[] attribute_hidden;
 
@@ -219,6 +223,10 @@ _dl_sysdep_start (void **start_argptr,
     }
 #endif
 
+#if HAVE_TUNABLES
+  __tunables_init (_environ);
+#endif
+
 #ifdef DL_SYSDEP_INIT
   DL_SYSDEP_INIT;
 #endif
diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
new file mode 100644
index 0000000..d1591b6
--- /dev/null
+++ b/elf/dl-tunable-types.h
@@ -0,0 +1,46 @@
+/* Tunable type information.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _TUNABLE_TYPES_H_
+# define _TUNABLE_TYPES_H_
+#include <stddef.h>
+
+typedef void (*tunable_callback_t) (void *);
+
+typedef enum
+{
+  TUNABLE_TYPE_INT_32,
+  TUNABLE_TYPE_SIZE_T,
+  TUNABLE_TYPE_STRING
+} tunable_type_code_t;
+
+typedef struct
+{
+  tunable_type_code_t type_code;
+  int64_t min;
+  int64_t max;
+} tunable_type_t;
+
+typedef union
+{
+  int64_t numval;
+  const char *strval;
+} tunable_val_t;
+
+#endif
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
new file mode 100644
index 0000000..91340b6
--- /dev/null
+++ b/elf/dl-tunables.c
@@ -0,0 +1,310 @@
+/* The tunable framework.  See the README to know how to use the tunable in
+   a glibc module.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <libc-internal.h>
+#include <sysdep.h>
+#include <fcntl.h>
+
+#define TUNABLES_INTERNAL 1
+#include "dl-tunables.h"
+
+/* Compare environment names, bounded by the name hardcoded in glibc.  */
+static bool
+is_name (const char *orig, const char *envname)
+{
+  for (;*orig != '\0' && *envname != '\0'; envname++, orig++)
+    if (*orig != *envname)
+      break;
+
+  /* The ENVNAME is immediately followed by a value.  */
+  if (*orig == '\0' && *envname == '=')
+    return true;
+  else
+    return false;
+}
+
+static char **
+get_next_env (char **envp, char **name, size_t *namelen, char **val)
+{
+  while (envp != NULL && *envp != NULL)
+    {
+      char *envline = *envp;
+      int len = 0;
+
+      while (envline[len] != '\0' && envline[len] != '=')
+ len++;
+
+      /* Just the name and no value, go to the next one.  */
+      if (envline[len] == '\0')
+ continue;
+
+      *name = envline;
+      *namelen = len;
+      *val = &envline[len + 1];
+
+      return ++envp;
+    }
+
+  return NULL;
+}
+
+static int
+tunables_unsetenv (char **ep, const char *name)
+{
+  while (*ep != NULL)
+    {
+      size_t cnt = 0;
+
+      while ((*ep)[cnt] == name[cnt] && name[cnt] != '\0')
+ ++cnt;
+
+      if (name[cnt] == '\0' && (*ep)[cnt] == '=')
+ {
+  /* Found it.  Remove this pointer by moving later ones to
+     the front.  */
+  char **dp = ep;
+
+  do
+    dp[0] = dp[1];
+  while (*dp++);
+  /* Continue the loop in case NAME appears again.  */
+ }
+      else
+ ++ep;
+    }
+
+  return 0;
+}
+
+/* A stripped down strtoul-like implementation for very early use.  It does not
+   set errno if the result is outside bounds because it gets called before
+   errno may have been set up.  */
+static unsigned long int
+tunables_strtoul (const char *nptr)
+{
+  unsigned long int result = 0;
+  long int sign = 1;
+  unsigned max_digit;
+
+  while (*nptr == ' ' || *nptr == '\t')
+    ++nptr;
+
+  if (*nptr == '-')
+    {
+      sign = -1;
+      ++nptr;
+    }
+  else if (*nptr == '+')
+    ++nptr;
+
+  if (*nptr < '0' || *nptr > '9')
+    return 0UL;
+
+  int base = 10;
+  max_digit = 9;
+  if (*nptr == '0')
+    {
+      if (nptr[1] == 'x' || nptr[1] == 'X')
+ {
+  base = 16;
+  nptr += 2;
+ }
+      else
+ {
+  base = 8;
+  max_digit = 7;
+ }
+    }
+
+  while (1)
+    {
+      unsigned long int digval;
+      if (*nptr >= '0' && *nptr <= '0' + max_digit)
+        digval = *nptr - '0';
+      else if (base == 16)
+        {
+  if (*nptr >= 'a' && *nptr <= 'f')
+    digval = *nptr - 'a' + 10;
+  else if (*nptr >= 'A' && *nptr <= 'F')
+    digval = *nptr - 'A' + 10;
+  else
+    break;
+ }
+      else
+        break;
+
+      if (result > ULONG_MAX / base
+  || (result == ULONG_MAX / base && digval > ULONG_MAX % base))
+ return ULONG_MAX;
+      result *= base;
+      result += digval;
+      ++nptr;
+    }
+
+  return result * sign;
+}
+
+/* If the value does not validate, don't bother initializing the internal type
+   and also take care to clear the recorded string value in STRVAL.  */
+#define RETURN_IF_INVALID_RANGE(__cur, __val) \
+({      \
+  __typeof ((__cur)->type.min) min = (__cur)->type.min;      \
+  __typeof ((__cur)->type.max) max = (__cur)->type.max;      \
+  if (min != max && ((__val) < min || (__val) > max))      \
+    return;      \
+})
+
+/* 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)
+{
+  switch (cur->type.type_code)
+    {
+    case TUNABLE_TYPE_INT_32:
+ {
+  int32_t val = (int32_t) tunables_strtoul (strval);
+  RETURN_IF_INVALID_RANGE (cur, val);
+  cur->val.numval = (int64_t) val;
+  cur->strval = strval;
+  break;
+ }
+    case TUNABLE_TYPE_SIZE_T:
+ {
+  size_t val = (size_t) tunables_strtoul (strval);
+  RETURN_IF_INVALID_RANGE (cur, val);
+  cur->val.numval = (int64_t) val;
+  cur->strval = strval;
+  break;
+ }
+    case TUNABLE_TYPE_STRING:
+ {
+  cur->val.strval = cur->strval = strval;
+  break;
+ }
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+/* Disable a tunable if it is set.  */
+static void
+disable_tunable (tunable_id_t id, char **envp)
+{
+  const char *env_alias = tunable_list[id].env_alias;
+
+  if (env_alias)
+    tunables_unsetenv (envp, tunable_list[id].env_alias);
+}
+
+/* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
+   the system administrator overrides it by creating the /etc/suid-debug
+   file.  This is a special case where we want to conditionally enable/disable
+   a tunable even for setuid binaries.  We use the special version of access()
+   to avoid setting ERRNO, which is a TLS variable since TLS has not yet been
+   set up.  */
+static inline void
+__always_inline
+maybe_disable_malloc_check (void)
+{
+  if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) != 0)
+    disable_tunable (TUNABLE_ENUM_NAME(glibc, malloc, check), __environ);
+}
+
+/* Initialize the tunables list from the environment.  For now we only use the
+   ENV_ALIAS to find values.  Later we will also use the tunable names to find
+   values.  */
+void
+__tunables_init (char **envp)
+{
+  char *envname = NULL;
+  char *envval = NULL;
+  size_t len = 0;
+
+  maybe_disable_malloc_check ();
+
+  while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
+    {
+      for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+ {
+  tunable_t *cur = &tunable_list[i];
+
+  /* Skip over tunables that have either been set already or should be
+     skipped.  */
+  if (cur->strval != NULL || cur->env_alias == NULL
+      || (__libc_enable_secure && !cur->is_secure))
+    continue;
+
+  const char *name = cur->env_alias;
+
+  /* We have a match.  Initialize and move on to the next line.  */
+  if (is_name (name, envname))
+    {
+      tunable_initialize (cur, envval);
+      break;
+    }
+ }
+    }
+}
+
+/* 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_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;
+
+  if (valp == NULL)
+    goto cb;
+
+  switch (cur->type.type_code)
+    {
+    case TUNABLE_TYPE_INT_32:
+ {
+  *((int32_t *) valp) = (int32_t) cur->val.numval;
+  break;
+ }
+    case TUNABLE_TYPE_SIZE_T:
+ {
+  *((size_t *) valp) = (size_t) cur->val.numval;
+  break;
+ }
+    case TUNABLE_TYPE_STRING:
+ {
+  *((const char **)valp) = cur->val.strval;
+  break;
+ }
+    default:
+      __builtin_unreachable ();
+    }
+
+cb:
+  if (callback)
+    callback (&cur->val);
+}
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
new file mode 100644
index 0000000..10a644b
--- /dev/null
+++ b/elf/dl-tunables.h
@@ -0,0 +1,78 @@
+/* The tunable framework.  See the README to know how to use the tunable in
+   a glibc module.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _TUNABLES_H_
+# define _TUNABLES_H_
+# include <stddef.h>
+# include "dl-tunable-types.h"
+
+/* A tunable.  */
+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 is_secure; /* Whether the tunable must be read
+   even for setuid binaries.  Note that
+   even if the tunable is read, it may
+   not get used by the target module if
+   the value is considered unsafe.  */
+  /* Compatibility elements.  */
+  const char *env_alias; /* The compatibility environment
+   variable name.  */
+};
+
+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
+
+#include "dl-tunable-list.h"
+
+extern void __tunables_init (char **);
+extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
+
+/* 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) \
+({      \
+  __tunable_set_val      \
+   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
+    NULL);      \
+})
+
+/* Same as TUNABLE_SET_VAL, but also set the callback function to __CB and call
+   it.  */
+# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
+({      \
+  __tunable_set_val      \
+   (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val),      \
+    DL_TUNABLE_CALLBACK (__cb));      \
+})
+
+/* Namespace sanity for callback functions.  Use this macro to keep the
+   namespace of the modules clean.  */
+#define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name
+#endif
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
new file mode 100644
index 0000000..11504c4
--- /dev/null
+++ b/elf/dl-tunables.list
@@ -0,0 +1,69 @@
+# Copyright (C) 2016 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+# Allowed attributes for tunables:
+#
+# type: Defaults to STRING
+# minval: Optional minimum acceptable value
+# maxval: Optional maximum acceptable value
+# env_alias: An alias environment variable
+# is_secure: Specify whether the environment variable should be read for
+# setuid binaries.
+
+glibc {
+  malloc {
+    check {
+      type: INT_32
+      minval: 0
+      maxval: 3
+      env_alias: MALLOC_CHECK_
+      is_secure: true
+    }
+    top_pad {
+      type: SIZE_T
+      env_alias: MALLOC_TOP_PAD_
+    }
+    perturb {
+      type: INT_32
+      minval: 0
+      maxval: 0xff
+      env_alias: MALLOC_PERTURB_
+    }
+    mmap_threshold {
+      type: SIZE_T
+      env_alias: MALLOC_MMAP_THRESHOLD_
+    }
+    trim_threshold {
+      type: SIZE_T
+      env_alias: MALLOC_TRIM_THRESHOLD_
+    }
+    mmap_max {
+      type: INT_32
+      env_alias: MALLOC_MMAP_MAX_
+    }
+    arena_max {
+      type: SIZE_T
+      env_alias: MALLOC_ARENA_MAX
+      minval: 1
+    }
+    arena_test {
+      type: SIZE_T
+      env_alias: MALLOC_ARENA_TEST
+      minval: 1
+    }
+  }
+}
diff --git a/elf/rtld.c b/elf/rtld.c
index 647661c..eb211a2 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2527,11 +2527,13 @@ process_envvars (enum mode *modep)
  }
       while (*nextp != '\0');
 
+#if !HAVE_TUNABLES
       if (__access ("/etc/suid-debug", F_OK) != 0)
  {
   unsetenv ("MALLOC_CHECK_");
   GLRO(dl_debug_mask) = 0;
  }
+#endif
 
       if (mode != normal)
  _exit (5);
diff --git a/malloc/Makefile b/malloc/Makefile
index d79d66f..e7c2cb7 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -37,6 +37,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 tests-static := \
  tst-interpose-static-nothread \
  tst-interpose-static-thread \
+ tst-malloc-usable-static
 
 tests += $(tests-static)
 test-srcs = tst-mtrace
@@ -155,6 +156,7 @@ endif
 
 tst-mcheck-ENV = MALLOC_CHECK_=3
 tst-malloc-usable-ENV = MALLOC_CHECK_=3
+tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV)
 
 # Uncomment this for test releases.  For public releases it is too expensive.
 #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1
diff --git a/malloc/arena.c b/malloc/arena.c
index 9760483..7df7445 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -19,6 +19,11 @@
 
 #include <stdbool.h>
 
+#if HAVE_TUNABLES
+# define TUNABLE_NAMESPACE malloc
+# include <elf/dl-tunables.h>
+#endif
+
 /* Compile-time constants.  */
 
 #define HEAP_MIN_SIZE (32 * 1024)
@@ -204,6 +209,34 @@ __malloc_fork_unlock_child (void)
   __libc_lock_init (list_lock);
 }
 
+#if HAVE_TUNABLES
+static inline int do_set_mallopt_check (int32_t value);
+void
+DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp)
+{
+  int32_t value = *(int32_t *) valp;
+  do_set_mallopt_check (value);
+  if (check_action != 0)
+    __malloc_check_init ();
+}
+
+# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \
+static inline int do_ ## __name (__type value);      \
+void      \
+DL_TUNABLE_CALLBACK (__name) (void *valp)      \
+{      \
+  __type value = *(__type *) valp;      \
+  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)
+#else
 /* Initialization routine. */
 #include <string.h>
 extern char **_environ;
@@ -238,6 +271,7 @@ next_env_entry (char ***position)
 
   return result;
 }
+#endif
 
 
 #ifdef SHARED
@@ -272,6 +306,24 @@ ptmalloc_init (void)
 #endif
 
   thread_arena = &main_arena;
+
+#if HAVE_TUNABLES
+  /* Ensure initialization/consolidation and do it under a lock so that a
+     thread attempting to use the arena in parallel waits on us till we
+     finish.  */
+  __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);
+  __libc_lock_unlock (main_arena.mutex);
+#else
   const char *s = NULL;
   if (__glibc_likely (_environ != NULL))
     {
@@ -340,6 +392,8 @@ ptmalloc_init (void)
       if (check_action != 0)
         __malloc_check_init ();
     }
+#endif
+
 #if HAVE_MALLOC_INIT_HOOK
   void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook);
   if (hook != NULL)
diff --git a/malloc/tst-malloc-usable-static.c b/malloc/tst-malloc-usable-static.c
new file mode 100644
index 0000000..8907db0
--- /dev/null
+++ b/malloc/tst-malloc-usable-static.c
@@ -0,0 +1 @@
+#include <malloc/tst-malloc-usable.c>
diff --git a/manual/install.texi b/manual/install.texi
index de1c203..ffc166d 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -189,6 +189,11 @@ configure with @option{--disable-werror}.
 By default for x86_64, @theglibc{} is built with the vector math library.
 Use this option to disable the vector math library.
 
+@item --enable-tunables
+Tunables support allows additional library parameters to be customized at
+runtime.  This is an experimental feature and affects startup time and is thus
+disabled by default.
+
 @item --build=@var{build-system}
 @itemx --host=@var{host-system}
 These options are for cross-compiling.  If you specify both options and
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
new file mode 100644
index 0000000..b65b5a4
--- /dev/null
+++ b/scripts/gen-tunables.awk
@@ -0,0 +1,157 @@
+# Generate dl-tunable-list.h from dl-tunables.list
+
+BEGIN {
+  tunable=""
+  ns=""
+  top_ns=""
+}
+
+# Skip over blank lines and comments.
+/^#/ {
+  next
+}
+
+/^[ \t]*$/ {
+  next
+}
+
+# Beginning of either a top namespace, tunable namespace or a tunable, decided
+# on the current value of TUNABLE, NS or TOP_NS.
+$2 == "{" {
+  if (top_ns == "") {
+    top_ns = $1
+  }
+  else if (ns == "") {
+    ns = $1
+  }
+  else if (tunable == "") {
+    tunable = $1
+  }
+  else {
+    printf ("Unexpected occurrence of '{': %s:%d\n", FILENAME, FNR)
+    exit 1
+  }
+
+  next
+}
+
+# End of either a top namespace, tunable namespace or a tunable.
+$1 == "}" {
+  if (tunable != "") {
+    # Tunables definition ended, now fill in default attributes.
+    if (!types[top_ns][ns][tunable]) {
+      types[top_ns][ns][tunable] = "STRING"
+    }
+    if (!minvals[top_ns][ns][tunable]) {
+      minvals[top_ns][ns][tunable] = "0"
+    }
+    if (!maxvals[top_ns][ns][tunable]) {
+      maxvals[top_ns][ns][tunable] = "0"
+    }
+    if (!env_alias[top_ns][ns][tunable]) {
+      env_alias[top_ns][ns][tunable] = "NULL"
+    }
+    if (!is_secure[top_ns][ns][tunable]) {
+      is_secure[top_ns][ns][tunable] = "false"
+    }
+
+    tunable = ""
+  }
+  else if (ns != "") {
+    ns = ""
+  }
+  else if (top_ns != "") {
+    top_ns = ""
+  }
+  else {
+    printf ("syntax error: extra }: %s:%d\n", FILENAME, FNR)
+    exit 1
+  }
+  next
+}
+
+# Everything else, which could either be a tunable without any attributes or a
+# tunable attribute.
+{
+  if (ns == "") {
+    printf("Line %d: Invalid tunable outside a namespace: %s\n", NR, $0)
+    exit 1
+  }
+
+  if (tunable == "") {
+    # We encountered a tunable without any attributes, so note it with a
+    # default.
+    types[top_ns][ns][$1] = "STRING"
+    next
+  }
+
+  # Otherwise, we have encountered a tunable attribute.
+  split($0, arr, ":")
+  attr = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[1])
+  val = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[2])
+
+  if (attr == "type") {
+    types[top_ns][ns][tunable] = val
+  }
+  else if (attr == "minval") {
+    minvals[top_ns][ns][tunable] = val
+  }
+  else if (attr == "maxval") {
+    maxvals[top_ns][ns][tunable] = val
+  }
+  else if (attr == "env_alias") {
+    env_alias[top_ns][ns][tunable] = sprintf("\"%s\"", val)
+  }
+  else if (attr == "is_secure") {
+    if (val == "true" || val == "false") {
+      is_secure[top_ns][ns][tunable] = val
+    }
+    else {
+      printf("Line %d: Invalid value (%s) for is_secure: %s, ", NR, val,
+     $0)
+      print("Allowed values are 'true' or 'false'")
+      exit 1
+    }
+  }
+}
+
+END {
+  if (ns != "") {
+    print "Unterminated namespace.  Is a closing brace missing?"
+    exit 1
+  }
+
+  print "/* AUTOGENERATED by gen-tunables.awk.  */"
+  print "#ifndef _TUNABLES_H_"
+  print "# error \"Do not include this file directly.\""
+  print "# error \"Include tunables.h instead.\""
+  print "#endif"
+
+  # Now, the enum names
+  print "\ntypedef enum"
+  print "{"
+  for (t in types) {
+    for (n in types[t]) {
+      for (m in types[t][n]) {
+        printf ("  TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m);
+      }
+    }
+  }
+  print "} tunable_id_t;\n"
+
+  # Finally, the tunable list.
+  print "\n#ifdef TUNABLES_INTERNAL"
+  print "static tunable_t tunable_list[] = {"
+  for (t in types) {
+    for (n in types[t]) {
+      for (m in types[t][n]) {
+        printf ("  {TUNABLE_NAME_S(%s, %s, %s)", t, n, m)
+        printf (", {TUNABLE_TYPE_%s, %s, %s}, {.numval = 0}, NULL, %s, %s},\n",
+ types[t][n][m], minvals[t][n][m], maxvals[t][n][m],
+ is_secure[t][n][m], env_alias[t][n][m]);
+      }
+    }
+  }
+  print "};"
+  print "#endif"
+}
diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
index 76140cf..d2b7422 100644
--- a/sysdeps/mach/hurd/dl-sysdep.c
+++ b/sysdeps/mach/hurd/dl-sysdep.c
@@ -44,6 +44,10 @@
 #include <dl-machine.h>
 #include <dl-procinfo.h>
 
+#if HAVE_TUNABLES
+# include <dl-tunables.h>
+#endif
+
 extern void __mach_init (void);
 
 extern int _dl_argc;
@@ -143,6 +147,10 @@ _dl_sysdep_start (void **start_argptr,
 
       __libc_enable_secure = _dl_hurd_data->flags & EXEC_SECURE;
 
+#if HAVE_TUNABLES
+      __tunables_init (_environ);
+#endif
+
       if (_dl_hurd_data->flags & EXEC_STACK_ARGS &&
   _dl_hurd_data->user_entry == 0)
  _dl_hurd_data->user_entry = (vm_address_t) ENTRY_POINT;
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/6] Enhance --enable-tunables to select tunables frontend at build time

Siddhesh Poyarekar-9
In reply to this post by Siddhesh Poyarekar-9
At the GNU Tools Cauldron 2016, the state of the current tunables
patchset was considered OK with the addition of a way to select the
frontend to be used for the tunables.  That is, to avoid being locked
in to one type of frontend initially, it should be possible to build
tunables with a different frontend with something as simple as a
configure switch.

To that effect, this patch enhances the --enable-tunables option to
accept more values than just 'yes' or 'no'.  The current frontend (and
default when enable-tunables is 'yes') is called 'valstring', to
select the frontend where a single environment variable is set to a
colon-separated value string.  More such frontends can be added in
future.

        * Makeconfig (have-tunables): Check for non-negative instead
        of positive.
        * configure.ac: Add 'valstring' as a valid value for
        --enable-tunables.
        * configure: Regenerate.
        * elf/Makefile (have-tunables): Check for non-negative instead
        of positive.
        (CPPFLAGS-dl-tunables.c): Define TUNABLES_FRONTEND for
        dl-tunables.c.
        * elf/dl-tunables.c (GLIBC_TUNABLES): Define only when
        TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring.
        (tunables_strdup): Likewise.
        (disable_tunables): Likewise.
        (parse_tunables): Likewise.
        (__tunables_init): Process GLIBC_TUNABLES envvar only when.
        TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring.
        * elf/dl-tunables.h (TUNABLES_FRONTEND_valstring): New macro.
        (TUNABLES_FRONTEND_yes): New macro, define as
        TUNABLES_FRONTEND_valstring by default.
        * manual/install.texi: Document new acceptable values for
        --enable-tunables.
        * INSTALL: Regenerate.
---
 INSTALL             | 15 ++++++++++++++-
 Makeconfig          |  4 ++--
 configure           |  3 ++-
 configure.ac        |  2 +-
 elf/Makefile        |  4 +++-
 elf/dl-tunables.c   | 12 +++++++++++-
 elf/dl-tunables.h   |  4 ++++
 manual/install.texi | 18 +++++++++++++++++-
 8 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/INSTALL b/INSTALL
index ccacccc..55f6a23 100644
--- a/INSTALL
+++ b/INSTALL
@@ -161,7 +161,20 @@ will be used, and CFLAGS sets optimization options for the compiler.
 '--enable-tunables'
      Tunables support allows additional library parameters to be
      customized at runtime.  This is an experimental feature and affects
-     startup time and is thus disabled by default.
+     startup time and is thus disabled by default.  This option can take
+     the following values:
+
+        * NO This is the default if the option is not passed to
+          configure.  This disables tunables.
+
+        * YES This is the default if the option is passed to configure.
+          This enables tunables and selects the default frontend
+          (currently VALSTRING).
+
+        * VALSTRING This enables tunables and selects the VALSTRING
+          frontend for tunables.  This frontend allows users to specify
+          tunables as a colon-separated list in a single environment
+          variable GLIBC_TUNABLES.
 
 '--build=BUILD-SYSTEM'
 '--host=HOST-SYSTEM'
diff --git a/Makeconfig b/Makeconfig
index 7f5d645..0eebc92 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -884,7 +884,7 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \
  $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \
    $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F)))
 
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 CPPFLAGS += -DTOP_NAMESPACE=glibc
 endif
 
@@ -1064,7 +1064,7 @@ endif
 
 # Build the tunables list header early since it could be used by any module in
 # glibc.
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 before-compile += $(common-objpfx)dl-tunable-list.h
 
 $(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \
diff --git a/configure b/configure
index f3526f4..8d1da68 100755
--- a/configure
+++ b/configure
@@ -1445,7 +1445,8 @@ Optional Features:
   --disable-build-nscd    disable building and installing the nscd daemon
   --disable-nscd          library functions will not contact the nscd daemon
   --enable-pt_chown       Enable building and installing pt_chown
-  --enable-tunables       Enable tunables support
+  --enable-tunables       Enable tunables support. Known values are 'yes',
+                          'no' and 'valstring'
   --enable-mathvec        Enable building and installing mathvec [default
                           depends on architecture]
 
diff --git a/configure.ac b/configure.ac
index 1255b08..20c419a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -397,7 +397,7 @@ fi
 
 AC_ARG_ENABLE([tunables],
       [AS_HELP_STRING([--enable-tunables],
-       [Enable tunables support])],
+       [Enable tunables support. Known values are 'yes', 'no' and 'valstring'])],
       [have_tunables=$enableval],
       [have_tunables=no])
 AC_SUBST(have_tunables)
diff --git a/elf/Makefile b/elf/Makefile
index 8d3f4ff..5dc3d66 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -36,8 +36,10 @@ ifeq (yes,$(use-ldconfig))
 dl-routines += dl-cache
 endif
 
-ifeq (yes,$(have-tunables))
+ifneq (no,$(have-tunables))
 dl-routines += dl-tunables
+tunables-type = $(addprefix TUNABLES_FRONTEND_,$(have-tunables))
+CPPFLAGS-dl-tunables.c = -DTUNABLES_FRONTEND=$(tunables-type)
 endif
 
 all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 6612885..82e9d9a 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -30,7 +30,9 @@
 #define TUNABLES_INTERNAL 1
 #include "dl-tunables.h"
 
-#define GLIBC_TUNABLES "GLIBC_TUNABLES"
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
+# define GLIBC_TUNABLES "GLIBC_TUNABLES"
+#endif
 
 /* Compare environment names, bounded by the name hardcoded in glibc.  */
 static bool
@@ -47,6 +49,7 @@ is_name (const char *orig, const char *envname)
     return false;
 }
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
 static char *tunables_strdup (const char *in)
 {
   size_t i = 0;
@@ -67,6 +70,7 @@ static char *tunables_strdup (const char *in)
 
   return out;
 }
+#endif
 
 static char **
 get_next_env (char **envp, char **name, size_t *namelen, char **val)
@@ -232,6 +236,7 @@ tunable_initialize (tunable_t *cur, const char *strval)
     }
 }
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
 static void
 parse_tunables (char *tunestr)
 {
@@ -297,6 +302,7 @@ parse_tunables (char *tunestr)
  return;
     }
 }
+#endif
 
 /* Disable a tunable if it is set.  */
 static void
@@ -307,6 +313,7 @@ disable_tunable (tunable_id_t id, char **envp)
   if (env_alias)
     tunables_unsetenv (envp, tunable_list[id].env_alias);
 
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
   char *tunable = getenv (GLIBC_TUNABLES);
   const char *cmp = tunable_list[id].name;
   const size_t len = strlen (cmp);
@@ -323,6 +330,7 @@ disable_tunable (tunable_id_t id, char **envp)
  }
       tunable++;
     }
+#endif
 }
 
 /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless
@@ -353,6 +361,7 @@ __tunables_init (char **envp)
 
   while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL)
     {
+#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
       if (is_name (GLIBC_TUNABLES, envname))
  {
   char *val = tunables_strdup (envval);
@@ -360,6 +369,7 @@ __tunables_init (char **envp)
     parse_tunables (val);
   continue;
  }
+#endif
 
       for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
  {
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 10a644b..27b1b64 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -75,4 +75,8 @@ extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
 /* 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 TUNABLES_FRONTEND_valstring 1
+/* The default value for TUNABLES_FRONTEND.  */
+# define TUNABLES_FRONTEND_yes TUNABLES_FRONTEND_valstring
 #endif
diff --git a/manual/install.texi b/manual/install.texi
index ffc166d..e34c7bf 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -192,7 +192,23 @@ Use this option to disable the vector math library.
 @item --enable-tunables
 Tunables support allows additional library parameters to be customized at
 runtime.  This is an experimental feature and affects startup time and is thus
-disabled by default.
+disabled by default.  This option can take the following values:
+
+@itemize @bullet
+@item @var{no}
+This is the default if the option is not passed to configure. This disables
+tunables.
+
+@item @var{yes}
+This is the default if the option is passed to configure. This enables tunables
+and selects the default frontend (currently @var{valstring}).
+
+@item @var{valstring}
+This enables tunables and selects the @var{valstring} frontend for tunables.
+This frontend allows users to specify tunables as a colon-separated list in a
+single environment variable @var{GLIBC_TUNABLES}.
+
+@end itemize
 
 @item --build=@var{build-system}
 @itemx --host=@var{host-system}
--
2.7.4

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/6] Initialize tunable list with the GLIBC_TUNABLES environment variable

Andreas Schwab
In reply to this post by Siddhesh Poyarekar-9
On Okt 24 2016, Siddhesh Poyarekar <[hidden email]> wrote:

> @@ -44,6 +47,27 @@ is_name (const char *orig, const char *envname)
>      return false;
>  }
>  
> +static char *tunables_strdup (const char *in)

Style: break after return type.

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/6] User manual documentation for tunables

Joseph Myers
In reply to this post by Siddhesh Poyarekar-9
On Mon, 24 Oct 2016, Siddhesh Poyarekar wrote:

> +the @code{GLIBC_TUNABLES} environment variable by setting it to a string

@env.

> +of colon-separated @code{name=value} pairs.  For example, the following

@var{name}=@var{value}.

> +It is possible to implement multiple 'frontends' for the tunables allowing

Use ` as opening quote in Texinfo.

> +@itemize @bullet
> +@item @var{0} Disable all error reporting.  The alternate allocator is selected
> +and heap corruption detection is in place, but any such errors detected are
> +ignored.  This is currently a supported use, but is not recommended.
> +@item @var{1} Report errors.  The alternate allocator is selected and heap
> +corruption, if detected, is reported as diagnostic messages to @var{stderr} and
> +the program continues execution.
> +@item @var{2} Abort on errors.  The alternate allocator is selected and if heap
> +corruption is detected, the program is ended immediately by calling
> +@code{abort}.
> +@item @var{3} Fully enabled.  The alternate allocator is selected and is fully
> +functional.  That is, if heap corruption is detected, a verbose diagnostic
> +message is printed to @var{stderr} and the program is ended by calling
> +@code{abort}.
> +@end itemize

@var is for metasyntactic variables.  It is not for names of literal C
language entities such as stderr (use @code) or for integers such as 0, 1,
2, 3.

> +diverges from normal program behavior by writing to @var{stderr}, which could

Likewise.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/6] Initialize tunable list with the GLIBC_TUNABLES environment variable

Siddhesh Poyarekar-9
In reply to this post by Andreas Schwab
On Monday 24 October 2016 08:31 PM, Andreas Schwab wrote:

> On Okt 24 2016, Siddhesh Poyarekar <[hidden email]> wrote:
>
>> @@ -44,6 +47,27 @@ is_name (const char *orig, const char *envname)
>>      return false;
>>  }
>>  
>> +static char *tunables_strdup (const char *in)
>
> Style: break after return type.
>

Thanks, fixed in my local copy.

Siddhesh

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/6] User manual documentation for tunables

Siddhesh Poyarekar-9
In reply to this post by Joseph Myers
On Monday 24 October 2016 10:51 PM, Joseph Myers wrote:

> On Mon, 24 Oct 2016, Siddhesh Poyarekar wrote:
>
>> +the @code{GLIBC_TUNABLES} environment variable by setting it to a string
>
> @env.
>
>> +of colon-separated @code{name=value} pairs.  For example, the following
>
> @var{name}=@var{value}.
>
>> +It is possible to implement multiple 'frontends' for the tunables allowing
>
> Use ` as opening quote in Texinfo.
>
>> +@itemize @bullet
>> +@item @var{0} Disable all error reporting.  The alternate allocator is selected
>> +and heap corruption detection is in place, but any such errors detected are
>> +ignored.  This is currently a supported use, but is not recommended.
>> +@item @var{1} Report errors.  The alternate allocator is selected and heap
>> +corruption, if detected, is reported as diagnostic messages to @var{stderr} and
>> +the program continues execution.
>> +@item @var{2} Abort on errors.  The alternate allocator is selected and if heap
>> +corruption is detected, the program is ended immediately by calling
>> +@code{abort}.
>> +@item @var{3} Fully enabled.  The alternate allocator is selected and is fully
>> +functional.  That is, if heap corruption is detected, a verbose diagnostic
>> +message is printed to @var{stderr} and the program is ended by calling
>> +@code{abort}.
>> +@end itemize
>
> @var is for metasyntactic variables.  It is not for names of literal C
> language entities such as stderr (use @code) or for integers such as 0, 1,
> 2, 3.
>
>> +diverges from normal program behavior by writing to @var{stderr}, which could
>
> Likewise.
>

Thanks, fixed all this and more in my local copy.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/6] Static inline functions for mallopt helpers

DJ Delorie-2
In reply to this post by Siddhesh Poyarekar-9

-      /* Forbid setting the threshold too high. */

This comment got lost.

It would be nice if the functions were in the same order as the cases in
the switch statement.

Otherwise OK.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/6] Static inline functions for mallopt helpers

Siddhesh Poyarekar-9
On Wednesday 26 October 2016 11:21 PM, DJ Delorie wrote:
>
> -      /* Forbid setting the threshold too high. */
>
> This comment got lost.
>
> It would be nice if the functions were in the same order as the cases in
> the switch statement.
>
> Otherwise OK.

Thanks, I've fixed both points and pushed this patch.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

[PING][PATCH v5 0/6] glibc tunables

Siddhesh Poyarekar-8
In reply to this post by Siddhesh Poyarekar-9
Ping for 2-6/6, I pushed 1/6 since it is an independent cleanup.

Siddhesh

On Monday 24 October 2016 08:12 PM, Siddhesh Poyarekar wrote:

> Hi,
>
> ... and I'm back!
>
> Here is another updated patch set with suggestions from multiple people
> incorporated, including results from discussions at the GNU Tools Cauldron last
> month.
>
>  - I had inadvertently opened a hole with MALLOC_CHECK_ where it could be read
>    in setuid binaries even if /etc/suid-debug was not present.  This is now
>    fixed.
>
>  - __tunables_init is now called *really* early so that tunables are set up
>    before apply_irel is called.  H. J. will now have to split his patch such
>    that his IFUNC resolver looks at both, the tunables and the result of cpuid
>    instead of overriding cpuid using tunables.  I think this is a good thing
>    since it keeps the cpuid information honest and only masks it for the
>    specific purpose of IFUNC.
>
>  - The very early initialization meant that I needed a new version of __access
>    that does not set errno.  That's another patch to the patchset.  This is an
>    internal function, so there is no ABI event here.
>
>  - Enhanced the --enable-tunables option (in a separate patch) to accept string
>    values other than 'yes' and 'no'.  As discussed at Cauldron, this would
>    allow us to experiment with different frontends without committing to one
>    yet.  Right now there is only one frontend, i.e. 'valstring' that allows
>    setting tunables using the single GLIBC_TUNABLES environment variable.
>
>  - Wrote a manual node for tunables.
>
> As usual, the branch siddhesh/tunables has these patches and any patches they
> may depend on (like the doc changes for malloc).
>
> Siddhesh Poyarekar (6):
>   Static inline functions for mallopt helpers
>   New internal function __access_noerrno
>   Add framework for tunables
>   Initialize tunable list with the GLIBC_TUNABLES environment variable
>   Enhance --enable-tunables to select tunables frontend at build time
>   User manual documentation for tunables
>
>  INSTALL                                    |  18 ++
>  Makeconfig                                 |  16 ++
>  README.tunables                            |  84 ++++++
>  config.h.in                                |   3 +
>  config.make.in                             |   1 +
>  configure                                  |  17 ++
>  configure.ac                               |  10 +
>  csu/init-first.c                           |   2 -
>  csu/libc-start.c                           |  11 +
>  elf/Makefile                               |   7 +
>  elf/Versions                               |   3 +
>  elf/dl-support.c                           |   2 +
>  elf/dl-sysdep.c                            |   8 +
>  elf/dl-tunable-types.h                     |  46 +++
>  elf/dl-tunables.c                          | 435 +++++++++++++++++++++++++++++
>  elf/dl-tunables.h                          |  82 ++++++
>  elf/dl-tunables.list                       |  69 +++++
>  elf/rtld.c                                 |   2 +
>  include/unistd.h                           |   6 +
>  io/Makefile                                |   1 +
>  io/access.c                                |  10 +-
>  io/access_noerrno.c                        |  21 ++
>  malloc/Makefile                            |   6 +
>  malloc/arena.c                             |  54 ++++
>  malloc/malloc.c                            | 126 ++++++---
>  malloc/tst-malloc-usable-static-tunables.c |   1 +
>  malloc/tst-malloc-usable-static.c          |   1 +
>  malloc/tst-malloc-usable-tunables.c        |   1 +
>  manual/Makefile                            |   3 +-
>  manual/install.texi                        |  21 ++
>  manual/probes.texi                         |   2 +-
>  manual/tunables.texi                       | 184 ++++++++++++
>  scripts/gen-tunables.awk                   | 157 +++++++++++
>  sysdeps/mach/hurd/access.c                 |  20 +-
>  sysdeps/mach/hurd/dl-sysdep.c              |   8 +
>  sysdeps/nacl/access.c                      |  16 +-
>  sysdeps/nacl/nacl-interfaces.h             |   4 +
>  sysdeps/unix/access_noerrno.c              |  38 +++
>  sysdeps/unix/sysv/linux/generic/access.c   |  19 +-
>  39 files changed, 1469 insertions(+), 46 deletions(-)
>  create mode 100644 README.tunables
>  create mode 100644 elf/dl-tunable-types.h
>  create mode 100644 elf/dl-tunables.c
>  create mode 100644 elf/dl-tunables.h
>  create mode 100644 elf/dl-tunables.list
>  create mode 100644 io/access_noerrno.c
>  create mode 100644 malloc/tst-malloc-usable-static-tunables.c
>  create mode 100644 malloc/tst-malloc-usable-static.c
>  create mode 100644 malloc/tst-malloc-usable-tunables.c
>  create mode 100644 manual/tunables.texi
>  create mode 100644 scripts/gen-tunables.awk
>  create mode 100644 sysdeps/unix/access_noerrno.c
>
Reply | Threaded
Open this post in threaded view
|

[PING 2][PATCH v5 0/6] glibc tunables

Siddhesh Poyarekar-8
Ping!

On Thursday 03 November 2016 03:56 PM, Siddhesh Poyarekar wrote:

> Ping for 2-6/6, I pushed 1/6 since it is an independent cleanup.
>
> Siddhesh
>
> On Monday 24 October 2016 08:12 PM, Siddhesh Poyarekar wrote:
>> Hi,
>>
>> ... and I'm back!
>>
>> Here is another updated patch set with suggestions from multiple people
>> incorporated, including results from discussions at the GNU Tools Cauldron last
>> month.
>>
>>  - I had inadvertently opened a hole with MALLOC_CHECK_ where it could be read
>>    in setuid binaries even if /etc/suid-debug was not present.  This is now
>>    fixed.
>>
>>  - __tunables_init is now called *really* early so that tunables are set up
>>    before apply_irel is called.  H. J. will now have to split his patch such
>>    that his IFUNC resolver looks at both, the tunables and the result of cpuid
>>    instead of overriding cpuid using tunables.  I think this is a good thing
>>    since it keeps the cpuid information honest and only masks it for the
>>    specific purpose of IFUNC.
>>
>>  - The very early initialization meant that I needed a new version of __access
>>    that does not set errno.  That's another patch to the patchset.  This is an
>>    internal function, so there is no ABI event here.
>>
>>  - Enhanced the --enable-tunables option (in a separate patch) to accept string
>>    values other than 'yes' and 'no'.  As discussed at Cauldron, this would
>>    allow us to experiment with different frontends without committing to one
>>    yet.  Right now there is only one frontend, i.e. 'valstring' that allows
>>    setting tunables using the single GLIBC_TUNABLES environment variable.
>>
>>  - Wrote a manual node for tunables.
>>
>> As usual, the branch siddhesh/tunables has these patches and any patches they
>> may depend on (like the doc changes for malloc).
>>
>> Siddhesh Poyarekar (6):
>>   Static inline functions for mallopt helpers
>>   New internal function __access_noerrno
>>   Add framework for tunables
>>   Initialize tunable list with the GLIBC_TUNABLES environment variable
>>   Enhance --enable-tunables to select tunables frontend at build time
>>   User manual documentation for tunables
>>
>>  INSTALL                                    |  18 ++
>>  Makeconfig                                 |  16 ++
>>  README.tunables                            |  84 ++++++
>>  config.h.in                                |   3 +
>>  config.make.in                             |   1 +
>>  configure                                  |  17 ++
>>  configure.ac                               |  10 +
>>  csu/init-first.c                           |   2 -
>>  csu/libc-start.c                           |  11 +
>>  elf/Makefile                               |   7 +
>>  elf/Versions                               |   3 +
>>  elf/dl-support.c                           |   2 +
>>  elf/dl-sysdep.c                            |   8 +
>>  elf/dl-tunable-types.h                     |  46 +++
>>  elf/dl-tunables.c                          | 435 +++++++++++++++++++++++++++++
>>  elf/dl-tunables.h                          |  82 ++++++
>>  elf/dl-tunables.list                       |  69 +++++
>>  elf/rtld.c                                 |   2 +
>>  include/unistd.h                           |   6 +
>>  io/Makefile                                |   1 +
>>  io/access.c                                |  10 +-
>>  io/access_noerrno.c                        |  21 ++
>>  malloc/Makefile                            |   6 +
>>  malloc/arena.c                             |  54 ++++
>>  malloc/malloc.c                            | 126 ++++++---
>>  malloc/tst-malloc-usable-static-tunables.c |   1 +
>>  malloc/tst-malloc-usable-static.c          |   1 +
>>  malloc/tst-malloc-usable-tunables.c        |   1 +
>>  manual/Makefile                            |   3 +-
>>  manual/install.texi                        |  21 ++
>>  manual/probes.texi                         |   2 +-
>>  manual/tunables.texi                       | 184 ++++++++++++
>>  scripts/gen-tunables.awk                   | 157 +++++++++++
>>  sysdeps/mach/hurd/access.c                 |  20 +-
>>  sysdeps/mach/hurd/dl-sysdep.c              |   8 +
>>  sysdeps/nacl/access.c                      |  16 +-
>>  sysdeps/nacl/nacl-interfaces.h             |   4 +
>>  sysdeps/unix/access_noerrno.c              |  38 +++
>>  sysdeps/unix/sysv/linux/generic/access.c   |  19 +-
>>  39 files changed, 1469 insertions(+), 46 deletions(-)
>>  create mode 100644 README.tunables
>>  create mode 100644 elf/dl-tunable-types.h
>>  create mode 100644 elf/dl-tunables.c
>>  create mode 100644 elf/dl-tunables.h
>>  create mode 100644 elf/dl-tunables.list
>>  create mode 100644 io/access_noerrno.c
>>  create mode 100644 malloc/tst-malloc-usable-static-tunables.c
>>  create mode 100644 malloc/tst-malloc-usable-static.c
>>  create mode 100644 malloc/tst-malloc-usable-tunables.c
>>  create mode 100644 manual/tunables.texi
>>  create mode 100644 scripts/gen-tunables.awk
>>  create mode 100644 sysdeps/unix/access_noerrno.c
>>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/6] New internal function __access_noerrno

Adhemerval Zanella-2
In reply to this post by Siddhesh Poyarekar-9


On 24/10/2016 12:42, Siddhesh Poyarekar wrote:

> Implement an internal version of __access called __access_noerrno that
> avoids setting errno.  This is useful to check accessibility of files
> very early on in process startup i.e. before TLS setup.  This allows
> tunables to replace MALLOC_CHECK_ safely (i.e. check existence of
> /etc/suid-debug to enable/disable MALLOC_CHECK) and at the same time
> initialize very early so that it can override IFUNCs.
>
> * include/unistd.h [IS_IN (rtld) || !defined SHARED]: Declare
> __access_noerrno.
> * io/Makefile (routines): Add access_noerrno.
> * io/access.c (__ACCESS)[!__ACCESS]: Define as __access.
> (__access): Rename to __ACCESS.
> [!NOERRNO]: Retain default __access logic.
> * io/access_noerrno.c: New file.
> * sysdeps/mach/hurd/access.c (__ACCESS)[!__ACCESS]: Define as
> __access.
> (__HURD_FAIL): New macro.
> (__access): Rename to __ACCESS.  Use __HURD_FAIL instead of
> __hurd_fail.
> [!NOERRNO]: Set weak alias to access.
> * sysdeps/nacl/access.c (__ACCESS)[!__ACCESS]: Define as
> __access.
> (DO_NACL_CALL): New macro.
> (__access): Rename to __ACCESS.  Use DO_NACL_CALL instead of
> NACL_CALL.
> [!NOERRNO]: Set weak alias to access.
> * sysdeps/nacl/nacl-interfaces.h (NACL_CALL_NOERRNO): New
> macro.
> * sysdeps/unix/access_noerrno.c: New file.
> * sysdeps/unix/sysv/linux/generic/access.c: Include sysdep.h.
> (__ACCESS)[!__ACCESS]: Define as __access.
> (__access): Rename to __ACCESS.
> [NOERRNO]: Call faccessat syscall without setting errno.
> ---
>  include/unistd.h                         |  6 +++++
>  io/Makefile                              |  1 +
>  io/access.c                              | 10 ++++++++-
>  io/access_noerrno.c                      | 21 ++++++++++++++++++
>  sysdeps/mach/hurd/access.c               | 20 +++++++++++++----
>  sysdeps/nacl/access.c                    | 16 ++++++++++++--
>  sysdeps/nacl/nacl-interfaces.h           |  4 ++++
>  sysdeps/unix/access_noerrno.c            | 38 ++++++++++++++++++++++++++++++++
>  sysdeps/unix/sysv/linux/generic/access.c | 19 +++++++++++++++-
>  9 files changed, 127 insertions(+), 8 deletions(-)
>  create mode 100644 io/access_noerrno.c
>  create mode 100644 sysdeps/unix/access_noerrno.c
>
> diff --git a/include/unistd.h b/include/unistd.h
> index d2802b2..6144f41 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -181,6 +181,12 @@ extern int __getlogin_r_loginuid (char *name, size_t namesize)
>  #   include <dl-unistd.h>
>  #  endif
>  
> +#  if IS_IN (rtld) || !defined SHARED
> +/* __access variant that does not set errno.  Used in very early initialization
> +   code in libc.a and ld.so.  */

Is this comment correct? Checking the patch I am seeing it builds
only for libc and there is no resulting __access_noerrno on ld.so.

> diff --git a/sysdeps/unix/access_noerrno.c b/sysdeps/unix/access_noerrno.c
> new file mode 100644
> index 0000000..1ff90a2
> --- /dev/null
> +++ b/sysdeps/unix/access_noerrno.c
> @@ -0,0 +1,38 @@
> +/* Test for access to a file but do not set errno on error.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by Chris Metcalf <[hidden email]>, 2011.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <stddef.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sysdep-cancel.h>
> +#include <sysdep.h>
> +
> +/* Test for access to FILE.  */
> +int
> +__access_noerrno (const char *file, int type)
> +{
> +  INTERNAL_SYSCALL_DECL (err);
> +  int res;
> +  res = INTERNAL_SYSCALL (access, err, 2, file, type);
> +  if (INTERNAL_SYSCALL_ERROR_P (res, err))
> +    return INTERNAL_SYSCALL_ERRNO (res, err);
> +  else
> +    return 0;
> +}

I think it would be simpler to just 1. consolidation Linux access
implementation and 2. add the '_noerrno' on same file.

The 1. would be simpler to just:

   1.1. Remove access from sysdeps/unix/syscalls.list
   1.2. Move sysdeps/unix/sysv/linux/generic/access.c to
        sysdeps/unix/sysv/linux/access.c
   1.3. And implement access checking for __NR_access and
        using __NR_facessat if is not defined.  Something
        like:

[...]
/* Test for access to FILE.  */
int
__access (const char *file, int type)
{
#if __NR_access
  return INLINE_SYSCALL_CALL (access, file, type);
#else
  return INLINE_SYSCALL_CALL (faccessat, AT_FDCWD, file, type);
#endif
}
[...]

Then __access_noerro could be just implemented on same file.
I think it has the advantage of not splitting the access
on multiple files.

I think for hurd and nacl it would also result in simplify
code.  What do you think?


> diff --git a/sysdeps/unix/sysv/linux/generic/access.c b/sysdeps/unix/sysv/linux/generic/access.c
> index 586aa93..5bafc06 100644
> --- a/sysdeps/unix/sysv/linux/generic/access.c
> +++ b/sysdeps/unix/sysv/linux/generic/access.c
> @@ -21,11 +21,28 @@
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <sysdep-cancel.h>
> +#include <sysdep.h>
> +
> +#ifndef __ACCESS
> +# define __ACCESS __access
> +#endif
>  
>  /* Test for access to FILE.  */
>  int
> -__access (const char *file, int type)
> +__ACCESS (const char *file, int type)
>  {
> +#ifndef NOERRNO
>    return INLINE_SYSCALL (faccessat, 3, AT_FDCWD, file, type);
> +#else
> +  INTERNAL_SYSCALL_DECL (err);
> +  int res;
> +  res = INTERNAL_SYSCALL (faccessat, err, 3, AT_FDCWD, file, type);
> +  if (INTERNAL_SYSCALL_ERROR_P (res, err))
> +    return INTERNAL_SYSCALL_ERRNO (res, err);
> +  else
> +    return 0;
> +#endif
>  }
> +#ifndef NOERRNO
>  weak_alias (__access, access)
> +#endif
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/6] New internal function __access_noerrno

Siddhesh Poyarekar-8
On Tuesday 08 November 2016 10:39 PM, Adhemerval Zanella wrote:
> Is this comment correct? Checking the patch I am seeing it builds
> only for libc and there is no resulting __access_noerrno on ld.so.

It builds for both.  rtld-Rules has a rule that looks at all of the libc
functions that rtld uses and rebuilds all of them inside ld.so.

$ nm elf/ld.so | grep access_noerrno
00000000000199d0 t __access_noerrno

> I think it would be simpler to just 1. consolidation Linux access
> implementation and 2. add the '_noerrno' on same file.
>
> The 1. would be simpler to just:
>
>    1.1. Remove access from sysdeps/unix/syscalls.list
>    1.2. Move sysdeps/unix/sysv/linux/generic/access.c to
> sysdeps/unix/sysv/linux/access.c
>    1.3. And implement access checking for __NR_access and
> using __NR_facessat if is not defined.  Something
> like:
>
> [...]
> /* Test for access to FILE.  */
> int
> __access (const char *file, int type)
> {
> #if __NR_access
>   return INLINE_SYSCALL_CALL (access, file, type);
> #else
>   return INLINE_SYSCALL_CALL (faccessat, AT_FDCWD, file, type);
> #endif
> }
> [...]
>
> Then __access_noerro could be just implemented on same file.
> I think it has the advantage of not splitting the access
> on multiple files.
>
> I think for hurd and nacl it would also result in simplify
> code.  What do you think?

The nacl code can be hacked in there somehow I guess, but the hurd code
seems quite different and I am a bit reluctant to hack at it without
access to a hurd instance to test on.

I can hack at sysdeps/unix/sysv/linux/access.c and drop
sysdeps/unix/access_noerrno.c.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/6] New internal function __access_noerrno

Siddhesh Poyarekar-8
On Wednesday 09 November 2016 12:30 AM, Siddhesh Poyarekar wrote:
> I can hack at sysdeps/unix/sysv/linux/access.c and drop
> sysdeps/unix/access_noerrno.c.

Turns out I can't do this because I will be breaking non-Linux
configurations that use the SYS_ prefix instead of the __NR_ prefix.  I
also discovered that I hadn't actually tested the code properly on
aarch64 and it is broken, so I'll resend the patchset.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/6] New internal function __access_noerrno

Adhemerval Zanella-2


On 10/11/2016 03:44, Siddhesh Poyarekar wrote:

> On Wednesday 09 November 2016 12:30 AM, Siddhesh Poyarekar wrote:
>> I can hack at sysdeps/unix/sysv/linux/access.c and drop
>> sysdeps/unix/access_noerrno.c.
>
> Turns out I can't do this because I will be breaking non-Linux
> configurations that use the SYS_ prefix instead of the __NR_ prefix.  I
> also discovered that I hadn't actually tested the code properly on
> aarch64 and it is broken, so I'll resend the patchset.
>
> Siddhesh

In this branch [1] I have a WIP for this.  First patch basically
consolidates access Linux implementation and it uses the strategy
to check for __NR_access first (and since it on sysdeps/.../linux
it should not worry about __SYS_*).

Second one is the __access_errno implementation.  It basically handles
the 3 cases (hurd, nacl, and Linux) by adding the __access_errno on
access.c.

I think it should a cleaner solution to __access_noerro (less macro
handling and build objects).


[1] https://github.com/zatrazz/glibc/commits/master-access_noerrno
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/6] New internal function __access_noerrno

Siddhesh Poyarekar-8
On Thursday 10 November 2016 05:44 PM, Adhemerval Zanella wrote:
> In this branch [1] I have a WIP for this.  First patch basically
> consolidates access Linux implementation and it uses the strategy
> to check for __NR_access first (and since it on sysdeps/.../linux
> it should not worry about __SYS_*).

Ahh, you meant sysdeps/unix/sysv/linux, I thought you meant
sysdeps/unix.  Yes, that makes sense.  Can you please post that on the
list for review?

> Second one is the __access_errno implementation.  It basically handles
> the 3 cases (hurd, nacl, and Linux) by adding the __access_errno on
> access.c.
>
> I think it should a cleaner solution to __access_noerro (less macro
> handling and build objects).

Sure, I don't mind this kind of implementation either.  Do you want to
post this one for review too?

Thanks,
Siddhesh
12