At the moment --enable-lock-elision is already specified while configuring glibc for Linux on z Systems. Thus pthread_mutex_t locks are elided with transactions if the CPU and the kernel supports transactional execution. A user has no chance to simply compare performance of his application with / without eliding pthread_mutex_t locks. The attached patch disables the elision by default even if --enable-lock-elision was specified at configure time and enables it only if the specific environment variable LOCK_ELISION_ENABLE=yes is set. Otherwise it falls back to standard locks.
Created attachment 1209526 [details] Patch disables lock elision by default and enables it with environment variable.
Florian, what do you thing about this request? I can't see this patch in upstream glibc, but is it possible to carry it as a downstream patch? For a limited period of time? How about having same feature on Power?
(In reply to Dan Horák from comment #8) > Florian, what do you thing about this request? I can't see this patch in > upstream glibc, but is it possible to carry it as a downstream patch? For a > limited period of time? How about having same feature on Power? I just commented on the POWER bug for this. The upstream elision is in place for POWER and z Systems, but the env var to enable is not. This needs to go upstream first e.g. * --enable-lock-elision should just enable the code but not turn elision on. * Opt-in env var e.g. FEDORA_GLIBC_TUNABLE='elision=1' should turn on elision for the application.
I think we could carry a downstream FEDORA_GLIBC_TUNABLE hack to just parse elision env var information (compatible with upstream tunable framework as it is currently proposed).
------- Comment From mgrf.com 2017-02-15 10:13 EDT------- Any update on this ?
With the tunables framework available in 2.25 what should be next step with this feature?
(In reply to Dan Horák from comment #12) > With the tunables framework available in 2.25 what should be next step with > this feature? We need to disable elision by default upstream (even if compiled in) and implement a tunable to switch it on. As far as I know, there is no patch for that yet.
Created attachment 1250771 [details] elision-upstream-wip.diff
(In reply to Florian Weimer from comment #13) > (In reply to Dan Horák from comment #12) > > With the tunables framework available in 2.25 what should be next step with > > this feature? > > We need to disable elision by default upstream (even if compiled in) and > implement a tunable to switch it on. As far as I know, there is no patch > for that yet. I've attached my WIP patch to use tunables for this, and it's probably close to what I'll propose upstream. The patch does: - Always builds with elision support. - Runs with elision disabled by default. - Allows tunable to enable elision per-process.
This bug appears to have been reported against 'rawhide' during the Fedora 26 development cycle. Changing version to '26'.
Created attachment 1267814 [details] elision-upstream-wip_s390ontop.diff ------- Comment on attachment From STLI.com 2017-03-31 07:22 EDT------- Have you already posted this patch on glibc mailinglist? Unfortunately I haven't recognized it. Thus I've applied the patch elision-upstream-wip.diff on glibc-master (not fedora-srpm-glibc) in order to test it. Here are some comments: elf/dl-tunables.list: You've specified "is_secure: false". After recent upstream commits "Update old tunables framework document/script." (65eff7fbdbddad8c1f9af7cb48cd3b5dca3c5c9d) and before, this should be be replaced by "security_level: SXID_ERASE or SXID_IGNORE". In s390 / ppc elision-conf.c: -Missing inclusion of elf/dl-tunables.h. -The name of argument of DL_TUNABLE_CALLBACK (set_elsion_enable) should be valp instead of elision_enable. After upstream commit "Fix getting tunable values on big-endian (BZ #21109)" (8cbc826c37c0221ada65a7a622fe079b4e89a4b0), the argument must be a pointer to tunable_val_t instead of void. You've removed ENABLE_LOCK_ELISION in config.h.in, but this define is used: ./nptl/tst-mutex8.c:132:#ifndef ENABLE_LOCK_ELISION ./nptl/tst-mutex8.c:160:#ifndef ENABLE_LOCK_ELISION ./nptl/tst-mutex8.c:210:#ifndef ENABLE_LOCK_ELISION ./nptl/tst-mutex8.c:283:#ifndef ENABLE_LOCK_ELISION Perhaps we should run the tests twice. One time with and one time without enabled lock-elision. I've also found the definition in elision-conf.h: /* Tell the test suite to test elision for this architecture. */ #define HAVE_ELISION 1 ./sysdeps/powerpc/nptl/elide.h:22:#ifdef ENABLE_LOCK_ELISION ./sysdeps/powerpc/nptl/elide.h:123:#endif /* ENABLE_LOCK_ELISION */ ./sysdeps/powerpc/powerpc32/sysdep.h:91:#if ! IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) ./sysdeps/powerpc/powerpc64/sysdep.h:275:#if !IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) ./sysdeps/powerpc/sysdep.h:24:#ifdef ENABLE_LOCK_ELISION ./sysdeps/powerpc/sysdep.h:179:#if !IS_IN(rtld) && defined (ENABLE_LOCK_ELISION) ./sysdeps/unix/sysv/linux/powerpc/force-elision.h:19:#ifdef ENABLE_LOCK_ELISION ./sysdeps/unix/sysv/linux/s390/lowlevellock.h:25:# ifdef ENABLE_LOCK_ELISION ./sysdeps/unix/sysv/linux/s390/lowlevellock.h:48:# endif /* ENABLE_LOCK_ELISION */ ./sysdeps/unix/sysv/linux/s390/force-elision.h:19:#ifdef ENABLE_LOCK_ELISION ./sysdeps/unix/sysv/linux/s390/elision-conf.h:18:#ifdef ENABLE_LOCK_ELISION ./sysdeps/s390/nptl/bits/pthreadtypes.h:92:# ifdef ENABLE_LOCK_ELISION ./sysdeps/s390/nptl/bits/pthreadtypes.h:108:# ifdef ENABLE_LOCK_ELISION I've used the attached diff on top of your patch. (I haven't adjusted the patch to fix those changes on power) Then a simple test-program run on s390x/s390 showed: ./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! By the way, do you plan to support tweaking the elision tuning values via GLIBC_TUNABLES, too? See proposed patch "Patchwork [RFC] Tunable elision patch for siddhesh/tunables" (https://patchwork.sourceware.org/patch/8926/).
(In reply to IBM Bug Proxy from comment #17) > Have you already posted this patch on glibc mailinglist? I have not yet posted it to the list, but it's my next priority. I'm testing on x86_64, ppc64le, and s390x. The patch has gotten larger and I've addressed almost all of your comments here. e.g. modified: ChangeLog modified: config.h.in modified: configure modified: configure.ac modified: elf/dl-tunables.h modified: elf/dl-tunables.list modified: nptl/Makefile modified: nptl/tst-mutex8.c modified: scripts/gen-tunables.awk modified: sysdeps/powerpc/nptl/elide.h modified: sysdeps/powerpc/powerpc32/sysdep.h modified: sysdeps/powerpc/powerpc64/sysdep.h modified: sysdeps/powerpc/sysdep.h modified: sysdeps/s390/nptl/bits/pthreadtypes.h modified: sysdeps/unix/sysv/linux/powerpc/elision-conf.c modified: sysdeps/unix/sysv/linux/powerpc/force-elision.h modified: sysdeps/unix/sysv/linux/s390/elision-conf.c modified: sysdeps/unix/sysv/linux/s390/elision-conf.h modified: sysdeps/unix/sysv/linux/s390/force-elision.h modified: sysdeps/unix/sysv/linux/s390/lowlevellock.h modified: sysdeps/unix/sysv/linux/x86/elision-conf.c Removing ENABLE_LOCK_ELISION is something I'd forgotten to do with the WIP patch. Once I get it working completely for all 3 targets I'll post upstream (probably the rest of the day, or posting on Monday). For example for tst-mutex8 I just run the test with elision disabled and remove the ENABLE_LOCK_ELISION conditions. +# Disable elision for tst-mutex8 so it can verify error case for +# destroying a mutex. +tst-mutex8-ENV = GLIBC_TUNABLES=glibc.elision.enable=0 > By the way, do you plan to support tweaking the elision tuning values via > GLIBC_TUNABLES, too? Yes I do. It's the real reason to move to tunables. My idea is to do it in three steps (two for now): (a) Make elision always available, but not always enabled e.g. GLIBC_TUANBLES=glibc.elision.enable=1 to opt-in. (b) Add all the tunable parameters that are generic (possibly refactoring across arches) e.g. glibc.elision.skip_lock_busy=3 (c) Add tunable parameters per architecture (there aren't any yet but we can talk about it). > See proposed patch "Patchwork [RFC] Tunable elision patch for > siddhesh/tunables" (https://patchwork.sourceware.org/patch/8926/). That's a perfect place to start for (b), thanks for that.
------- Comment From STLI.com 2017-05-11 02:41 EDT------- (In reply to comment #19) > (In reply to IBM Bug Proxy from comment #17) > > Have you already posted this patch on glibc mailinglist? > I have not yet posted it to the list, but it's my next priority. > > I'm testing on x86_64, ppc64le, and s390x. > > The patch has gotten larger and I've addressed almost all of your comments > here. > Carlos, are there any news regarding your patch? Is there a newer version available which I can test on s390x?
(In reply to IBM Bug Proxy from comment #19) > ------- Comment From STLI.com 2017-05-11 02:41 EDT------- > (In reply to comment #19) > > (In reply to IBM Bug Proxy from comment #17) > > > Have you already posted this patch on glibc mailinglist? > > I have not yet posted it to the list, but it's my next priority. > > > > I'm testing on x86_64, ppc64le, and s390x. > > > > The patch has gotten larger and I've addressed almost all of your comments > > here. > > > > Carlos, > are there any news regarding your patch? > Is there a newer version available which I can test on s390x? Posted my recent x86_64 version with power/s390 fixes also, but I need to finish testing it more thoroughly. https://www.sourceware.org/ml/libc-alpha/2017-05/msg00335.html
------- Comment From mgrf.com 2017-07-19 09:57 EDT------- Can we get the fix for s390x for verification please ?
(In reply to IBM Bug Proxy from comment #21) > ------- Comment From mgrf.com 2017-07-19 09:57 EDT------- > Can we get the fix for s390x for verification please ? This will have to wait a bit as I'm currently tied up with glibc 2.26 release, and Fedora 27 release. I'll keep you updated on my progress.
------- Comment From mgrf.com 2017-09-13 09:51 EDT------- Carlos, what is the status on this please? Would you mind to let us start our tests now?
------- Comment From STLI.com 2017-09-13 10:17 EDT------- As information: Rogerio A. Cardoso <rcardoso.ibm.com> is currently proposing a similar patch on libc-alpha: "[RFC][PATCH] tunables: Add elision tunable" (https://www.sourceware.org/ml/libc-alpha/2017-09/msg00443.html)
------- Comment From STLI.com 2017-12-06 04:50 EDT------- With the upstream commit "Add elision tunables" (https://sourceware.org/git/?p=glibc.git;a=commit;h=07ed18d26a342741cb25a4739158c65ed9dd4d09), a user can enable lock-elision via environment variable GLIBC_TUNABLES=glibc.elision.enable=1. By default lock-elision is not enabled. Note: the glibc configure flag --enable-lock-elision is now removed!
(In reply to IBM Bug Proxy from comment #25) > ------- Comment From STLI.com 2017-12-06 04:50 EDT------- > With the upstream commit "Add elision tunables" > (https://sourceware.org/git/?p=glibc.git;a=commit; > h=07ed18d26a342741cb25a4739158c65ed9dd4d09), a user can enable lock-elision > via environment variable GLIBC_TUNABLES=glibc.elision.enable=1. By default > lock-elision is not enabled. > Note: the glibc configure flag --enable-lock-elision is now removed! Right. I've already made the spec file adjustments in the latest rawhide import, but I forgot to add a reference to this bug. Thanks. Note that there are some unrelated GCC and infrastructure issues which will delay the arrival of new packages in rawhide.
*** Bug 1560678 has been marked as a duplicate of this bug. ***
so that means even on x86_64 you need to set GLIBC_TUNABLES=glibc.elision.enable=1 or it's not enabled? * Wed Dec 06 2017 Florian Weimer <fweimer> - 2.26.9000-31 - Add elision tunables. Drop related configure flag. (#1383986) not cool because you *really* need to take care that all sorts fo autotests have it enabled as well as systemunits set the env-var
(In reply to Harald Reindl from comment #28) > so that means even on x86_64 you need to set > GLIBC_TUNABLES=glibc.elision.enable=1 or it's not enabled? > > * Wed Dec 06 2017 Florian Weimer <fweimer> - 2.26.9000-31 > - Add elision tunables. Drop related configure flag. (#1383986) Yes, x86-64 in particular is an architecture where I wouldn't want to enable elision by default.