diff mbox

[RFC] tunables: Add elision tunable

Message ID 27740ddf-7a0e-4326-db01-44e5b30fcafb@linux.vnet.ibm.com
State Not applicable
Headers

Commit Message

Stefan Liebler Sept. 12, 2017, 9:35 a.m. UTC 
  On 09/11/2017 03:25 PM, rcardoso wrote:
> Hello,
> 
> Attached to this mail I have a patch which add elision to tunables 
> framework. This is a rebase of Paul Murphy's patch [1]. Also there's a 
> RFC related to this [2].
> 
> [1] https://patchwork.sourceware.org/patch/10342/
> [2] https://patchwork.sourceware.org/patch/20407/


Hi Rogerio,

Thanks for working on this topic.
In addition to Paul's patches, here is a link to a newer patch from Carlos:
"RFC: Always-on elision with per-process opt-in using tunables."
https://www.sourceware.org/ml/libc-alpha/2017-05/msg00335.html

He also removes the --enable-lock-elision configure flag.
Is this also required for your patch?

I've applied your patch to test it on s390x.
Some changes are needed in order to get it work:



Note: I haven't tested it on power or x86.


With these mentioned changes and the comments below,
I could build glibc and the testsuite runs without regressions.

I've also used a small s390 specific test program to check wether a 
mutex was elided or not:
./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
Lock was elided via a transaction! (nesting-depth=1)

GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=2 ./prog
Lock was a normal lock!

GLIBC_TUNABLES=glibc.elision.enable=yes ./prog
Lock was a normal lock!

Note: I haven't tested setting the other variables.

Thanks,
Stefan

Comments

Gabriel F. T. Gomes Sept. 13, 2017, 1:42 p.m. UTC | #1 
On 12 Sep 2017, Stefan Liebler wrote:

>I've also used a small s390 specific test program to check wether a 
>mutex was elided or not:
>./prog
>Lock was a normal lock!
>
>GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
>Lock was a normal lock!
>
>GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
>Lock was elided via a transaction! (nesting-depth=1)
>
>GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
>Lock was a normal lock!

Would you be willing to share this test program (and your build
configurations)?  I understand that it is s390-specific, but it could help
anyway (at least for my education, it will).
Stefan Liebler Sept. 13, 2017, 2:13 p.m. UTC | #2 
On 09/13/2017 03:42 PM, Gabriel F. T. Gomes wrote:
> On 12 Sep 2017, Stefan Liebler wrote:
> 
>> I've also used a small s390 specific test program to check wether a
>> mutex was elided or not:
>> ./prog
>> Lock was a normal lock!
>>
>> GLIBC_TUNABLES=glibc.elision.enable=0 ./prog
>> Lock was a normal lock!
>>
>> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog
>> Lock was elided via a transaction! (nesting-depth=1)
>>
>> GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure
>> Lock was a normal lock!
> 
> Would you be willing to share this test program (and your build
> configurations)?  I understand that it is s390-specific, but it could help
> anyway (at least for my education, it will).
> 

Yes sure, please see the notes in the attached file.

Bye
Stefan
diff mbox

Patch

diff --git a/manual/tunables.texi b/manual/tunables.texi
index 166624c..fb845b1 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -211,6 +211,7 @@  bounded, the user may set this tunable to limit the 
number of chunks
  that are scanned from the unsorted list while searching for chunks to
  pre-fill the per-thread cache with.  The default, or when set to zero,
  is no limit.
+@end deftp

  @node Elision Tunables
  @section Elision Tunables
diff --git a/sysdeps/unix/sysv/linux/s390/elision-conf.c 
b/sysdeps/unix/sysv/linux/s390/elision-conf.c
index 5f9c6d1..3dcf580 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-conf.c
@@ -21,9 +21,9 @@ 
  #include <elision-conf.h>
  #include <unistd.h>
  #include <dl-procinfo.h>
-#ifdef HAVE_TUNABLES
-# include <elf/dl-tunables.h>
+#if HAVE_TUNABLES
  # define TUNABLE_NAMESPACE elision
+# include <elf/dl-tunables.h>
  #endif

  /* Reasonable initial tuning values, may be revised in the future.
@@ -57,7 +57,7 @@  struct elision_config __elision_aconf =

  int __pthread_force_elision attribute_hidden = 0;

-#ifdef HAVE_TUNABLES
+#if HAVE_TUNABLES
  static inline void
  __always_inline
  do_set_elision_enable (int32_t elision_enable)
@@ -106,7 +106,7 @@  elision_init (int argc __attribute__ ((unused)),
               char **argv  __attribute__ ((unused)),
               char **environ)
  {
-#ifdef HAVE_TUNABLES
+#if HAVE_TUNABLES
    /* Elision depends on tunables and must be explicitly turned on by 
setting
       the appropriate tunable on a supported platform.  */