Patchwork [1/4] Add framework for tunables

login
register
mail settings
Submitter Siddhesh Poyarekar
Date Feb. 7, 2017, 6:49 a.m.
Message ID <b8437a22-61eb-7d2e-a472-6ec0ca635291@sourceware.org>
Download mbox | patch
Permalink /patch/19135/
State New
Headers show

Comments

Siddhesh Poyarekar - Feb. 7, 2017, 6:49 a.m.
On Tuesday 07 February 2017 04:06 AM, Andreas Schwab wrote:
> On Dez 30 2016, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:
> 
>> diff --git a/malloc/arena.c b/malloc/arena.c
>> index eed4247..234035f 100644
>> --- a/malloc/arena.c
>> +++ b/malloc/arena.c
>> @@ -19,6 +19,11 @@
>>  
>>  #include <stdbool.h>
>>  
>> +#if HAVE_TUNABLES
>> +# define TUNABLE_NAMESPACE malloc
>> +#endif
>> +#include <elf/dl-tunables.h>
>> +
>>  /* 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;
> 
> This is wrong.  The callback receives a pointer to tunable_val_t.

Does the attached (sanity tested on little-endian) patch work for you?

Siddhesh
Andreas Schwab - Feb. 7, 2017, 4:05 p.m.
On Feb 07 2017, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:

> Does the attached (sanity tested on little-endian) patch work for you?

https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc
https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc64
https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/s/s390x

All four tst-malloc-usable tests are now passing.

Andreas.
Siddhesh Poyarekar - Feb. 8, 2017, 8:48 a.m.
On Tuesday 07 February 2017 09:35 PM, Andreas Schwab wrote:
> On Feb 07 2017, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:
> 
>> Does the attached (sanity tested on little-endian) patch work for you?
> 
> https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc
> https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc64
> https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/s/s390x
> 
> All four tst-malloc-usable tests are now passing.

Thanks, I have pushed the patch now.

Siddhesh

Patch

diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index a986f0b..37a4e80 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -21,8 +21,6 @@ 
 # define _TUNABLE_TYPES_H_
 #include <stddef.h>
 
-typedef void (*tunable_callback_t) (void *);
-
 typedef enum
 {
   TUNABLE_TYPE_INT_32,
@@ -43,6 +41,8 @@  typedef union
   const char *strval;
 } tunable_val_t;
 
+typedef void (*tunable_callback_t) (tunable_val_t *);
+
 /* Security level for tunables.  This decides what to do with individual
    tunables for AT_SECURE binaries.  */
 typedef enum
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index a8d53d6..e42aa67 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -455,6 +455,8 @@  __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
   if (cur->strval == NULL)
     return;
 
+  /* Caller does not need the value, just call the callback with our tunable
+     value.  */
   if (valp == NULL)
     goto cb;
 
diff --git a/malloc/arena.c b/malloc/arena.c
index b91d7d6..d49e4a2 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -212,9 +212,9 @@  __malloc_fork_unlock_child (void)
 #if HAVE_TUNABLES
 static inline int do_set_mallopt_check (int32_t value);
 void
-DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp)
+DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
 {
-  int32_t value = *(int32_t *) valp;
+  int32_t value = (int32_t) valp->numval;
   do_set_mallopt_check (value);
   if (check_action != 0)
     __malloc_check_init ();
@@ -223,9 +223,9 @@  DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp)
 # define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \
 static inline int do_ ## __name (__type value);				      \
 void									      \
-DL_TUNABLE_CALLBACK (__name) (void *valp)				      \
+DL_TUNABLE_CALLBACK (__name) (tunable_val_t *valp)			      \
 {									      \
-  __type value = *(__type *) valp;					      \
+  __type value = (__type) (valp)->numval;				      \
   do_ ## __name (value);						      \
 }