From 12d2dd706099aa42224149358634bce48b355823 Mon Sep 17 00:00:00 2001 From: Stefan Liebler Date: Tue, 6 Jun 2017 09:41:56 +0200 Subject: Optimize generic spinlock code and use C11 like atomic macros. This patch optimizes the generic spinlock code. The type pthread_spinlock_t is a typedef to volatile int on all archs. Passing a volatile pointer to the atomic macros which are not mapped to the C11 atomic builtins can lead to extra stores and loads to stack if such a macro creates a temporary variable by using "__typeof (*(mem)) tmp;". Thus, those macros which are used by spinlock code - atomic_exchange_acquire, atomic_load_relaxed, atomic_compare_exchange_weak - have to be adjusted. According to the comment from Szabolcs Nagy, the type of a cast expression is unqualified (see http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_423.htm): __typeof ((__typeof (*(mem)) *(mem)) tmp; Thus from spinlock perspective the variable tmp is of type int instead of type volatile int. This patch adjusts those macros in include/atomic.h. With this construct GCC >= 5 omits the extra stores and loads. The atomic macros are replaced by the C11 like atomic macros and thus the code is aligned to it. The pthread_spin_unlock implementation is now using release memory order instead of sequentially consistent memory order. The issue with passed volatile int pointers applies to the C11 like atomic macros as well as the ones used before. I've added a glibc_likely hint to the first atomic exchange in pthread_spin_lock in order to return immediately to the caller if the lock is free. Without the hint, there is an additional jump if the lock is free. I've added the atomic_spin_nop macro within the loop of plain reads. The plain reads are also realized by C11 like atomic_load_relaxed macro. The new define ATOMIC_EXCHANGE_USES_CAS determines if the first try to acquire the spinlock in pthread_spin_lock or pthread_spin_trylock is an exchange or a CAS. This is defined in atomic-machine.h for all architectures. The define SPIN_LOCK_READS_BETWEEN_CMPXCHG is now removed. There is no technical reason for throwing in a CAS every now and then, and so far we have no evidence that it can improve performance. If that would be the case, we have to adjust other spin-waiting loops elsewhere, too! Using a CAS loop without plain reads is not a good idea on many targets and wasn't used by one. Thus there is now no option to do so. Architectures are now using the generic spinlock automatically if they do not provide an own implementation. Thus the pthread_spin_lock.c files in sysdeps folder are deleted. ChangeLog: * NEWS: Mention new spinlock implementation. * include/atomic.h: (__atomic_val_bysize): Cast type to omit volatile qualifier. (atomic_exchange_acq): Likewise. (atomic_load_relaxed): Likewise. (ATOMIC_EXCHANGE_USES_CAS): Check definition. * nptl/pthread_spin_init.c (pthread_spin_init): Use atomic_store_relaxed. * nptl/pthread_spin_lock.c (pthread_spin_lock): Use C11-like atomic macros. * nptl/pthread_spin_trylock.c (pthread_spin_trylock): Likewise. * nptl/pthread_spin_unlock.c (pthread_spin_unlock): Use atomic_store_release. * sysdeps/aarch64/nptl/pthread_spin_lock.c: Delete File. * sysdeps/arm/nptl/pthread_spin_lock.c: Likewise. * sysdeps/hppa/nptl/pthread_spin_lock.c: Likewise. * sysdeps/m68k/nptl/pthread_spin_lock.c: Likewise. * sysdeps/microblaze/nptl/pthread_spin_lock.c: Likewise. * sysdeps/mips/nptl/pthread_spin_lock.c: Likewise. * sysdeps/nios2/nptl/pthread_spin_lock.c: Likewise. * sysdeps/aarch64/atomic-machine.h (ATOMIC_EXCHANGE_USES_CAS): Define. * sysdeps/alpha/atomic-machine.h: Likewise. * sysdeps/arm/atomic-machine.h: Likewise. * sysdeps/i386/atomic-machine.h: Likewise. * sysdeps/ia64/atomic-machine.h: Likewise. * sysdeps/m68k/coldfire/atomic-machine.h: Likewise. * sysdeps/m68k/m680x0/m68020/atomic-machine.h: Likewise. * sysdeps/microblaze/atomic-machine.h: Likewise. * sysdeps/mips/atomic-machine.h: Likewise. * sysdeps/powerpc/powerpc32/atomic-machine.h: Likewise. * sysdeps/powerpc/powerpc64/atomic-machine.h: Likewise. * sysdeps/s390/atomic-machine.h: Likewise. * sysdeps/sparc/sparc32/atomic-machine.h: Likewise. * sysdeps/sparc/sparc32/sparcv9/atomic-machine.h: Likewise. * sysdeps/sparc/sparc64/atomic-machine.h: Likewise. * sysdeps/tile/tilegx/atomic-machine.h: Likewise. * sysdeps/tile/tilepro/atomic-machine.h: Likewise. * sysdeps/unix/sysv/linux/hppa/atomic-machine.h: Likewise. * sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h: Likewise. * sysdeps/unix/sysv/linux/nios2/atomic-machine.h: Likewise. * sysdeps/unix/sysv/linux/sh/atomic-machine.h: Likewise. * sysdeps/x86_64/atomic-machine.h: Likewise. --- sysdeps/unix/sysv/linux/hppa/atomic-machine.h | 3 +++ sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h | 3 +++ sysdeps/unix/sysv/linux/nios2/atomic-machine.h | 3 +++ sysdeps/unix/sysv/linux/sh/atomic-machine.h | 3 +++ 4 files changed, 12 insertions(+) (limited to 'sysdeps/unix') diff --git a/sysdeps/unix/sysv/linux/hppa/atomic-machine.h b/sysdeps/unix/sysv/linux/hppa/atomic-machine.h index 2cd22352b1..59581bd4bc 100644 --- a/sysdeps/unix/sysv/linux/hppa/atomic-machine.h +++ b/sysdeps/unix/sysv/linux/hppa/atomic-machine.h @@ -39,6 +39,9 @@ typedef uintmax_t uatomic_max_t; #define __HAVE_64B_ATOMICS 0 #define USE_ATOMIC_COMPILER_BUILTINS 0 +/* XXX Is this actually correct? */ +#define ATOMIC_EXCHANGE_USES_CAS 1 + /* prev = *addr; if (prev == old) *addr = new; diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h b/sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h index 6755a056f4..1daac514a9 100644 --- a/sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/atomic-machine.h @@ -39,6 +39,9 @@ typedef uintmax_t uatomic_max_t; #define __HAVE_64B_ATOMICS 0 #define USE_ATOMIC_COMPILER_BUILTINS 0 +/* XXX Is this actually correct? */ +#define ATOMIC_EXCHANGE_USES_CAS 1 + /* The only basic operation needed is compare and exchange. */ /* For ColdFire we'll have to trap into the kernel mode anyway, so trap from the library rather then from the kernel wrapper. */ diff --git a/sysdeps/unix/sysv/linux/nios2/atomic-machine.h b/sysdeps/unix/sysv/linux/nios2/atomic-machine.h index 6111ccf886..d4dd025603 100644 --- a/sysdeps/unix/sysv/linux/nios2/atomic-machine.h +++ b/sysdeps/unix/sysv/linux/nios2/atomic-machine.h @@ -34,6 +34,9 @@ typedef uintmax_t uatomic_max_t; #define __HAVE_64B_ATOMICS 0 #define USE_ATOMIC_COMPILER_BUILTINS 0 +/* XXX Is this actually correct? */ +#define ATOMIC_EXCHANGE_USES_CAS 1 + #define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \ (abort (), (__typeof (*mem)) 0) #define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \ diff --git a/sysdeps/unix/sysv/linux/sh/atomic-machine.h b/sysdeps/unix/sysv/linux/sh/atomic-machine.h index 3c58b70c02..5a7e953ac4 100644 --- a/sysdeps/unix/sysv/linux/sh/atomic-machine.h +++ b/sysdeps/unix/sysv/linux/sh/atomic-machine.h @@ -47,6 +47,9 @@ typedef uintmax_t uatomic_max_t; #define __HAVE_64B_ATOMICS 0 #define USE_ATOMIC_COMPILER_BUILTINS 0 +/* XXX Is this actually correct? */ +#define ATOMIC_EXCHANGE_USES_CAS 1 + /* SH kernel has implemented a gUSA ("g" User Space Atomicity) support for the user space atomicity. The atomicity macros use this scheme. -- cgit 1.4.1