From patchwork Tue Nov 21 12:56:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 24399 Received: (qmail 30153 invoked by alias); 21 Nov 2017 12:56:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 30143 invoked by uid 89); 21 Nov 2017 12:56:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [RFC][PATCH] tunables: Add elision tunable To: libc-alpha@sourceware.org References: <68de6583-4f5c-5fa4-9328-1682e614ef56@linux.vnet.ibm.com> <581813ef-53f8-ba33-4026-21e8970d0155@linux.vnet.ibm.com> <82fe9665-1329-98c1-7a15-c1b953bda91d@linux.vnet.ibm.com> From: Stefan Liebler Date: Tue, 21 Nov 2017 13:56:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <82fe9665-1329-98c1-7a15-c1b953bda91d@linux.vnet.ibm.com> X-TM-AS-GCONF: 00 x-cbid: 17112112-0040-0000-0000-000003F0E6AF X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17112112-0041-0000-0000-000025F3ADDE Message-Id: <605c6017-4e5d-553a-d1ff-36b6480e7313@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-11-21_05:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=15 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711210176 On 11/16/2017 07:44 PM, Rogerio Alves wrote: > Hi Carlos, > > All documentation suggestions was made on patch v5. Thanks for the review. > > Regards > > Rogerio. > Hi Rogerio, I've applied your patch in order to test it on s390. Unfortunately building glibc is now failing. Here are some notes: git apply warned: 0001-Add-elision-tunables.patch:291: trailing whitespace. # Disable elision for tst-mutex8 so it can verify error case for warning: 1 line adds whitespace errors. Changing configure.ac is not sufficient. Please regenerate configure in order to reflect the changes in configure.ac. There are also usages of enable_lock_elision and ENABLE_LOCK_ELISION in config.make.in, sysdeps/unix/sysv/linux/s390/Makefile, sysdeps/s390/nptl/bits/pthreadtypes-arch.h, sysdeps/s390/configure.ac. Please have a look into attached diff which applies on top of your patch v5. With those changes I was able to build glibc on s390. Afterwards I've run a small test program which determines if pthread_mutex_lock is using a transaction or not: ./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=0 ./prog Lock was a normal lock! chmod +s prog_secure GLIBC_TUNABLES=glibc.elision.enable=1 ./prog_secure Lock was a normal lock! Bye. Stefan diff --git a/config.make.in b/config.make.in index bd84a57..9da77d1 100644 --- a/config.make.in +++ b/config.make.in @@ -99,7 +99,6 @@ build-nscd = @build_nscd@ 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. diff --git a/configure b/configure index d9d9243..47431bd 100755 --- a/configure +++ b/configure @@ -679,7 +679,6 @@ enable_werror all_warnings force_install bindnow -enable_lock_elision hardcoded_path_in_tests enable_timezone_tools use_default_link @@ -768,7 +767,6 @@ enable_profile enable_timezone_tools enable_hardcoded_path_in_tests enable_stackguard_randomization -enable_lock_elision enable_hidden_plt enable_bind_now enable_stack_protector @@ -1428,8 +1426,6 @@ Optional Features: --enable-stackguard-randomization initialize __stack_chk_guard canary with a random number at program start - --enable-lock-elision=yes/no - Enable lock elision for pthread mutexes by default --disable-hidden-plt do not hide internal function calls to avoid PLT --enable-bind-now disable lazy relocations in DSOs --enable-stack-protector=[yes|no|all|strong] @@ -3395,19 +3391,6 @@ if test "$enable_stackguard_randomize" = yes; then fi -# Check whether --enable-lock-elision was given. -if test "${enable_lock_elision+set}" = set; then : - enableval=$enable_lock_elision; enable_lock_elision=$enableval -else - enable_lock_elision=no -fi - - -if test "$enable_lock_elision" = yes ; then - $as_echo "#define ENABLE_LOCK_ELISION 1" >>confdefs.h - -fi - # Check whether --enable-hidden-plt was given. if test "${enable_hidden_plt+set}" = set; then : enableval=$enable_hidden_plt; hidden=$enableval diff --git a/sysdeps/s390/configure b/sysdeps/s390/configure index d4a4a3d..74b415f 100644 --- a/sysdeps/s390/configure +++ b/sysdeps/s390/configure @@ -35,7 +35,7 @@ fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_builtin_tbegin" >&5 $as_echo "$libc_cv_gcc_builtin_tbegin" >&6; } -if test "$enable_lock_elision" = yes && test "$libc_cv_gcc_builtin_tbegin" = no ; then +if test "$libc_cv_gcc_builtin_tbegin" = no ; then critic_missing="$critic_missing The used GCC has no support for __builtin_tbegin, which is needed for lock-elision on target S390." fi diff --git a/sysdeps/s390/configure.ac b/sysdeps/s390/configure.ac index 7d0b5ce..1cdb021 100644 --- a/sysdeps/s390/configure.ac +++ b/sysdeps/s390/configure.ac @@ -26,7 +26,7 @@ else fi rm -f conftest* ]) -if test "$enable_lock_elision" = yes && test "$libc_cv_gcc_builtin_tbegin" = no ; then +if test "$libc_cv_gcc_builtin_tbegin" = no ; then critic_missing="$critic_missing The used GCC has no support for __builtin_tbegin, which is needed for lock-elision on target S390." fi diff --git a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h index 1ae2773..fd15931 100644 --- a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h @@ -40,11 +40,7 @@ /* Definitions for internal mutex struct. */ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END -#ifdef ENABLE_LOCK_ELISION #define __PTHREAD_MUTEX_LOCK_ELISION 1 -#else -#define __PTHREAD_MUTEX_LOCK_ELISION 0 -#endif #define __PTHREAD_MUTEX_NUSERS_AFTER_KIND (__WORDSIZE != 64) #define __PTHREAD_MUTEX_USE_UNION (__WORDSIZE != 64) diff --git a/sysdeps/unix/sysv/linux/s390/Makefile b/sysdeps/unix/sysv/linux/s390/Makefile index c5f544d..77f3852 100644 --- a/sysdeps/unix/sysv/linux/s390/Makefile +++ b/sysdeps/unix/sysv/linux/s390/Makefile @@ -16,7 +16,6 @@ sysdep_routines += dl-vdso endif ifeq ($(subdir),nptl) -ifeq ($(enable-lock-elision),yes) libpthread-sysdep_routines += elision-lock elision-unlock elision-timed \ elision-trylock @@ -26,7 +25,6 @@ CFLAGS-elision-timed.c = $(elision-CFLAGS) CFLAGS-elision-trylock.c = $(elision-CFLAGS) CFLAGS-elision-unlock.c = $(elision-CFLAGS) endif -endif ifeq ($(subdir),misc) tests += tst-ptrace-singleblock