arm64: support for 8.1 LSE atomic instructions
Subsystems (5)
cover arm64: support for 8.1 LSE atomic instructions
Hi all,
This patch series adds support for the new atomic instructions, introduced as part of the Large System Extension in ARMv8.1, to the Linux kernel.
Whilst the new instructions can be configured out at compile time via the CONFIG_ARM64_LSE_ATOMICS option, we make heavy use of alternative patching to ensure that we can continue to boot a kernel Image anywhere, regardless of the CPU support.
I've tested this on Juno, Seattle and Hikey (none of which have the new instructions) and also on the FastModel with 8.1 enabled.
Feedback welcome,
Will
--->8
Will Deacon (18):
arm64: cpufeature.h: add missing #include of kernel.h
arm64: atomics: move ll/sc atomics into separate header file
arm64: elf: advertise 8.1 atomic instructions as new hwcap
arm64: alternatives: add cpu feature for lse atomics
arm64: introduce CONFIG_ARM64_LSE_ATOMICS as fallback to ll/sc atomics
arm64: atomics: patch in lse instructions when supported by the CPU
arm64: locks: patch in lse instructions when supported by the CPU
arm64: bitops: patch in lse instructions when supported by the CPU
arm64: xchg: patch in lse instructions when supported by the CPU
arm64: cmpxchg: patch in lse instructions when supported by the CPU
arm64: cmpxchg_dbl: patch in lse instructions when supported by the
CPU
arm64: cmpxchg: avoid "cc" clobber in ll/sc routines
arm64: cmpxchg: avoid memory barrier on comparison failure
arm64: atomics: tidy up common atomic{,64}_* macros
arm64: atomics: prefetch the destination word for write prior to stxr
arm64: atomics: implement atomic{,64}_cmpxchg using cmpxchg
arm64: atomic64_dec_if_positive: fix incorrect branch condition
arm64: kconfig: select HAVE_CMPXCHG_LOCAL
arch/arm64/Kconfig | 13 ++ arch/arm64/Makefile | 13 +- arch/arm64/include/asm/atomic.h | 262 ++++++------------------------- arch/arm64/include/asm/atomic_ll_sc.h | 237 +++++++++++++++++++++++++++++ arch/arm64/include/asm/atomic_lse.h | 279 ++++++++++++++++++++++++++++++++++ arch/arm64/include/asm/cmpxchg.h | 192 +++++++++-------------- arch/arm64/include/asm/cpufeature.h | 5 +- arch/arm64/include/asm/futex.h | 2 + arch/arm64/include/asm/lse.h | 58 +++++++ arch/arm64/include/asm/spinlock.h | 132 ++++++++++++---- arch/arm64/include/uapi/asm/hwcap.h | 1 + arch/arm64/kernel/setup.c | 18 +++ arch/arm64/lib/Makefile | 13 ++ arch/arm64/lib/atomic_ll_sc.c | 3 + arch/arm64/lib/bitops.S | 45 +++--- 15 files changed, 893 insertions(+), 380 deletions(-) create mode 100644 arch/arm64/include/asm/atomic_ll_sc.h create mode 100644 arch/arm64/include/asm/atomic_lse.h create mode 100644 arch/arm64/include/asm/lse.h create mode 100644 arch/arm64/lib/atomic_ll_sc.c
Signature · 1 line
2.1.4
PATCH 1/18 arm64: cpufeature.h: add missing #include of kernel.h
cpufeature.h makes use of DECLARE_BITMAP, which in turn relies on the BITS_TO_LONGS and DIV_ROUND_UP macros.
This patch includes kernel.h in cpufeature.h to prevent all users having to do the same thing.
--- arch/arm64/include/asm/cpufeature.h | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/include/asm/cpufeature.h +2 -0
@@ -30,6 +30,8 @@ #ifndef __ASSEMBLY__ +#include <linux/kernel.h> + struct arm64_cpu_capabilities { const char *desc; u16 capability;
Signature · 1 line
2.1.4
PATCH 2/18 arm64: atomics: move ll/sc atomics into separate header file
In reply to Will DeaconIn preparation for the Large System Extension (LSE) atomic instructions introduced by ARM v8.1, move the current exclusive load/store (LL/SC) atomics into their own header file.
--- arch/arm64/include/asm/atomic.h | 162 +-------------------------- arch/arm64/include/asm/atomic_ll_sc.h | 205 ++++++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 160 deletions(-) create mode 100644 arch/arm64/include/asm/atomic_ll_sc.h
arch/arm64/include/asm/atomic.h +2 -160
@@ -30,6 +30,8 @@#ifdef __KERNEL__ +#include <asm/atomic_ll_sc.h> +/* * On ARM, ordinary assignment (str instruction) doesn't clear the local * strex/ldrex monitor on some implementations. The reason we can use it for
@@ -38,79 +40,6 @@#define atomic_read (v) ACCESS_ONCE((v)->counter)#define atomic_set (v,i) (((v)->counter) = (i)) -/* -* AArch64 UP and SMP safe atomic ops. We use load exclusive and -* store exclusive to ensure that these are atomic. We may loop -* to ensure that the update happens. -*/ - -#define ATOMIC_OP (op, asm_op) \ -static inline void atomic_##op(int i, atomic_t *v) \ -{ \ - unsigned long tmp; \ - int result; \ - \ - asm volatile("// atomic_" #op "\n" \ -"1: ldxr %w0, %2\n" \ -" " #asm_op " %w0, %w0, %w3\n" \ -" stxr %w1, %w0, %2\n" \ -" cbnz %w1, 1b" \ - : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ - : "Ir" (i)); \ -} \ - -#define ATOMIC_OP_RETURN (op, asm_op) \ -static inline int atomic_##op##_return(int i, atomic_t *v) \ -{ \ - unsigned long tmp; \ - int result; \ - \ - asm volatile("// atomic_" #op "_return\n" \ -"1: ldxr %w0, %2\n" \ -" " #asm_op " %w0, %w0, %w3\n" \ -" stlxr %w1, %w0, %2\n" \ -" cbnz %w1, 1b" \ - : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ - : "Ir" (i) \ - : "memory"); \ - \ - smp_mb(); \ - return result; \ -} - -#define ATOMIC_OPS (op, asm_op) \ - ATOMIC_OP(op, asm_op) \ - ATOMIC_OP_RETURN(op, asm_op) - -ATOMIC_OPS(add, add) -ATOMIC_OPS(sub, sub) - -#undef ATOMIC_OPS -#undef ATOMIC_OP_RETURN -#undef ATOMIC_OP - -static inline int atomic_cmpxchg (atomic_t *ptr,int old,int new) -{ -unsigned long tmp; -int oldval; - -smp_mb (); - - asmvolatile ("// atomic_cmpxchg\n" -"1: ldxr %w1, %2\n" -" cmp %w1, %w3\n" -" b.ne 2f\n" -" stxr %w0, %w4, %2\n" -" cbnz %w0, 1b\n" -"2:" - :"=&r" (tmp),"=&r" (oldval),"+Q" (ptr->counter) - :"Ir" (old),"r" (new) - :"cc" ); - -smp_mb (); -return oldval; -} -#define atomic_xchg (v, new) (xchg(&((v)->counter), new))static inline int __atomic_add_unless (atomic_t *v,int a,int u)
@@ -142,95 +71,8 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u)#define atomic64_read (v) ACCESS_ONCE((v)->counter)#define atomic64_set (v,i) (((v)->counter) = (i)) -#define ATOMIC64_OP (op, asm_op) \ -static inline void atomic64_##op(long i, atomic64_t *v) \ -{ \ - long result; \ - unsigned long tmp; \ - \ - asm volatile("// atomic64_" #op "\n" \ -"1: ldxr %0, %2\n" \ -" " #asm_op " %0, %0, %3\n" \ -" stxr %w1, %0, %2\n" \ -" cbnz %w1, 1b" \ - : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ - : "Ir" (i)); \ -} \ - -#define ATOMIC64_OP_RETURN (op, asm_op) \ -static inline long atomic64_##op##_return(long i, atomic64_t *v) \ -{ \ - long result; \ - unsigned long tmp; \ - \ - asm volatile("// atomic64_" #op "_return\n" \ -"1: ldxr %0, %2\n" \ -" " #asm_op " %0, %0, %3\n" \ -" stlxr %w1, %0, %2\n" \ -" cbnz %w1, 1b" \ - : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ - : "Ir" (i) \ - : "memory"); \ - \ - smp_mb(); \ - return result; \ -} - -#define ATOMIC64_OPS (op, asm_op) \ - ATOMIC64_OP(op, asm_op) \ - ATOMIC64_OP_RETURN(op, asm_op) - -ATOMIC64_OPS(add, add) -ATOMIC64_OPS(sub, sub) - -#undef ATOMIC64_OPS -#undef ATOMIC64_OP_RETURN -#undef ATOMIC64_OP - -static inline long atomic64_cmpxchg (atomic64_t *ptr,long old,long new) -{ -long oldval; -unsigned long res; - -smp_mb (); - - asmvolatile ("// atomic64_cmpxchg\n" -"1: ldxr %1, %2\n" -" cmp %1, %3\n" -" b.ne 2f\n" -" stxr %w0, %4, %2\n" -" cbnz %w0, 1b\n" -"2:" - :"=&r" (res),"=&r" (oldval),"+Q" (ptr->counter) - :"Ir" (old),"r" (new) - :"cc" ); - -smp_mb (); -return oldval; -} -#define atomic64_xchg (v, new) (xchg(&((v)->counter), new)) -static inline long atomic64_dec_if_positive (atomic64_t *v) -{ -long result; -unsigned long tmp; - - asmvolatile ("// atomic64_dec_if_positive\n" -"1: ldxr %0, %2\n" -" subs %0, %0, #1\n" -" b.mi 2f\n" -" stlxr %w1, %0, %2\n" -" cbnz %w1, 1b\n" -" dmb ish\n" -"2:" - :"=&r" (result),"=&r" (tmp),"+Q" (v->counter) - : - :"cc" ,"memory" ); - -return result; -} -static inline int atomic64_add_unless (atomic64_t *v,long a,long u) {long c, old;
arch/arm64/include/asm/atomic_ll_sc.h new file +205 -0
205 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 3/18 arm64: elf: advertise 8.1 atomic instructions as new hwcap
In reply to Will DeaconThe ARM v8.1 architecture introduces new atomic instructions to the A64 instruction set for things like cmpxchg, so advertise their availability to userspace using a hwcap.
--- arch/arm64/include/uapi/asm/hwcap.h | 1 + arch/arm64/kernel/setup.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+)
arch/arm64/include/uapi/asm/hwcap.h +1 -0
@@ -27,5 +27,6 @@#define HWCAP_SHA1 (1 << 5)#define HWCAP_SHA2 (1 << 6)#define HWCAP_CRC32 (1 << 7) +#define HWCAP_ATOMICS (1 << 8)#endif /* _UAPI__ASM_HWCAP_H */
arch/arm64/kernel/setup.c +14 -0
@@ -280,6 +280,19 @@ static void __init setup_processor(void)if (block && !(block &0x8 )) elf_hwcap |=HWCAP_CRC32 ; + block = (features >>20 ) &0xf ; +if (!(block &0x8 )) { +switch (block) { +default : +case 2 : + elf_hwcap |=HWCAP_ATOMICS ; +case 1 : +/* RESERVED */ +case 0 : +break ; + } + } +#ifdef CONFIG_COMPAT /* * ID_ISAR5_EL1 carries similar information as above, but pertaining to
@@ -456,6 +469,7 @@ static const char *hwcap_str[] = {"sha1" ,"sha2" ,"crc32" , +"atomics" , NULL };
Signature · 1 line
2.1.4
Catalin Marinas
maintainerPATCH 3/18 arm64: elf: advertise 8.1 atomic instructions as new hwcap
Quoted 11 lines · Will Deacon
#endif /*
_UAPI__ASM_HWCAP_H*/arch/arm64/kernel/setup.c +2
-0@@ -280,6 +280,19 @@ static void __init setup_processor(void)if (block && !(block &0x8 )) elf_hwcap |=HWCAP_CRC32 ; + block = (features >>20 ) &0xf ; +if (!(block &0x8 )) {
arch/arm64/kernel/setup.c static void __init setup_processor(void) · line 294
Quoted 10 lines
arch/arm64/kernel/setup.c +10
-0+switch (block) { +default : +case 2 : + elf_hwcap |=HWCAP_ATOMICS ; +case 1 : +/* RESERVED */ +case 0 : +break ; + } + }
At some point, we should move the elf_hwcap setting to the cpu features infrastructure. The PAN patch series introduces an "enable" method for detected CPU features (can be cleaned up for 4.4).
Signature · 1 line
Catalin
PATCH 3/18 arm64: elf: advertise 8.1 atomic instructions as new hwcap
Quoted 10 lines · Catalin Marinas
Quoted 9 lines · Will Deacon
#endif /*
_UAPI__ASM_HWCAP_H*/arch/arm64/kernel/setup.c +0
-0@@ -280,6 +280,19 @@ static void __init setup_processor(void)if (block && !(block &0x8 )) elf_hwcap |=HWCAP_CRC32 ;
Quoted 16 lines
arch/arm64/kernel/setup.c static void __init setup_processor(void) · line 294
Quoted 12 lines
arch/arm64/kernel/setup.c +12
-0+ block = (features >>20 ) &0xf ; +if (!(block &0x8 )) { +switch (block) { +default : +case 2 : + elf_hwcap |=HWCAP_ATOMICS ; +case 1 : +/* RESERVED */ +case 0 : +break ; + } + }At some point, we should move the
elf_hwcapsetting to the cpu features infrastructure. The PAN patch series introduces an "enable" method for detected CPU features (can be cleaned up for 4.4).
On 32-bit ARM, we have this accessor:
cpuid_feature_extract_field()
which is there to properly deal with sign extending the 4-bit values, and avoids all the if (!(block & 8)) { crap.
The above could then become the much simpler:
block = cpuid_feature_extract_field(...); if (block > 0) elf_hwcap |= HWCAP_CRC32; block = cpuid_feature_extract_field(isarN, 20); if (block > 1) elf_hwcap |= HWCAP_ATOMICS;
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH 4/18 arm64: alternatives: add cpu feature for lse atomics
In reply to Will DeaconAdd a CPU feature for the LSE atomic instructions, so that they can be patched in at runtime when we detect that they are supported.
--- arch/arm64/include/asm/cpufeature.h | 3 ++- arch/arm64/kernel/setup.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
arch/arm64/include/asm/cpufeature.h +2 -1
@@ -25,8 +25,9 @@#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1#define ARM64_WORKAROUND_845719 2#define ARM64_HAS_SYSREG_GIC_CPUIF 3 +#define ARM64_CPU_FEAT_LSE_ATOMICS 4 -#define ARM64_NCAPS 4 +#define ARM64_NCAPS 5#ifndef __ASSEMBLY__
arch/arm64/kernel/setup.c +1 -0
@@ -286,6 +286,7 @@ static void __init setup_processor(void)default :case 2 : elf_hwcap |=HWCAP_ATOMICS ; +cpus_set_cap (ARM64_CPU_FEAT_LSE_ATOMICS );case 1 :/* RESERVED */ case 0 :
Signature · 1 line
2.1.4
PATCH 5/18 arm64: introduce CONFIG_ARM64_LSE_ATOMICS as fallback to ll/sc atomics
In reply to Will DeaconIn order to patch in the new atomic instructions at runtime, we need to generate wrappers around the out-of-line exclusive load/store atomics.
This patch adds a new Kconfig option, CONFIG_ARM64_LSE_ATOMICS. which causes our atomic functions to branch to the out-of-line ll/sc implementations. To avoid the register spill overhead of the PCS, the out-of-line functions are compiled with specific compiler flags to force out-of-line save/restore of any registers that are usually caller-saved.
--- arch/arm64/Kconfig | 12 +++ arch/arm64/include/asm/atomic.h | 9 ++ arch/arm64/include/asm/atomic_ll_sc.h | 19 +++- arch/arm64/include/asm/atomic_lse.h | 181 ++++++++++++++++++++++++++++++++++ arch/arm64/lib/Makefile | 13 +++ arch/arm64/lib/atomic_ll_sc.c | 3 + 6 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 arch/arm64/include/asm/atomic_lse.h create mode 100644 arch/arm64/lib/atomic_ll_sc.c
arch/arm64/Kconfig +12 -0
@@ -664,6 +664,18 @@ config SETEND_EMULATION If unsure, say Y endif +config ARM64_LSE_ATOMICS + bool "ARMv8.1 atomic instructions" + help + As part of the Large System Extensions, ARMv8.1 introduces new + atomic instructions that are designed specifically to scale in + very large systems. + + Say Y here to make use of these instructions for the in-kernel + atomic routines. This incurs a small overhead on CPUs that do + not support these instructions and requires the kernel to be + built with binutils >= 2.25. + endmenu menu "Boot options"
arch/arm64/include/asm/atomic.h +9 -0
@@ -21,6 +21,7 @@#define __ASM_ATOMIC_H#include <linux/compiler.h> +#include <linux/stringify.h> #include <linux/types.h> #include <asm/barrier.h>
@@ -30,7 +31,15 @@#ifdef __KERNEL__ +#define __ARM64_IN_ATOMIC_IMPL + +#ifdef CONFIG_ARM64_LSE_ATOMICS +#include <asm/atomic_lse.h> +#else #include <asm/atomic_ll_sc.h> +#endif + +#undef __ARM64_IN_ATOMIC_IMPL/* * On ARM, ordinary assignment (str instruction) doesn't clear the local
arch/arm64/include/asm/atomic_ll_sc.h +17 -2
@@ -21,6 +21,10 @@#ifndef __ASM_ATOMIC_LL_SC_H#define __ASM_ATOMIC_LL_SC_H +#ifndef __ARM64_IN_ATOMIC_IMPL +#error "please don't include this file directly" +#endif +/* * AArch64 UP and SMP safe atomic ops. We use load exclusive and * store exclusive to ensure that these are atomic. We may loop
@@ -41,6 +45,10 @@#define __LL_SC_PREFIX (x) x#endif +#ifndef __LL_SC_EXPORT +#define __LL_SC_EXPORT (x) +#endif +#define ATOMIC_OP (op, asm_op) \ __LL_SC_INLINE void \ __LL_SC_PREFIX(atomic_##op(int i, atomic_t *v)) \
@@ -56,6 +64,7 @@ __LL_SC_PREFIX(atomic_##op(int i, atomic_t *v)) \ : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ : "Ir" (i)); \ } \ +__LL_SC_EXPORT(atomic_##op);#define ATOMIC_OP_RETURN (op, asm_op) \ __LL_SC_INLINE int \
@@ -75,7 +84,8 @@ __LL_SC_PREFIX(atomic_##op##_return(int i, atomic_t *v)) \ \smp_mb (); \return result; \ -} +} \ +__LL_SC_EXPORT(atomic_##op##_return);#define ATOMIC_OPS (op, asm_op) \ ATOMIC_OP(op, asm_op) \
@@ -110,6 +120,7 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new))smp_mb ();return oldval; } +__LL_SC_EXPORT (atomic_cmpxchg);#define ATOMIC64_OP (op, asm_op) \ __LL_SC_INLINE void \
@@ -126,6 +137,7 @@ __LL_SC_PREFIX(atomic64_##op(long i, atomic64_t *v)) \ : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ : "Ir" (i)); \ } \ +__LL_SC_EXPORT(atomic64_##op);#define ATOMIC64_OP_RETURN (op, asm_op) \ __LL_SC_INLINE long \
@@ -145,7 +157,8 @@ __LL_SC_PREFIX(atomic64_##op##_return(long i, atomic64_t *v)) \ \smp_mb (); \return result; \ -} +} \ +__LL_SC_EXPORT(atomic64_##op##_return);#define ATOMIC64_OPS (op, asm_op) \ ATOMIC64_OP(op, asm_op) \
@@ -180,6 +193,7 @@ __LL_SC_PREFIX(atomic64_cmpxchg(atomic64_t *ptr, long old, long new))smp_mb ();return oldval; } +__LL_SC_EXPORT (atomic64_cmpxchg);__LL_SC_INLINE long __LL_SC_PREFIX (atomic64_dec_if_positive (atomic64_t *v))
@@ -201,5 +215,6 @@ __LL_SC_PREFIX(atomic64_dec_if_positive(atomic64_t *v)) return result; } +__LL_SC_EXPORT (atomic64_dec_if_positive);#endif /* __ASM_ATOMIC_LL_SC_H */
arch/arm64/include/asm/atomic_lse.h new file +181 -0
181 lines not on this page · raw message
arch/arm64/lib/Makefile +13 -0
@@ -3,3 +3,16 @@ lib-y := bitops.o clear_user.o delay.o copy_from_user.o \ clear_page.o memchr.o memcpy.o memmove.o memset.o \ memcmp.o strcmp.o strncmp.o strlen.o strnlen.o \ strchr.o strrchr.o + +# Tell the compiler to treat all general purpose registers as +# callee-saved, which allows for efficient runtime patching of the bl +# instruction in the caller with an atomic instruction when supported by +# the CPU. Result and argument registers are handled correctly, based on +# the function prototype. +lib-$(CONFIG_ARM64_LSE_ATOMICS) += atomic_ll_sc.o +CFLAGS_atomic_ll_sc.o := -fcall-used-x0 -ffixed-x1 -ffixed-x2 \ + -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 \ + -ffixed-x7 -fcall-saved-x8 -fcall-saved-x9 \ + -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12 \ + -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15 \ + -fcall-saved-x16 -fcall-saved-x17 -fcall-saved-x18
arch/arm64/lib/atomic_ll_sc.c new file +3 -0
@@ -0,0 +1,3 @@ +#include <asm/atomic.h> +#define __ARM64_IN_ATOMIC_IMPL +#include <asm/atomic_ll_sc.h>
Signature · 1 line
2.1.4
Catalin Marinas
maintainerPATCH 5/18 arm64: introduce CONFIG_ARM64_LSE_ATOMICS as fallback to ll/sc atomics
Quoted 9 lines · Will Deacon
In order to patch in the new atomic instructions at runtime, we need to generate wrappers around the out-of-line exclusive load/store atomics.
This patch adds a new
Kconfigoption,CONFIG_ARM64_LSE_ATOMICS. which causes our atomic functions to branch to the out-of-line ll/sc implementations. To avoid the register spill overhead of the PCS, the out-of-line functions are compiled with specific compiler flags to force out-of-line save/restore of any registers that are usually caller-saved.
I'm still trying to get my head around those -ffixed -fcall-used options.
Quoted 47 lines
arch/arm64/include/asm/atomic_lse.h new file +44
-0@@ -0,0 +1,181 @@ +/* +* Based on arch/arm/include/asm/atomic.h +* +* Copyright (C) 1996 Russell King. +* Copyright (C) 2002 Deep Blue Solutions Ltd. +* Copyright (C) 2012 ARM Ltd. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef __ASM_ATOMIC_LSE_H +#define __ASM_ATOMIC_LSE_H + +#ifndef __ARM64_IN_ATOMIC_IMPL +#error "please don't include this file directly" +#endif + +/* Move the ll/sc atomics out-of-line */ +#define __LL_SC_INLINE +#define __LL_SC_PREFIX (x) __ll_sc_##x +#define __LL_SC_EXPORT (x) EXPORT_SYMBOL(__LL_SC_PREFIX(x)) + +/* Macros for constructing calls to out-of-line ll/sc atomics */ +#define __LL_SC_SAVE_LR (r) "mov\t" #r ", x30\n" +#define __LL_SC_RESTORE_LR (r) "mov\tx30, " #r "\n" +#define __LL_SC_CALL (op) \ + "bl\t" __stringify(__LL_SC_PREFIX(atomic_##op)) "\n" +#define __LL_SC_CALL64 (op) \ + "bl\t" __stringify(__LL_SC_PREFIX(atomic64_##op)) "\n" + +#define ATOMIC_OP (op, asm_op) \ +static inline void atomic_##op(int i, atomic_t *v) \ +{ \ + unsigned long lr; \
arch/arm64/include/asm/atomic_lse.h line 54
Quoted 10 lines
arch/arm64/include/asm/atomic_lse.h new file +10
-0+ register int w0 asm ("w0") = i; \ + register atomic_t *x1 asm ("x1") = v; \ + \ + asm volatile( \ + __LL_SC_SAVE_LR(%0) \ + __LL_SC_CALL(op) \ + __LL_SC_RESTORE_LR(%0) \ + : "=&r" (lr), "+r" (w0), "+Q" (v->counter) \ + : "r" (x1)); \ +} \
Since that's an inline function, in most cases we wouldn't need to save/restore LR for a BL call, it may already be on the stack of the including functions. Can we just not tell gcc that LR is clobbered by this asm and it makes its own decision about saving/restoring?
As for v->counter, could we allocate it in callee-saved registers already and avoid the -ffixed etc. options.
But note that I'm still trying to understand all these tricks, so I may be wrong.
Signature · 1 line
Catalin
PATCH 5/18 arm64: introduce CONFIG_ARM64_LSE_ATOMICS as fallback to ll/sc atomics
Hi Catalin,
Quoted 11 lines · Catalin Marinas
Quoted 9 lines · Will Deacon
In order to patch in the new atomic instructions at runtime, we need to generate wrappers around the out-of-line exclusive load/store atomics.
This patch adds a new
Kconfigoption,CONFIG_ARM64_LSE_ATOMICS. which causes our atomic functions to branch to the out-of-line ll/sc implementations. To avoid the register spill overhead of the PCS, the out-of-line functions are compiled with specific compiler flags to force out-of-line save/restore of any registers that are usually caller-saved.
Quoted 2 lines
I'm still trying to get my head around those -ffixed -fcall-used options.
Yeah, they're pretty funky, but note that x86 does similar tricks for some of its patching too (see ARCH_HWEIGHT_CFLAGS).
Quoted 11 lines
Quoted 11 lines
arch/arm64/include/asm/atomic_lse.h new file +11
-0+#define ATOMIC_OP (op, asm_op) \ +static inline void atomic_##op(int i, atomic_t *v) \ +{ \ + unsigned long lr; \ + register int w0 asm ("w0") = i; \ + register atomic_t *x1 asm ("x1") = v; \ + \ + asm volatile( \ + __LL_SC_SAVE_LR(%0) \ + __LL_SC_CALL(op) \ + __LL_SC_RESTORE_LR(%0) \
Quoted 8 lines
arch/arm64/include/asm/atomic_lse.h line 54
Quoted 3 lines
arch/arm64/include/asm/atomic_lse.h new file +3
-0+ : "=&r" (lr), "+r" (w0), "+Q" (v->counter) \ + : "r" (x1)); \ +} \Since that's an inline function, in most cases we wouldn't need to save/restore LR for a BL call, it may already be on the stack of the including functions. Can we just not tell gcc that LR is clobbered by this asm and it makes its own decision about saving/restoring?
If we put lr in the clobber list, then it will get saved/restored by GCC even when we are using the LSE atomics and don't touch lr at all. Also note that later on the temporary register used to hold lr for the out-of-line case is used as part of the LSE atomic, so there's no real cost to having it.
Quoted 2 lines
As for
v->counter, could we allocate it in callee-saved registers already and avoid the -ffixed etc. options.
The issue with that is when we don't use LSE and want to in-line the ll/sc variants. Also, the weird compiler options also apply to any temporary variables that the out-of-line code uses, so we'd need knowledge of that here in order to allocate registers correctly (and then I have no idea how you'd unpack things on the other side).
My first stab at this tried to specify fcall-used on a per-function-prototype basis using target attributes, but GCC just silently ignores those :(
Quoted 2 lines
But note that I'm still trying to understand all these tricks, so I may be wrong.
Sorry for all the tricks, but it's the best I could come up with whilst still generating decent disassembly for all cases. You get used to it after a bit.
Will
PATCH 6/18 arm64: atomics: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of atomic_t and atomic64_t routines so that the call-site for the out-of-line ll/sc sequences is patched with an LSE atomic instruction when we detect that the CPU supports it.
If binutils is not recent enough to assemble the LSE instructions, then the ll/sc sequences are inlined as though CONFIG_ARM64_LSE_ATOMICS=n.
--- arch/arm64/Makefile | 13 +- arch/arm64/include/asm/atomic.h | 4 +- arch/arm64/include/asm/atomic_ll_sc.h | 12 -- arch/arm64/include/asm/atomic_lse.h | 274 ++++++++++++++++++++-------------- arch/arm64/include/asm/lse.h | 36 +++++ arch/arm64/kernel/setup.c | 3 + 6 files changed, 214 insertions(+), 128 deletions(-) create mode 100644 arch/arm64/include/asm/lse.h
arch/arm64/Makefile +12 -1
@@ -17,7 +17,18 @@ GZFLAGS :=-9 KBUILD_DEFCONFIG := defconfig -KBUILD_CFLAGS += -mgeneral-regs-only +# Check for binutils support for specific extensions +lseinstr := $(call as-instr,.arch_extension lse,-DCONFIG_AS_LSE=1) + +ifeq ($(CONFIG_ARM64_LSE_ATOMICS), y) + ifeq ($(lseinstr),) +$(warning LSE atomics not supported by binutils) + endif +endif + +KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) +KBUILD_AFLAGS += $(lseinstr) + ifeq ($(CONFIG_CPU_BIG_ENDIAN), y) KBUILD_CPPFLAGS += -mbig-endian AS += -EB
arch/arm64/include/asm/atomic.h +2 -2
@@ -21,11 +21,11 @@#define __ASM_ATOMIC_H#include <linux/compiler.h> -#include <linux/stringify.h> #include <linux/types.h> #include <asm/barrier.h> #include <asm/cmpxchg.h> +#include <asm/lse.h> #define ATOMIC_INIT (i) { (i) }
@@ -33,7 +33,7 @@#define __ARM64_IN_ATOMIC_IMPL -#ifdef CONFIG_ARM64_LSE_ATOMICS +#if defined(CONFIG_ARM64_LSE_ATOMICS ) && defined(CONFIG_AS_LSE )#include <asm/atomic_lse.h> #else #include <asm/atomic_ll_sc.h>
arch/arm64/include/asm/atomic_ll_sc.h +0 -12
@@ -37,18 +37,6 @@* (the optimize attribute silently ignores these options). */ -#ifndef __LL_SC_INLINE -#define __LL_SC_INLINE static inline -#endif - -#ifndef __LL_SC_PREFIX -#define __LL_SC_PREFIX (x) x -#endif - -#ifndef __LL_SC_EXPORT -#define __LL_SC_EXPORT (x) -#endif -#define ATOMIC_OP (op, asm_op) \ __LL_SC_INLINE void \ __LL_SC_PREFIX(atomic_##op(int i, atomic_t *v)) \
arch/arm64/include/asm/atomic_lse.h +161 -113
317 lines not on this page · raw message
arch/arm64/include/asm/lse.h new file +36 -0
@@ -0,0 +1,36 @@ +#ifndef __ASM_LSE_H +#define __ASM_LSE_H + +#if defined(CONFIG_AS_LSE ) && defined(CONFIG_ARM64_LSE_ATOMICS ) + +#include <linux/stringify.h> + +#include <asm/alternative.h> +#include <asm/cpufeature.h> + +__asm__(".arch_extension lse" ); + +/* Move the ll/sc atomics out-of-line */ +#define __LL_SC_INLINE +#define __LL_SC_PREFIX (x) __ll_sc_##x +#define __LL_SC_EXPORT (x) EXPORT_SYMBOL(__LL_SC_PREFIX(x)) + +/* Macros for constructing calls to out-of-line ll/sc atomics */ +#define __LL_SC_SAVE_LR (r) "mov\t" #r ", x30\n" +#define __LL_SC_RESTORE_LR (r) "mov\tx30, " #r "\n" +#define __LL_SC_CALL (op) "bl\t" __stringify(__LL_SC_PREFIX(op)) "\n" + +/* In-line patching at runtime */ +#define ARM64_LSE_ATOMIC_INSN (llsc, lse) \ + ALTERNATIVE(llsc, lse, ARM64_CPU_FEAT_LSE_ATOMICS) + +#else + +#define __LL_SC_INLINE static inline +#define __LL_SC_PREFIX (x) x +#define __LL_SC_EXPORT (x) + +#define ARM64_LSE_ATOMIC_INSN (llsc, lse) llsc + +#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */ +#endif /* __ASM_LSE_H */
arch/arm64/kernel/setup.c +3 -0
@@ -287,6 +287,9 @@ static void __init setup_processor(void)case 2 : elf_hwcap |=HWCAP_ATOMICS ;cpus_set_cap (ARM64_CPU_FEAT_LSE_ATOMICS ); +if (IS_ENABLED (CONFIG_AS_LSE ) && +IS_ENABLED (CONFIG_ARM64_LSE_ATOMICS )) +pr_info ("LSE atomics supported\n" );case 1 :/* RESERVED */ case 0 :
Signature · 1 line
2.1.4
PATCH 7/18 arm64: locks: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of our locking functions so that LSE atomic instructions are used for spinlocks and rwlocks.
--- arch/arm64/include/asm/spinlock.h | 132 +++++++++++++++++++++++++++++--------- 1 file changed, 103 insertions(+), 29 deletions(-)
arch/arm64/include/asm/spinlock.h +103 -29
@@ -16,6 +16,7 @@#ifndef __ASM_SPINLOCK_H#define __ASM_SPINLOCK_H +#include <asm/lse.h> #include <asm/spinlock_types.h> #include <asm/processor.h>
@@ -38,11 +39,21 @@ static inline void arch_spin_lock(arch_spinlock_t *lock) asmvolatile (/* Atomically increment the next ticket. */ + ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ " prfm pstl1strm, %3\n" "1: ldaxr %w0, %3\n" " add %w1, %w0, %w5\n" " stxr %w2, %w1, %3\n" -" cbnz %w2, 1b\n" +" cbnz %w2, 1b\n", +/* LSE atomics */ +" mov %w2, %w5\n" +" ldadda %w2, %w0, %3\n" +" nop\n" +" nop\n" +" nop\n" + ) + /* Did we get the lock? */" eor %w1, %w0, %w0, ror #16\n" " cbz %w1, 3f\n"
@@ -67,15 +78,25 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)unsigned int tmp;arch_spinlock_t lockval; - asmvolatile ( -" prfm pstl1strm, %2\n" -"1: ldaxr %w0, %2\n" -" eor %w1, %w0, %w0, ror #16\n" -" cbnz %w1, 2f\n" -" add %w0, %w0, %3\n" -" stxr %w1, %w0, %2\n" -" cbnz %w1, 1b\n" -"2:" + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ + " prfm pstl1strm, %2\n" + "1: ldaxr %w0, %2\n" + " eor %w1, %w0, %w0, ror #16\n" + " cbnz %w1, 2f\n" + " add %w0, %w0, %3\n" + " stxr %w1, %w0, %2\n" + " cbnz %w1, 1b\n" + "2:", +/* LSE atomics */ +" ldar %w0, %2\n" +" eor %w1, %w0, %w0, ror #16\n" +" cbnz %w1, 1f\n" +" add %w1, %w0, %3\n" +" casa %w0, %w1, %2\n" +" and %w1, %w1, #0xffff\n" +" eor %w1, %w1, %w0, lsr #16\n" +"1:" ) : "=&r" (lockval), "=&r" (tmp), "+Q" (*lock) : "I" (1 << TICKET_SHIFT) : "memory");
@@ -85,10 +106,19 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)static inline void arch_spin_unlock (arch_spinlock_t *lock) { - asmvolatile ( -" stlrh %w1, %0\n" - :"=Q" (lock->owner) - :"r" (lock->owner +1 ) +unsigned long tmp; + + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ + " ldr %w1, %0\n" + " add %w1, %w1, #1\n" + " stlrh %w1, %0", + /* LSE atomics */ + " mov %w1, #1\n" + " nop\n" + " staddlh %w1, %0") + : "=Q" (lock->owner), "=&r" (tmp) + : : "memory"); }
@@ -125,11 +155,19 @@ static inline void arch_write_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" -" cbnz %w0, 2b\n" +" cbnz %w0, 2b" , +/* LSE atomics */ +"1: wfe\n" +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" +" nop\n" +" cbnz %w0, 1b" ) : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");
@@ -139,11 +177,16 @@ static inline int arch_write_trylock(arch_rwlock_t *rw) {unsigned int tmp; - asmvolatile ( + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ " ldaxr %w0, %1\n" " cbnz %w0, 1f\n" " stxr %w0, %w2, %1\n" -"1:\n" + "1:", + /* LSE atomics */ + " mov %w0, wzr\n" + " casa %w0, %w2, %1\n" + " nop") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");
@@ -153,9 +196,10 @@ static inline int arch_write_trylock(arch_rwlock_t *rw)static inline void arch_write_unlock (arch_rwlock_t *rw) { - asmvolatile ( -" stlr %w1, %0\n" - :"=Q" (rw->lock) :"r" (0 ) :"memory" ); + asm volatile(ARM64_LSE_ATOMIC_INSN( + " stlr wzr, %0", + " swpl wzr, wzr, %0") + : "=Q" (rw->lock) :: "memory"); }/* write_can_lock - would write_trylock() succeed? */
@@ -172,6 +216,10 @@ static inline void arch_write_unlock(arch_rwlock_t *rw)* * The memory barriers are implicit with the load-acquire and store-release * instructions. +* +* Note that in UNDEFINED cases, such as unlocking a lock twice, the LL/SC +* and LSE implementations may exhibit different behaviour (although this +* will have no effect on lockdep). */ static inline void arch_read_lock (arch_rwlock_t *rw) {
@@ -179,26 +227,43 @@ static inline void arch_read_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %2\n" " add %w0, %w0, #1\n" " tbnz %w0, #31, 1b\n" " stxr %w1, %w0, %2\n" -" cbnz %w1, 2b\n" +" nop\n" +" cbnz %w1, 2b" , +/* LSE atomics */ +"1: wfe\n" +"2: ldr %w0, %2\n" +" adds %w1, %w0, #1\n" +" tbnz %w1, #31, 1b\n" +" casa %w0, %w1, %2\n" +" sbc %w0, %w1, %w0\n" +" cbnz %w0, 2b" ) : "=&r" (tmp), "=&r" (tmp2), "+Q" (rw->lock) : - :"memory" ); + : "cc", "memory"); } static inline void arch_read_unlock(arch_rwlock_t *rw) {unsigned int tmp, tmp2; - asmvolatile ( + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ "1: ldxr %w0, %2\n" " sub %w0, %w0, #1\n" " stlxr %w1, %w0, %2\n" -" cbnz %w1, 1b\n" + " cbnz %w1, 1b", + /* LSE atomics */ + " movn %w0, #0\n" + " nop\n" + " nop\n" + " staddl %w0, %2") : "=&r" (tmp), "=&r" (tmp2), "+Q" (rw->lock) : : "memory");
@@ -206,17 +271,26 @@ static inline void arch_read_unlock(arch_rwlock_t *rw)static inline int arch_read_trylock (arch_rwlock_t *rw) { -unsigned int tmp, tmp2 =1 ; +unsigned int tmp, tmp2; - asmvolatile ( + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ + " mov %w1, #1\n" " ldaxr %w0, %2\n" " add %w0, %w0, #1\n" " tbnz %w0, #31, 1f\n" " stxr %w1, %w0, %2\n" -"1:\n" - :"=&r" (tmp),"+r" (tmp2),"+Q" (rw->lock) + "1:", + /* LSE atomics */ + " ldr %w0, %2\n" + " adds %w1, %w0, #1\n" + " tbnz %w1, #31, 1f\n" + " casa %w0, %w1, %2\n" + " sbc %w1, %w1, %w0\n" + "1:") + : "=&r" (tmp), "=&r" (tmp2), "+Q" (rw->lock) : - :"memory" ); + : "cc", "memory");return !tmp2; }
Signature · 1 line
2.1.4
Catalin Marinas
maintainerPATCH 7/18 arm64: locks: patch in lse instructions when supported by the CPU
Quoted 15 lines · Will Deacon
arch/arm64/include/asm/spinlock.h +2
-9@@ -67,15 +78,25 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)unsigned int tmp;arch_spinlock_t lockval; - asmvolatile ( -" prfm pstl1strm, %2\n" -"1: ldaxr %w0, %2\n" -" eor %w1, %w0, %w0, ror #16\n" -" cbnz %w1, 2f\n" -" add %w0, %w0, %3\n" -" stxr %w1, %w0, %2\n" -" cbnz %w1, 1b\n" -"2:" + asm volatile(ARM64_LSE_ATOMIC_INSN( +/* LL/SC */
arch/arm64/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 92
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +10
-0+" prfm pstl1strm, %2\n" +"1: ldaxr %w0, %2\n" +" eor %w1, %w0, %w0, ror #16\n" +" cbnz %w1, 2f\n" +" add %w0, %w0, %3\n" +" stxr %w1, %w0, %2\n" +" cbnz %w1, 1b\n" +"2:" , +/* LSE atomics */ +" ldar %w0, %2\n"
Do we still need an acquire if we fail to take the lock?
arch/arm64/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 102
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +7
-0+" eor %w1, %w0, %w0, ror #16\n" +" cbnz %w1, 1f\n" +" add %w1, %w0, %3\n" +" casa %w0, %w1, %2\n" +" and %w1, %w1, #0xffff\n" +" eor %w1, %w1, %w0, lsr #16\n" + "1:") : "=&r" (lockval), "=&r" (tmp), "+Q" (*lock) : "I" (1 << TICKET_SHIFT) : "memory");
I wonder if this is any faster with LSE. CAS would have to re-load the lock but we already have the value loaded (though most likely the reload will be from cache and we save a cbnz that can be mispredicted). I guess we'll re-test when we get some real hardware.
Does prfm help in any way with the LDAR?
Quoted 35 lines
arch/arm64/include/asm/spinlock.h +15
-5@@ -85,10 +106,19 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)static inline void arch_spin_unlock (arch_spinlock_t *lock) { - asmvolatile ( -" stlrh %w1, %0\n" - :"=Q" (lock->owner) - :"r" (lock->owner +1 ) +unsigned long tmp; + + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ + " ldr %w1, %0\n" + " add %w1, %w1, #1\n" + " stlrh %w1, %0", + /* LSE atomics */ + " mov %w1, #1\n" + " nop\n" + " staddlh %w1, %0") + : "=Q" (lock->owner), "=&r" (tmp) + : : "memory"); }@@ -125,11 +155,19 @@ static inline void arch_write_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" - " cbnz %w0, 2b\n"
arch/arm64/include/asm/spinlock.h static inline void arch_write_lock(arch_rwlock_t *rw) · line 173
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +7
-0+ " cbnz %w0, 2b", +/* LSE atomics */ +"1: wfe\n" +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" +" nop\n" + " cbnz %w0, 1b") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");
With WFE in the LL/SC case, we rely on LDAXR to set the exclusive monitor and an event would be generated every time it gets cleared. With CAS, we no longer have this behaviour, so what guarantees a SEV?
arch/arm64/include/asm/spinlock.h static inline int arch_write_trylock(arch_rwlock_t *rw) · line 184
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +2
-1@@ -139,11 +177,16 @@ static inline int arch_write_trylock(arch_rwlock_t *rw) {unsigned int tmp; - asmvolatile ( + asm volatile(ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ " ldaxr %w0, %1\n" " cbnz %w0, 1f\n" " stxr %w0, %w2, %1\n"
Not a comment to your patch but to the original code: don't we need a branch back to ldaxr if stxr fails, like we do for arch_spin_trylock? The same comment for arch_read_trylock.
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +5
-1-"1:\n" + "1:", +/* LSE atomics */ +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" + " nop") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");
arch/arm64/include/asm/spinlock.h static inline int arch_write_trylock(arch_rwlock_t *rw) · line 202
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +4
-3@@ -153,9 +196,10 @@ static inline int arch_write_trylock(arch_rwlock_t *rw)static inline void arch_write_unlock (arch_rwlock_t *rw) { - asmvolatile ( -" stlr %w1, %0\n" - :"=Q" (rw->lock) :"r" (0 ) :"memory" ); + asm volatile(ARM64_LSE_ATOMIC_INSN( + " stlr wzr, %0", + " swpl wzr, wzr, %0") + : "=Q" (rw->lock) :: "memory");
Is this any better than just STLR? We don't need the memory read.
Quoted 27 lines
arch/arm64/include/asm/spinlock.h +10
-1@@ -172,6 +216,10 @@ static inline void arch_write_unlock(arch_rwlock_t *rw)* * The memory barriers are implicit with the load-acquire and store-release * instructions. +* +* Note that in UNDEFINED cases, such as unlocking a lock twice, the LL/SC +* and LSE implementations may exhibit different behaviour (although this +* will have no effect on lockdep). */ static inline void arch_read_lock (arch_rwlock_t *rw) {@@ -179,26 +227,43 @@ static inline void arch_read_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %2\n" " add %w0, %w0, #1\n" " tbnz %w0, #31, 1b\n" " stxr %w1, %w0, %2\n" - " cbnz %w1, 2b\n" +" nop\n" +" cbnz %w1, 2b" , +/* LSE atomics */ + "1: wfe\n"
arch/arm64/include/asm/spinlock.h static inline void arch_read_lock(arch_rwlock_t *rw) · line 249
Quoted 10 lines
arch/arm64/include/asm/spinlock.h +7
-1+"2: ldr %w0, %2\n" +" adds %w1, %w0, #1\n" +" tbnz %w1, #31, 1b\n" +" casa %w0, %w1, %2\n" +" sbc %w0, %w1, %w0\n" + " cbnz %w0, 2b") : "=&r" (tmp), "=&r" (tmp2), "+Q" (rw->lock) : - : "memory"); + : "cc", "memory");
Same comment here about WFE.
Signature · 1 line
Catalin
PATCH 7/18 arm64: locks: patch in lse instructions when supported by the CPU
Hi Catalin,
Quoted 20 lines · Catalin Marinas
Quoted 19 lines · Will Deacon
arch/arm64/include/asm/spinlock.h +6
-9@@ -67,15 +78,25 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)unsigned int tmp;arch_spinlock_t lockval; - asmvolatile ( -" prfm pstl1strm, %2\n" -"1: ldaxr %w0, %2\n" -" eor %w1, %w0, %w0, ror #16\n" -" cbnz %w1, 2f\n" -" add %w0, %w0, %3\n" -" stxr %w1, %w0, %2\n" -" cbnz %w1, 1b\n" -"2:" + asm volatile(ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ +" prfm pstl1strm, %2\n" +"1: ldaxr %w0, %2\n" +" eor %w1, %w0, %w0, ror #16\n" + " cbnz %w1, 2f\n"
Quoted 8 lines
arch/arm64/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 92
Quoted 6 lines
arch/arm64/include/asm/spinlock.h +6
-0+" add %w0, %w0, %3\n" +" stxr %w1, %w0, %2\n" +" cbnz %w1, 1b\n" +"2:" , +/* LSE atomics */ +" ldar %w0, %2\n" Do we still need an acquire if we fail to take the lock?
Good point, I think we can drop that. Read-after-read ordering is sufficient to order LDR -> CASA in a trylock loop.
Quoted 16 lines
arch/arm64/include/asm/spinlock.h static inline int arch_spin_trylock(arch_spinlock_t *lock) · line 102
Quoted 11 lines
arch/arm64/include/asm/spinlock.h +7
-0+" eor %w1, %w0, %w0, ror #16\n" +" cbnz %w1, 1f\n" +" add %w1, %w0, %3\n" +" casa %w0, %w1, %2\n" +" and %w1, %w1, #0xffff\n" +" eor %w1, %w1, %w0, lsr #16\n" + "1:") : "=&r" (lockval), "=&r" (tmp), "+Q" (*lock) : "I" (1 << TICKET_SHIFT) : "memory");I wonder if this is any faster with LSE. CAS would have to re-load the lock but we already have the value loaded (though most likely the reload will be from cache and we save a cbnz that can be mispredicted). I guess we'll re-test when we get some real hardware.
Yeah, I'll definitely want to evaluate this on real hardware when it shows up. For now, I'm basically trying to avoid explicit dirtying at L1, so the CAS at least gives hardware the chance to go far in the face of contention.
Quoted 1 line
Does prfm help in any way with the LDAR?
We'd need to prfm for write, which would end up forcing everything near (which I'm avoiding for now).
Quoted 41 lines
Quoted 41 lines
arch/arm64/include/asm/spinlock.h +21
-5@@ -85,10 +106,19 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)static inline void arch_spin_unlock (arch_spinlock_t *lock) { - asmvolatile ( -" stlrh %w1, %0\n" - :"=Q" (lock->owner) - :"r" (lock->owner +1 ) +unsigned long tmp; + + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ + " ldr %w1, %0\n" + " add %w1, %w1, #1\n" + " stlrh %w1, %0", + /* LSE atomics */ + " mov %w1, #1\n" + " nop\n" + " staddlh %w1, %0") + : "=Q" (lock->owner), "=&r" (tmp) + : : "memory"); }@@ -125,11 +155,19 @@ static inline void arch_write_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" - " cbnz %w0, 2b\n" +" cbnz %w0, 2b" , +/* LSE atomics */ +"1: wfe\n" +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" + " nop\n"
Quoted 8 lines
arch/arm64/include/asm/spinlock.h static inline void arch_write_lock(arch_rwlock_t *rw) · line 173
Quoted 4 lines
arch/arm64/include/asm/spinlock.h +1
-0+ " cbnz %w0, 1b") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");With WFE in the LL/SC case, we rely on LDAXR to set the exclusive monitor and an event would be generated every time it gets cleared. With CAS, we no longer have this behaviour, so what guarantees a SEV?
My understanding was that failed CAS will set the exclusive monitor, but what I have for a spec doesn't actually comment on this behaviour. I'll go digging...
Quoted 4 lines
Quoted 4 lines
arch/arm64/include/asm/spinlock.h +0
-0@@ -139,11 +177,16 @@ static inline int arch_write_trylock(arch_rwlock_t *rw) {unsigned int tmp;
Quoted 10 lines
arch/arm64/include/asm/spinlock.h static inline int arch_write_trylock(arch_rwlock_t *rw) · line 184
Quoted 6 lines
arch/arm64/include/asm/spinlock.h +2
-1- asmvolatile ( + asm volatile(ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ " ldaxr %w0, %1\n" " cbnz %w0, 1f\n" " stxr %w0, %w2, %1\n"Not a comment to your patch but to the original code: don't we need a branch back to ldaxr if stxr fails, like we do for
arch_spin_trylock? The same comment forarch_read_trylock.
I don't think Linux specifies the behaviour here, to be honest. C11 distinguishes between "weak" and "strong" cmpxchg to try and address this sort of thing. Since we're not sure, I suppose we should loop (like we do for arch/arm/).
Bear in mind that I'm currently moving us over to the qrwlock once I've got this out of the way and added generic support for relaxed atomics, which will result in the deletion of all of this anyway.
Quoted 12 lines
Quoted 12 lines
arch/arm64/include/asm/spinlock.h +5
-1-"1:\n" + "1:", +/* LSE atomics */ +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" + " nop") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");@@ -153,9 +196,10 @@ static inline int arch_write_trylock(arch_rwlock_t *rw)
Quoted 11 lines
arch/arm64/include/asm/spinlock.h static inline int arch_write_trylock(arch_rwlock_t *rw) · line 202
Quoted 9 lines
arch/arm64/include/asm/spinlock.h +4
-3static inline void arch_write_unlock (arch_rwlock_t *rw) { - asmvolatile ( -" stlr %w1, %0\n" - :"=Q" (rw->lock) :"r" (0 ) :"memory" ); + asm volatile(ARM64_LSE_ATOMIC_INSN( + " stlr wzr, %0", + " swpl wzr, wzr, %0") + : "=Q" (rw->lock) :: "memory");Is this any better than just STLR? We don't need the memory read.
Again, I'm trying to avoid explicit dirtying at L1.
Will
PATCH 7/18 arm64: locks: patch in lse instructions when supported by the CPU
Quoted 28 lines · Will Deacon
Quoted 18 lines · Catalin Marinas
Quoted 17 lines · Will Deacon
arch/arm64/include/asm/spinlock.h +8
-1@@ -125,11 +155,19 @@ static inline void arch_write_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" - " cbnz %w0, 2b\n" +" cbnz %w0, 2b" , +/* LSE atomics */ +"1: wfe\n" +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" + " nop\n"Quoted 8 lines
arch/arm64/include/asm/spinlock.h static inline void arch_write_lock(arch_rwlock_t *rw) · line 173
Quoted 4 lines
arch/arm64/include/asm/spinlock.h +1
-0+ " cbnz %w0, 1b") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");With WFE in the LL/SC case, we rely on LDAXR to set the exclusive monitor and an event would be generated every time it gets cleared. With CAS, we no longer have this behaviour, so what guarantees a SEV?
Quoted 3 lines
My understanding was that failed CAS will set the exclusive monitor, but what I have for a spec doesn't actually comment on this behaviour. I'll go digging...
... and the winner is: not me! We do need an LDXR to set the exclusive monitor and doing that without introducing races is slightly confusing.
Here's what I now have for write_lock (read_lock is actually pretty simple):
static inline void arch_write_lock(arch_rwlock_t *rw)
{
unsigned int tmp;
asm volatile(ARM64_LSE_ATOMIC_INSN(
/* LL/SC */
" sevl\n"
"1: wfe\n"
"2: ldaxr %w0, %1\n"
" cbnz %w0, 1b\n"
" stxr %w0, %w2, %1\n"
" cbnz %w0, 2b\n"
" nop",
/* LSE atomics */
"1: mov %w0, wzr\n"
"2: casa %w0, %w2, %1\n"
" cbz %w0, 3f\n"
" ldxr %w0, %1\n"
" cbz %w0, 2b\n"
" wfe\n"
" b 1b\n"
"3:")
: "=&r" (tmp), "+Q" (rw->lock)
: "r" (0x80000000)
: "memory");
}What do you reckon?
Will
Catalin Marinas
maintainerPATCH 7/18 arm64: locks: patch in lse instructions when supported by the CPU
Quoted 65 lines · Will Deacon
Quoted 28 lines · Will Deacon
Quoted 18 lines · Catalin Marinas
Quoted 17 lines · Will Deacon
arch/arm64/include/asm/spinlock.h +8
-1@@ -125,11 +155,19 @@ static inline void arch_write_lock(arch_rwlock_t *rw) asmvolatile (" sevl\n" + ARM64_LSE_ATOMIC_INSN( +/* LL/SC */ "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" - " cbnz %w0, 2b\n" +" cbnz %w0, 2b" , +/* LSE atomics */ +"1: wfe\n" +" mov %w0, wzr\n" +" casa %w0, %w2, %1\n" + " nop\n"Quoted 8 lines
arch/arm64/include/asm/spinlock.h static inline void arch_write_lock(arch_rwlock_t *rw) · line 173
Quoted 4 lines
arch/arm64/include/asm/spinlock.h +1
-0+ " cbnz %w0, 1b") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory");With WFE in the LL/SC case, we rely on LDAXR to set the exclusive monitor and an event would be generated every time it gets cleared. With CAS, we no longer have this behaviour, so what guarantees a SEV?
Quoted 3 lines
My understanding was that failed CAS will set the exclusive monitor, but what I have for a spec doesn't actually comment on this behaviour. I'll go digging...
... and the winner is: not me! We do need an LDXR to set the exclusive monitor and doing that without introducing races is slightly confusing.
Here's what I now have for
write_lock(read_lockis actually pretty simple):static inline void
arch_write_lock(arch_rwlock_t *rw){ unsigned int tmp; asm volatile(ARM64_LSE_ATOMIC_INSN( /* LL/SC */ " sevl\n" "1: wfe\n" "2: ldaxr %w0, %1\n" " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" " cbnz %w0, 2b\n" " nop", /* LSE atomics */ "1: mov %w0, wzr\n" "2: casa %w0, %w2, %1\n" " cbz %w0, 3f\n" " ldxr %w0, %1\n" " cbz %w0, 2b\n" " wfe\n" " b 1b\n" "3:") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory"); }
Quoted 1 line
What do you reckon?
It looks fine. I thought I could reduce the number of branches but I still end up with 3. At least the no-contention case should be fast.
Signature · 1 line
Catalin
PATCH 8/18 arm64: bitops: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of our bitops functions so that LSE atomic instructions are used instead.
--- arch/arm64/include/asm/lse.h | 26 ++++++++++++++++++++++++-- arch/arm64/lib/bitops.S | 43 ++++++++++++++++++++++++------------------- 2 files changed, 48 insertions(+), 21 deletions(-)
arch/arm64/include/asm/lse.h +24 -2
@@ -4,9 +4,21 @@#if defined(CONFIG_AS_LSE ) && defined(CONFIG_ARM64_LSE_ATOMICS )#include <linux/stringify.h> +#include <asm/cpufeature.h> + +#ifdef __ASSEMBLER__ + +#include <asm/alternative-asm.h> + +.arch_extension lse + +.macro alt_lse, llsc, lse + alternative_insn "\llsc", "\lse", ARM64_CPU_FEAT_LSE_ATOMICS +.endm + +#else /* __ASSEMBLER__ */ #include <asm/alternative.h> -#include <asm/cpufeature.h> __asm__(".arch_extension lse" );
@@ -24,7 +36,16 @@ __asm__(".arch_extension lse");#define ARM64_LSE_ATOMIC_INSN (llsc, lse) \ ALTERNATIVE(llsc, lse, ARM64_CPU_FEAT_LSE_ATOMICS) -#else +#endif /* __ASSEMBLER__ */ +#else /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */ + +#ifdef __ASSEMBLER__ + +.macro alt_lse, llsc, lse + \llsc +.endm + +#else /* __ASSEMBLER__ */ #define __LL_SC_INLINE static inline#define __LL_SC_PREFIX (x) x
@@ -32,5 +53,6 @@ __asm__(".arch_extension lse");#define ARM64_LSE_ATOMIC_INSN (llsc, lse) llsc +#endif /* __ASSEMBLER__ */ #endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */ #endif /* __ASM_LSE_H */
arch/arm64/lib/bitops.S +24 -19
76 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 9/18 arm64: xchg: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of our xchg primitives so that the LSE swp instruction (yes, you read right!) is used instead.
--- arch/arm64/include/asm/cmpxchg.h | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-)
arch/arm64/include/asm/cmpxchg.h +33 -5
@@ -22,6 +22,7 @@#include <linux/mmdebug.h> #include <asm/barrier.h> +#include <asm/lse.h> static inline unsigned long __xchg (unsigned long x,volatile void *ptr,int size) {
@@ -29,37 +30,65 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizeswitch (size) { case 1: - asmvolatile ("// __xchg1\n" + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ "1: ldxrb %w0, %2\n" " stlxrb %w1, %w3, %2\n" " cbnz %w1, 1b\n" + " dmb ish", + /* LSE atomics */ + " nop\n" + " swpalb %w3, %w0, %2\n" + " nop\n" + " nop") : "=&r" (ret), "=&r" (tmp), "+Q" (*(u8 *)ptr) : "r" (x) : "memory");break ; case 2: - asmvolatile ("// __xchg2\n" + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ "1: ldxrh %w0, %2\n" " stlxrh %w1, %w3, %2\n" " cbnz %w1, 1b\n" + " dmb ish", + /* LSE atomics */ + " nop\n" + " swpalh %w3, %w0, %2\n" + " nop\n" + " nop") : "=&r" (ret), "=&r" (tmp), "+Q" (*(u16 *)ptr) : "r" (x) : "memory");break ; case 4: - asmvolatile ("// __xchg4\n" + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ "1: ldxr %w0, %2\n" " stlxr %w1, %w3, %2\n" " cbnz %w1, 1b\n" + " dmb ish", + /* LSE atomics */ + " nop\n" + " swpal %w3, %w0, %2\n" + " nop\n" + " nop") : "=&r" (ret), "=&r" (tmp), "+Q" (*(u32 *)ptr) : "r" (x) : "memory");break ; case 8: - asmvolatile ("// __xchg8\n" + asm volatile(ARM64_LSE_ATOMIC_INSN( + /* LL/SC */ "1: ldxr %0, %2\n" " stlxr %w1, %3, %2\n" " cbnz %w1, 1b\n" + " dmb ish", + /* LSE atomics */ + " nop\n" + " swpal %3, %0, %2\n" + " nop\n" + " nop") : "=&r" (ret), "=&r" (tmp), "+Q" (*(u64 *)ptr) : "r" (x) : "memory");
@@ -68,7 +97,6 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizeBUILD_BUG (); } -smp_mb ();return ret; }
Signature · 1 line
2.1.4
PATCH 10/18 arm64: cmpxchg: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of our cmpxchg primitives so that the LSE cas instruction is used instead.
--- arch/arm64/include/asm/atomic.h | 3 +- arch/arm64/include/asm/atomic_ll_sc.h | 38 ++++++++++++++++ arch/arm64/include/asm/atomic_lse.h | 40 +++++++++++++++++ arch/arm64/include/asm/cmpxchg.h | 84 ++++++++--------------------------- 4 files changed, 99 insertions(+), 66 deletions(-)
arch/arm64/include/asm/atomic.h +2 -1
@@ -24,7 +24,6 @@#include <linux/types.h> #include <asm/barrier.h> -#include <asm/cmpxchg.h> #include <asm/lse.h> #define ATOMIC_INIT (i) { (i) }
@@ -41,6 +40,8 @@#undef __ARM64_IN_ATOMIC_IMPL +#include <asm/cmpxchg.h> +/* * On ARM, ordinary assignment (str instruction) doesn't clear the local * strex/ldrex monitor on some implementations. The reason we can use it for
arch/arm64/include/asm/atomic_ll_sc.h +38 -0
@@ -205,4 +205,42 @@ __LL_SC_PREFIX(atomic64_dec_if_positive(atomic64_t *v)) }__LL_SC_EXPORT (atomic64_dec_if_positive); +#define __CMPXCHG_CASE (w, sz, name, mb, cl) \ +__LL_SC_INLINE unsigned long \ +__LL_SC_PREFIX(__cmpxchg_case_##name(volatile void *ptr, \ + unsigned long old, \ + unsigned long new)) \ +{ \ + unsigned long tmp, oldval; \ + \ + asm volatile( \ + " " #mb "\n" \ + "1: ldxr" #sz "\t%" #w "[oldval], %[v]\n" \ + " eor %" #w "[tmp], %" #w "[oldval], %" #w "[old]\n" \ + " cbnz %" #w "[tmp], 2f\n" \ + " stxr" #sz "\t%w[tmp], %" #w "[new], %[v]\n" \ + " cbnz %w[tmp], 1b\n" \ + " " #mb "\n" \ + " mov %" #w "[oldval], %" #w "[old]\n" \ + "2:" \ + : [tmp] "=&r" (tmp), [oldval] "=&r" (oldval), \ + [v] "+Q" (*(unsigned long *)ptr) \ + : [old] "Lr" (old), [new] "r" (new) \ + : cl); \ + \ + return oldval; \ +} \ +__LL_SC_EXPORT(__cmpxchg_case_##name); + +__CMPXCHG_CASE(w, b, 1, , ) +__CMPXCHG_CASE(w, h, 2, , ) +__CMPXCHG_CASE(w, , 4, , ) +__CMPXCHG_CASE( , , 8, , ) +__CMPXCHG_CASE(w, b, mb_1, dmb ish, "memory") +__CMPXCHG_CASE(w, h, mb_2, dmb ish, "memory") +__CMPXCHG_CASE(w, , mb_4, dmb ish, "memory") +__CMPXCHG_CASE( , , mb_8, dmb ish, "memory") + +#undef __CMPXCHG_CASE +#endif /* __ASM_ATOMIC_LL_SC_H */
arch/arm64/include/asm/atomic_lse.h +40 -0
@@ -226,4 +226,44 @@ static inline long atomic64_dec_if_positive(atomic64_t *v)#undef __LL_SC_ATOMIC64 +#define __LL_SC_CMPXCHG (op, tmp) \ + __LL_SC_SAVE_LR(tmp) \ + __LL_SC_CALL(__cmpxchg_case_##op) \ + __LL_SC_RESTORE_LR(tmp) + +#define __CMPXCHG_CASE (w, sz, name, mb, cl) \ +static inline unsigned long __cmpxchg_case_##name(volatile void *ptr, \ + unsigned long old, \ + unsigned long new) \ +{ \ + unsigned long tmp; \ + register unsigned long x0 asm ("x0") = (unsigned long)ptr; \ + register unsigned long x1 asm ("x1") = old; \ + register unsigned long x2 asm ("x2") = new; \ + \ + asm volatile( \ + ARM64_LSE_ATOMIC_INSN(__LL_SC_CMPXCHG(name, %[tmp]), \ + " mov %" #w "[tmp], %" #w "[old]\n" \ + " cas" #mb #sz "\t%" #w "[tmp], %" #w "[new], %[v]\n" \ + " mov %" #w "[ret], %" #w "[tmp]") \ + : [tmp] "=&r" (tmp), [ret] "+r" (x0), \ + [v] "+Q" (*(unsigned long *)ptr) \ + : [old] "r" (x1), [new] "r" (x2) \ + : cl); \ + \ + return x0; \ +} + +__CMPXCHG_CASE(w, b, 1, , ) +__CMPXCHG_CASE(w, h, 2, , ) +__CMPXCHG_CASE(w, , 4, , ) +__CMPXCHG_CASE( , , 8, , ) +__CMPXCHG_CASE(w, b, mb_1, al, "memory") +__CMPXCHG_CASE(w, h, mb_2, al, "memory") +__CMPXCHG_CASE(w, , mb_4, al, "memory") +__CMPXCHG_CASE( , , mb_8, al, "memory") + +#undef __LL_SC_CMPXCHG +#undef __CMPXCHG_CASE +#endif /* __ASM_ATOMIC_LSE_H */
arch/arm64/include/asm/cmpxchg.h +19 -65
112 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 11/18 arm64: cmpxchg_dbl: patch in lse instructions when supported by the CPU
In reply to Will DeaconOn CPUs which support the LSE atomic instructions introduced in ARMv8.1, it makes sense to use them in preference to ll/sc sequences.
This patch introduces runtime patching of our cmpxchg_double primitives so that the LSE casp instruction is used instead.
--- arch/arm64/include/asm/atomic_ll_sc.h | 34 ++++++++++++++++++ arch/arm64/include/asm/atomic_lse.h | 45 +++++++++++++++++++++++ arch/arm64/include/asm/cmpxchg.h | 68 +++++++++-------------------------- 3 files changed, 96 insertions(+), 51 deletions(-)
arch/arm64/include/asm/atomic_ll_sc.h +34 -0
@@ -243,4 +243,38 @@ __CMPXCHG_CASE( , , mb_8, dmb ish, "memory")#undef __CMPXCHG_CASE +#define __CMPXCHG_DBL (name, mb, cl) \ +__LL_SC_INLINE int \ +__LL_SC_PREFIX(__cmpxchg_double##name(unsigned long old1, \ + unsigned long old2, \ + unsigned long new1, \ + unsigned long new2, \ + volatile void *ptr)) \ +{ \ + unsigned long tmp, ret; \ + \ + asm volatile("// __cmpxchg_double" #name "\n" \ + " " #mb "\n" \ + "1: ldxp %0, %1, %2\n" \ + " eor %0, %0, %3\n" \ + " eor %1, %1, %4\n" \ + " orr %1, %0, %1\n" \ + " cbnz %1, 2f\n" \ + " stxp %w0, %5, %6, %2\n" \ + " cbnz %w0, 1b\n" \ + " " #mb "\n" \ + "2:" \ + : "=&r" (tmp), "=&r" (ret), "+Q" (*(unsigned long *)ptr) \ + : "r" (old1), "r" (old2), "r" (new1), "r" (new2) \ + : cl); \ + \ + return ret; \ +} \ +__LL_SC_EXPORT(__cmpxchg_double##name); + +__CMPXCHG_DBL( , , ) +__CMPXCHG_DBL(_mb, dmb ish, "memory") + +#undef __CMPXCHG_DBL +#endif /* __ASM_ATOMIC_LL_SC_H */
arch/arm64/include/asm/atomic_lse.h +45 -0
49 lines not on this page · raw message
arch/arm64/include/asm/cmpxchg.h +17 -51
89 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 12/18 arm64: cmpxchg: avoid "cc" clobber in ll/sc routines
In reply to Will DeaconWe can perform the cmpxchg comparison using eor and cbnz which avoids the "cc" clobber for the ll/sc case and consequently for the LSE case where we may have to fall-back on the ll/sc code at runtime.
--- arch/arm64/include/asm/atomic_ll_sc.h | 14 ++++++-------- arch/arm64/include/asm/atomic_lse.h | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-)
arch/arm64/include/asm/atomic_ll_sc.h +6 -8
@@ -96,14 +96,13 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new)) asmvolatile ("// atomic_cmpxchg\n" "1: ldxr %w1, %2\n" -" cmp %w1, %w3\n" -" b.ne 2f\n" +" eor %w0, %w1, %w3\n" +" cbnz %w0, 2f\n" " stxr %w0, %w4, %2\n" " cbnz %w0, 1b\n" "2:" :"=&r" (tmp),"=&r" (oldval),"+Q" (ptr->counter) - :"Ir" (old),"r" (new) - :"cc" ); + :"Lr" (old),"r" (new));smp_mb ();return oldval;
@@ -169,14 +168,13 @@ __LL_SC_PREFIX(atomic64_cmpxchg(atomic64_t *ptr, long old, long new)) asmvolatile ("// atomic64_cmpxchg\n" "1: ldxr %1, %2\n" -" cmp %1, %3\n" -" b.ne 2f\n" +" eor %0, %1, %3\n" +" cbnz %w0, 2f\n" " stxr %w0, %4, %2\n" " cbnz %w0, 1b\n" "2:" :"=&r" (res),"=&r" (oldval),"+Q" (ptr->counter) - :"Ir" (old),"r" (new) - :"cc" ); + :"Lr" (old),"r" (new));smp_mb ();return oldval;
arch/arm64/include/asm/atomic_lse.h +2 -2
@@ -105,7 +105,7 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new) " mov %w[ret], %w[tmp]") : [tmp] "=&r" (tmp), [ret] "+r" (x0), [v] "+Q" (ptr->counter) : [old] "r" (w1), [new] "r" (w2) - : "cc", "memory"); + : "memory"); return x0; }
@@ -191,7 +191,7 @@ static inline long atomic64_cmpxchg(atomic64_t *ptr, long old, long new) " mov %[ret], %[tmp]") : [tmp] "=&r" (tmp), [ret] "+r" (x0), [v] "+Q" (ptr->counter) : [old] "r" (x1), [new] "r" (x2) - : "cc", "memory"); + : "memory"); return x0; }
Signature · 1 line
2.1.4
Catalin Marinas
maintainerPATCH 12/18 arm64: cmpxchg: avoid "cc" clobber in ll/sc routines
Quoted 20 lines · Will Deacon
We can perform the cmpxchg comparison using eor and cbnz which avoids the "cc" clobber for the ll/sc case and consequently for the LSE case where we may have to fall-back on the ll/sc code at runtime.
Reviewed-by Steve Capper <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/include/asm/atomic_ll_sc.h | 14 ++++++-------- arch/arm64/include/asm/atomic_lse.h | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-)arch/arm64/include/asm/atomic_ll_sc.h +0
-1@@ -96,14 +96,13 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new)) asmvolatile ("// atomic_cmpxchg\n" "1: ldxr %w1, %2\n" -" cmp %w1, %w3\n"
arch/arm64/include/asm/atomic_ll_sc.h __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new)) · line 105
Quoted 10 lines
arch/arm64/include/asm/atomic_ll_sc.h +3
-3-" b.ne 2f\n" +" eor %w0, %w1, %w3\n" +" cbnz %w0, 2f\n" " stxr %w0, %w4, %2\n" " cbnz %w0, 1b\n" "2:" : "=&r" (tmp), "=&r" (oldval), "+Q" (ptr->counter) - : "Ir" (old), "r" (new) - : "cc"); + : "Lr" (old), "r" (new));
For the LL/SC case, does this make things any slower? We replace a cmp + b.ne with two arithmetic ops (eor and cbnz, unless the latter is somehow smarter). I don't think the condition flags usually need to be preserved across an asm statement, so the "cc" clobber probably didn't make much difference anyway.
Signature · 1 line
Catalin
PATCH 12/18 arm64: cmpxchg: avoid "cc" clobber in ll/sc routines
Quoted 29 lines · Catalin Marinas
Quoted 28 lines · Will Deacon
We can perform the cmpxchg comparison using eor and cbnz which avoids the "cc" clobber for the ll/sc case and consequently for the LSE case where we may have to fall-back on the ll/sc code at runtime.
Reviewed-by Steve Capper <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/include/asm/atomic_ll_sc.h | 14 ++++++-------- arch/arm64/include/asm/atomic_lse.h | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-)arch/arm64/include/asm/atomic_ll_sc.h +2
-3@@ -96,14 +96,13 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new)) asmvolatile ("// atomic_cmpxchg\n" "1: ldxr %w1, %2\n" -" cmp %w1, %w3\n" -" b.ne 2f\n" +" eor %w0, %w1, %w3\n" +" cbnz %w0, 2f\n" " stxr %w0, %w4, %2\n" " cbnz %w0, 1b\n" "2:" : "=&r" (tmp), "=&r" (oldval), "+Q" (ptr->counter) - : "Ir" (old), "r" (new)
Quoted 8 lines
arch/arm64/include/asm/atomic_ll_sc.h __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new)) · line 105
Quoted 2 lines
arch/arm64/include/asm/atomic_ll_sc.h +1
-1- : "cc"); + : "Lr" (old), "r" (new));For the LL/SC case, does this make things any slower? We replace a cmp + b.ne with two arithmetic ops (eor and cbnz, unless the latter is somehow smarter). I don't think the condition flags usually need to be preserved across an asm statement, so the "cc" clobber probably didn't make much difference anyway.
I doubt you can measure it either way. The main reason for changing this was for consistency with other, similar code and improved readability (since otherwise we have a mystery "cc" clobber in the LSE version).
Will
PATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
In reply to Will Deaconcmpxchg doesn't require memory barrier semantics when the value comparison fails, so make the barrier conditional on success.
--- arch/arm64/include/asm/atomic_ll_sc.h | 48 ++++++++++++++++------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
arch/arm64/include/asm/atomic_ll_sc.h +22 -26
@@ -92,19 +92,18 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new))unsigned long tmp;int oldval; -smp_mb (); - asmvolatile ("// atomic_cmpxchg\n" "1: ldxr %w1, %2\n" " eor %w0, %w1, %w3\n" " cbnz %w0, 2f\n" -" stxr %w0, %w4, %2\n" +" stlxr %w0, %w4, %2\n" " cbnz %w0, 1b\n" +" dmb ish\n" "2:" :"=&r" (tmp),"=&r" (oldval),"+Q" (ptr->counter) - :"Lr" (old),"r" (new)); + :"Lr" (old),"r" (new) + :"memory" ); -smp_mb ();return oldval; }__LL_SC_EXPORT (atomic_cmpxchg);
@@ -164,19 +163,18 @@ __LL_SC_PREFIX(atomic64_cmpxchg(atomic64_t *ptr, long old, long new))long oldval;unsigned long res; -smp_mb (); - asmvolatile ("// atomic64_cmpxchg\n" "1: ldxr %1, %2\n" " eor %0, %1, %3\n" " cbnz %w0, 2f\n" -" stxr %w0, %4, %2\n" +" stlxr %w0, %4, %2\n" " cbnz %w0, 1b\n" +" dmb ish\n" "2:" :"=&r" (res),"=&r" (oldval),"+Q" (ptr->counter) - :"Lr" (old),"r" (new)); + :"Lr" (old),"r" (new) + :"memory" ); -smp_mb ();return oldval; }__LL_SC_EXPORT (atomic64_cmpxchg);
@@ -203,7 +201,7 @@ __LL_SC_PREFIX(atomic64_dec_if_positive(atomic64_t *v)) }__LL_SC_EXPORT (atomic64_dec_if_positive); -#define __CMPXCHG_CASE (w, sz, name, mb, cl) \ +#define __CMPXCHG_CASE (w, sz, name, mb, rel, cl) \ __LL_SC_INLINE unsigned long \ __LL_SC_PREFIX(__cmpxchg_case_##name(volatile void *ptr, \ unsigned long old, \
@@ -212,11 +210,10 @@ __LL_SC_PREFIX(__cmpxchg_case_##name(volatile void *ptr, \unsigned long tmp, oldval; \ \ asm volatile( \ - " " #mb "\n" \ "1: ldxr" #sz "\t%" #w "[oldval], %[v]\n" \ " eor %" #w "[tmp], %" #w "[oldval], %" #w "[old]\n" \ " cbnz %" #w "[tmp], 2f\n" \ - " stxr" #sz "\t%w[tmp], %" #w "[new], %[v]\n" \ + " st" #rel "xr" #sz "\t%w[tmp], %" #w "[new], %[v]\n" \ " cbnz %w[tmp], 1b\n" \ " " #mb "\n" \ " mov %" #w "[oldval], %" #w "[old]\n" \
@@ -230,18 +227,18 @@ __LL_SC_PREFIX(__cmpxchg_case_##name(volatile void *ptr, \ } \ __LL_SC_EXPORT(__cmpxchg_case_##name); -__CMPXCHG_CASE(w, b, 1, , ) -__CMPXCHG_CASE(w, h, 2, , ) -__CMPXCHG_CASE(w, , 4, , ) -__CMPXCHG_CASE( , , 8, , ) -__CMPXCHG_CASE(w, b, mb_1, dmb ish, "memory") -__CMPXCHG_CASE(w, h, mb_2, dmb ish, "memory") -__CMPXCHG_CASE(w, , mb_4, dmb ish, "memory") -__CMPXCHG_CASE( , , mb_8, dmb ish, "memory") +__CMPXCHG_CASE(w, b, 1, , , ) +__CMPXCHG_CASE(w, h, 2, , , ) +__CMPXCHG_CASE(w, , 4, , , ) +__CMPXCHG_CASE( , , 8, , , ) +__CMPXCHG_CASE(w, b, mb_1, dmb ish, l, "memory") +__CMPXCHG_CASE(w, h, mb_2, dmb ish, l, "memory") +__CMPXCHG_CASE(w, , mb_4, dmb ish, l, "memory") +__CMPXCHG_CASE( , , mb_8, dmb ish, l, "memory")#undef __CMPXCHG_CASE -#define __CMPXCHG_DBL (name, mb, cl) \ +#define __CMPXCHG_DBL (name, mb, rel, cl) \ __LL_SC_INLINE int \ __LL_SC_PREFIX(__cmpxchg_double##name(unsigned long old1, \ unsigned long old2, \
@@ -252,13 +249,12 @@ __LL_SC_PREFIX(__cmpxchg_double##name(unsigned long old1, \unsigned long tmp, ret; \ \ asm volatile("// __cmpxchg_double" #name "\n" \ - " " #mb "\n" \ "1: ldxp %0, %1, %2\n" \ " eor %0, %0, %3\n" \ " eor %1, %1, %4\n" \ " orr %1, %0, %1\n" \ " cbnz %1, 2f\n" \ - " stxp %w0, %5, %6, %2\n" \ + " st" #rel "xp %w0, %5, %6, %2\n" \ " cbnz %w0, 1b\n" \ " " #mb "\n" \ "2:" \
@@ -270,8 +266,8 @@ __LL_SC_PREFIX(__cmpxchg_double##name(unsigned long old1, \ } \ __LL_SC_EXPORT(__cmpxchg_double##name); -__CMPXCHG_DBL( , , ) -__CMPXCHG_DBL(_mb, dmb ish, "memory") +__CMPXCHG_DBL( , , , ) +__CMPXCHG_DBL(_mb, dmb ish, l, "memory")#undef __CMPXCHG_DBL
Signature · 1 line
2.1.4
Peter Zijlstra
maintainerPATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 2 lines · Will Deacon
cmpxchg doesn't require memory barrier semantics when the value comparison fails, so make the barrier conditional on success.
So this isn't actually a documented condition.
I would very much like a fwe extra words on this, that you've indeed audited cmpxchg() users and preferably even a Documentation/ update to match.
PATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 5 lines · Peter Zijlstra
Quoted 2 lines · Will Deacon
cmpxchg doesn't require memory barrier semantics when the value comparison fails, so make the barrier conditional on success.
So this isn't actually a documented condition.
There's *something* in Documentation/atomic_ops.txt (literally, the last sentence in the file) but it's terrible at best:
"Note that this also means that for the case where the counter
is not dropping to zero, there are no memory ordering requirements."
[you probably want to see the example for some context]
Quoted 3 lines
I would very much like a fwe extra words on this, that you've indeed audited
cmpxchg()users and preferably even aDocumentation/update to match.
Happy to update the docs. In terms of code audit, I couldn't find any cmpxchg users that do something along the lines of "if the comparison fails, don't loop, and instead do something to an independent address, without barrier semantics that must be observed after the failed CAS":
- Most (as in, it's hard to find other cases) users just loop until success, so there's no issue there.
- One use-case with work on the failure path is stats update (e.g. drivers/net/ethernet/intel/ixgbe/ixgbe.h), but barrier semantics aren't required here anyway.
- Another use-case is where you optimistically try a cmpxchg, then fall back on a lock if you fail (e.g. slub and cmpxchg_double).
- Some other archs appear to do the same trick (alpha and powerpc).
So I'm confident with this change, but agree that a Docs update would be beneficial. Something like below, or do you want some additional text, too?
Will
--->8
Documentation/memory-barriers.txt +3 -3
@@ -2383,9 +2383,7 @@ about the state (old or new) implies an SMP-conditional general memory barrier explicit lock operations, described later). These include: xchg(); - cmpxchg(); atomic_xchg(); atomic_long_xchg(); - atomic_cmpxchg(); atomic_long_cmpxchg(); atomic_inc_return(); atomic_long_inc_return(); atomic_dec_return(); atomic_long_dec_return(); atomic_add_return(); atomic_long_add_return();
@@ -2398,7 +2396,9 @@ explicit lock operations, described later). These include: test_and_clear_bit(); test_and_change_bit(); - /* when succeeds (returns 1) */ + /* when succeeds */ + cmpxchg(); + atomic_cmpxchg(); atomic_long_cmpxchg(); atomic_add_unless(); atomic_long_add_unless(); These are used for such things as implementing ACQUIRE-class and RELEASE-class
Peter Zijlstra
maintainerPATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 32 lines · Will Deacon
Quoted 5 lines · Peter Zijlstra
Quoted 2 lines · Will Deacon
cmpxchg doesn't require memory barrier semantics when the value comparison fails, so make the barrier conditional on success.
So this isn't actually a documented condition.
There's *something* in
Documentation/atomic_ops.txt(literally, the last sentence in the file) but it's terrible at best:"Note that this also means that for the case where the counter
is not dropping to zero, there are no memory ordering requirements."[you probably want to see the example for some context]
Quoted 3 lines
I would very much like a fwe extra words on this, that you've indeed audited
cmpxchg()users and preferably even aDocumentation/update to match.Happy to update the docs. In terms of code audit, I couldn't find any cmpxchg users that do something along the lines of "if the comparison fails, don't loop, and instead do something to an independent address, without barrier semantics that must be observed after the failed CAS":
- Most (as in, it's hard to find other cases) users just loop until success, so there's no issue there.
- One use-case with work on the failure path is stats update (e.g.
drivers/net/ethernet/intel/ixgbe/ixgbe.h), but barrier semantics aren't required here anyway.
Quoted 8 lines
- Another use-case is where you optimistically try a cmpxchg, then fall back on a lock if you fail (e.g. slub and
cmpxchg_double).- Some other archs appear to do the same trick (alpha and powerpc).
So I'm confident with this change, but agree that a Docs update would be beneficial. Something like below, or do you want some additional text, too?
How about kernel/locking/qspinlock_paravirt.h:__pv_queued_spin_unlock()
In that case we rely on the full memory barrier of the failed cmpxchg() to order the load of &l->locked vs the content of node.
So in pv_wait_head() we:
pv_hash(lock)
MB
->locked = _SLOW_VAL
And in __pv_queued_spin_unlock() we fail the cmpxchg when _SLOW_VAL and rely on the barrier to ensure we observe the results of pv_hash().
PATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 27 lines · Peter Zijlstra
Quoted 12 lines · Will Deacon
Happy to update the docs. In terms of code audit, I couldn't find any cmpxchg users that do something along the lines of "if the comparison fails, don't loop, and instead do something to an independent address, without barrier semantics that must be observed after the failed CAS":
- Most (as in, it's hard to find other cases) users just loop until success, so there's no issue there.
- One use-case with work on the failure path is stats update (e.g.
drivers/net/ethernet/intel/ixgbe/ixgbe.h), but barrier semantics aren't required here anyway.Quoted 8 lines
- Another use-case is where you optimistically try a cmpxchg, then fall back on a lock if you fail (e.g. slub and
cmpxchg_double).- Some other archs appear to do the same trick (alpha and powerpc).
So I'm confident with this change, but agree that a Docs update would be beneficial. Something like below, or do you want some additional text, too?
How about kernel/locking/qspinlock_paravirt.h:__pv_queued_spin_unlock()
In that case we rely on the full memory barrier of the failed
cmpxchg()to order the load of&l->lockedvs the content of node.
Quoted 8 lines
So in
pv_wait_head()we:
pv_hash(lock)
MB
->locked =_SLOW_VALAnd in
__pv_queued_spin_unlock()we fail the cmpxchg when_SLOW_VALand rely on the barrier to ensure we observe the results ofpv_hash().
That's an interesting case, and I think it's also broken on Alpha and Power (which don't use this code). It's fun actually, because a failed cmpxchg on those architectures gives you the barrier *before* the cmpxchg, but not the one afterwards so it doesn't actually help here.
So there's three options afaict:
(1) Document failed cmpxchg as having ACQUIRE semantics, and change this patch (and propose changes for Alpha and Power).
-or-
(2) Change pv_unhash to use fake dependency ordering across the hash.
-or-
(3) Put down an smp_rmb() between the cmpxchg and pv_unhash
The first two sound horrible, so I'd err towards 3, particularly as this is x86-only code atm and I don't think it will have an effect there.
Will
Peter Zijlstra
maintainerPATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 18 lines · Will Deacon
That's an interesting case, and I think it's also broken on Alpha and Power (which don't use this code). It's fun actually, because a failed cmpxchg on those architectures gives you the barrier *before* the cmpxchg, but not the one afterwards so it doesn't actually help here.
So there's three options afaict:
(1) Document failed cmpxchg as having ACQUIRE semantics, and change this patch (and propose changes for Alpha and Power).
(quoted diff) +0
-2-or- (2) Change pv_unhash to use fake dependency ordering across the hash. -or- (3) Put down an smp_rmb() between the cmpxchg and pv_unhash
Quoted 2 lines
The first two sound horrible, so I'd err towards 3, particularly as this is x86-only code atm and I don't think it will have an effect there.
Right, I would definitely go for 3, but it does show there is code out there :/
PATCH 13/18 arm64: cmpxchg: avoid memory barrier on comparison failure
Quoted 17 lines · Peter Zijlstra
Quoted 16 lines · Will Deacon
That's an interesting case, and I think it's also broken on Alpha and Power (which don't use this code). It's fun actually, because a failed cmpxchg on those architectures gives you the barrier *before* the cmpxchg, but not the one afterwards so it doesn't actually help here.
So there's three options afaict:
(1) Document failed cmpxchg as having ACQUIRE semantics, and change this patch (and propose changes for Alpha and Power).
(quoted diff) +0
-2-or- (2) Change pv_unhash to use fake dependency ordering across the hash. -or-
Quoted 7 lines
Quoted 4 lines
(3) Put down an
smp_rmb()between the cmpxchg andpv_unhashThe first two sound horrible, so I'd err towards 3, particularly as this is x86-only code atm and I don't think it will have an effect there.
Right, I would definitely go for 3, but it does show there is code out there :/
Yeah... but I think it's rare enough that I'd be willing to call it a bug and fix it up. Especially as the code in question is both (a) new and (b) only built for x86 atm (which doesn't have any of these issues).
FWIW, patch below. A future change would be making the cmpxchg a cmpxchg_release, which looks good in the unlock path and makes the need for the smp_rmb more obvious imo.
Anyway, one step at a time.
Will
--->8
commit e24f911487db52898b7f0567a9701e93d3c3f13a
Author: Will Deacon <[email protected]> Date: Mon Jul 13 16:46:59 2015 +0100
locking/pvqspinlock: order pv_unhash after cmpxchg on unlock slowpath
When we unlock in __pv_queued_spin_unlock, a failed cmpxchg on the lock value indicates that we need to take the slow-path and unhash the corresponding node blocked on the lock.
Since a failed cmpxchg does not provide any memory-ordering guarantees, it is possible that the node data could be read before the cmpxchg on weakly-ordered architectures and therefore return a stale value, leading to hash corruption and/or a BUG().
This patch adds an smb_rmb() following the failed cmpxchg operation, so that the unhashing is ordered after the lock has been checked.
kernel/locking/qspinlock_paravirt.h +7 -0
@@ -296,6 +296,13 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock)return ;/* +* A failed cmpxchg doesn't provide any memory-ordering guarantees, +* so we need a barrier to order the read of the node data in +* pv_unhash *after* we've read the lock being _Q_SLOW_VAL. +*/ +smp_rmb (); + +/* * Since the above failed to release, this must be the SLOW path. * Therefore start by looking up the blocked node and unhashing it. */
tip-bot for Will Deacon
merge bot[tip:locking/core] locking/pvqspinlock: Order pv_unhash() after cmpxchg() on unlock slowpath
locking/pvqspinlock: Order pv_unhash() after cmpxchg() on unlock slowpath
Commit-ID: 3b3fdf10a8add87ef0050138d51bfee9ab4983df Gitweb: http://git.kernel.org/tip/3b3fdf10a8add87ef0050138d51bfee9ab4983df Author: Will Deacon <[email protected]> AuthorDate: Mon, 13 Jul 2015 16:58:30 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 3 Aug 2015 10:57:09 +0200
locking/pvqspinlock: Order pv_unhash() after cmpxchg() on unlock slowpath
When we unlock in __pv_queued_spin_unlock(), a failed cmpxchg() on the lock value indicates that we need to take the slow-path and unhash the corresponding node blocked on the lock.
Since a failed cmpxchg() does not provide any memory-ordering guarantees, it is possible that the node data could be read before the cmpxchg() on weakly-ordered architectures and therefore return a stale value, leading to hash corruption and/or a BUG().
This patch adds an smb_rmb() following the failed cmpxchg operation, so that the unhashing is ordered after the lock has been checked.
--- kernel/locking/qspinlock_paravirt.h | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
kernel/locking/qspinlock_paravirt.h +18 -5
@@ -244,13 +244,17 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)if (!lp) {/* ONCE */ lp =pv_hash (lock, pn);/* -* lp must be set before setting _Q_SLOW_VAL +* We must hash before setting _Q_SLOW_VAL, such that +* when we observe _Q_SLOW_VAL in __pv_queued_spin_unlock() +* we'll be sure to be able to observe our hash entry. * -* [S] lp = lock [RmW] l = l->locked = 0 -* MB MB -* [S] l->locked = _Q_SLOW_VAL [L] lp +* [S] pn->state +* [S] <hash> [Rmw] l->locked == _Q_SLOW_VAL +* MB RMB +* [RmW] l->locked = _Q_SLOW_VAL [L] <unhash> +* [L] pn->state * -* Matches the cmpxchg() in __pv_queued_spin_unlock(). +* Matches the smp_rmb() in __pv_queued_spin_unlock(). */ if (!cmpxchg (&l->locked, _Q_LOCKED_VAL, _Q_SLOW_VAL)) {/*
@@ -306,6 +310,15 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock) }/* +* A failed cmpxchg doesn't provide any memory-ordering guarantees, +* so we need a barrier to order the read of the node data in +* pv_unhash *after* we've read the lock being _Q_SLOW_VAL. +* +* Matches the cmpxchg() in pv_wait_head() setting _Q_SLOW_VAL. +*/ +smp_rmb (); + +/* * Since the above failed to release, this must be the SLOW path. * Therefore start by looking up the blocked node and unhashing it. */
PATCH 14/18 arm64: atomics: tidy up common atomic{,64}_* macros
In reply to Will DeaconThe common (i.e. identical for ll/sc and lse) atomic macros in atomic.h are needlessley different for atomic_t and atomic64_t.
This patch tidies up the definitions to make them consistent across the two atomic types and factors out common code such as the add_unless implementation based on cmpxchg.
--- arch/arm64/include/asm/atomic.h | 92 +++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 54 deletions(-)
arch/arm64/include/asm/atomic.h +38 -54
@@ -26,8 +26,6 @@#include <asm/barrier.h> #include <asm/lse.h> -#define ATOMIC_INIT (i) { (i) } -#ifdef __KERNEL__#define __ARM64_IN_ATOMIC_IMPL
@@ -42,67 +40,53 @@#include <asm/cmpxchg.h> -/* -* On ARM, ordinary assignment (str instruction) doesn't clear the local -* strex/ldrex monitor on some implementations. The reason we can use it for -* atomic_set() is the clrex or dummy strex done on every exception return. -*/ -#define atomic_read (v) ACCESS_ONCE((v)->counter) -#define atomic_set (v,i) (((v)->counter) = (i)) - -#define atomic_xchg (v, new) (xchg(&((v)->counter), new)) - -static inline int __atomic_add_unless (atomic_t *v,int a,int u) -{ -int c, old; - - c =atomic_read (v); -while (c != u && (old =atomic_cmpxchg ((v), c, c + a)) != c) - c = old; -return c; -} +#define ___atomic_add_unless (v, a, u, sfx) \ +({ \ + typeof((v)->counter) c, old; \ + \ + c = atomic##sfx##_read(v); \ + while (c != (u) && \ + (old = atomic##sfx##_cmpxchg((v), c, c + (a))) != c) \ + c = old; \ + c; \ + }) -#define atomic_inc (v) atomic_add(1, v) -#define atomic_dec (v) atomic_sub(1, v) +#define ATOMIC_INIT (i) { (i) } -#define atomic_inc_and_test (v) (atomic_add_return(1, v) == 0) -#define atomic_dec_and_test (v) (atomic_sub_return(1, v) == 0) -#define atomic_inc_return (v) (atomic_add_return(1, v)) -#define atomic_dec_return (v) (atomic_sub_return(1, v)) -#define atomic_sub_and_test (i, v) (atomic_sub_return(i, v) == 0) +#define atomic_read (v) READ_ONCE((v)->counter) +#define atomic_set (v, i) (((v)->counter) = (i)) +#define atomic_xchg (v, new) xchg(&((v)->counter), (new)) -#define atomic_add_negative (i,v) (atomic_add_return(i, v) < 0) +#define atomic_inc (v) atomic_add(1, (v)) +#define atomic_dec (v) atomic_sub(1, (v)) +#define atomic_inc_return (v) atomic_add_return(1, (v)) +#define atomic_dec_return (v) atomic_sub_return(1, (v)) +#define atomic_inc_and_test (v) (atomic_inc_return(v) == 0) +#define atomic_dec_and_test (v) (atomic_dec_return(v) == 0) +#define atomic_sub_and_test (i, v) (atomic_sub_return((i), (v)) == 0) +#define atomic_add_negative (i, v) (atomic_add_return((i), (v)) < 0) +#define __atomic_add_unless (v, a, u) ___atomic_add_unless(v, a, u,)/* * 64-bit atomic operations. */ -#define ATOMIC64_INIT (i) { (i) } - -#define atomic64_read (v) ACCESS_ONCE((v)->counter) -#define atomic64_set (v,i) (((v)->counter) = (i)) - -#define atomic64_xchg (v, new) (xchg(&((v)->counter), new)) - -static inline int atomic64_add_unless (atomic64_t *v,long a,long u) -{ -long c, old; - - c =atomic64_read (v); -while (c != u && (old =atomic64_cmpxchg ((v), c, c + a)) != c) - c = old; +#define ATOMIC64_INIT ATOMIC_INIT +#define atomic64_read atomic_read +#define atomic64_set atomic_set +#define atomic64_xchg atomic_xchg + +#define atomic64_inc (v) atomic64_add(1, (v)) +#define atomic64_dec (v) atomic64_sub(1, (v)) +#define atomic64_inc_return (v) atomic64_add_return(1, (v)) +#define atomic64_dec_return (v) atomic64_sub_return(1, (v)) +#define atomic64_inc_and_test (v) (atomic64_inc_return(v) == 0) +#define atomic64_dec_and_test (v) (atomic64_dec_return(v) == 0) +#define atomic64_sub_and_test (i, v) (atomic64_sub_return((i), (v)) == 0) +#define atomic64_add_negative (i, v) (atomic64_add_return((i), (v)) < 0) +#define atomic64_add_unless (v, a, u) (___atomic_add_unless(v, a, u, 64) != u) -return c != u; -} +#define atomic64_inc_not_zero (v) atomic64_add_unless((v), 1, 0) -#define atomic64_add_negative (a, v) (atomic64_add_return((a), (v)) < 0) -#define atomic64_inc (v) atomic64_add(1LL, (v)) -#define atomic64_inc_return (v) atomic64_add_return(1LL, (v)) -#define atomic64_inc_and_test (v) (atomic64_inc_return(v) == 0) -#define atomic64_sub_and_test (a, v) (atomic64_sub_return((a), (v)) == 0) -#define atomic64_dec (v) atomic64_sub(1LL, (v)) -#define atomic64_dec_return (v) atomic64_sub_return(1LL, (v)) -#define atomic64_dec_and_test (v) (atomic64_dec_return((v)) == 0) -#define atomic64_inc_not_zero (v) atomic64_add_unless((v), 1LL, 0LL)#endif #endif
Signature · 1 line
2.1.4
PATCH 15/18 arm64: atomics: prefetch the destination word for write prior to stxr
In reply to Will DeaconThe cost of changing a cacheline from shared to exclusive state can be significant, especially when this is triggered by an exclusive store, since it may result in having to retry the transaction.
This patch makes use of prfm to prefetch cachelines for write prior to ldxr/stxr loops when using the ll/sc atomic routines.
--- arch/arm64/include/asm/atomic_ll_sc.h | 9 +++++++++ arch/arm64/include/asm/cmpxchg.h | 8 ++++++++ arch/arm64/include/asm/futex.h | 2 ++ arch/arm64/lib/bitops.S | 2 ++ 4 files changed, 21 insertions(+)
arch/arm64/include/asm/atomic_ll_sc.h +9 -0
@@ -45,6 +45,7 @@ __LL_SC_PREFIX(atomic_##op(int i, atomic_t *v)) \int result; \ \ asm volatile("// atomic_" #op "\n" \ +" prfm pstl1strm, %2\n" \ "1: ldxr %w0, %2\n" \ " " #asm_op " %w0, %w0, %w3\n" \ " stxr %w1, %w0, %2\n" \
@@ -62,6 +63,7 @@ __LL_SC_PREFIX(atomic_##op##_return(int i, atomic_t *v)) \int result; \ \ asm volatile("// atomic_" #op "_return\n" \ +" prfm pstl1strm, %2\n" \ "1: ldxr %w0, %2\n" \ " " #asm_op " %w0, %w0, %w3\n" \ " stlxr %w1, %w0, %2\n" \
@@ -93,6 +95,7 @@ __LL_SC_PREFIX(atomic_cmpxchg(atomic_t *ptr, int old, int new))int oldval; asmvolatile ("// atomic_cmpxchg\n" +" prfm pstl1strm, %2\n" "1: ldxr %w1, %2\n" " eor %w0, %w1, %w3\n" " cbnz %w0, 2f\n"
@@ -116,6 +119,7 @@ __LL_SC_PREFIX(atomic64_##op(long i, atomic64_t *v)) \unsigned long tmp; \ \ asm volatile("// atomic64_" #op "\n" \ +" prfm pstl1strm, %2\n" \ "1: ldxr %0, %2\n" \ " " #asm_op " %0, %0, %3\n" \ " stxr %w1, %0, %2\n" \
@@ -133,6 +137,7 @@ __LL_SC_PREFIX(atomic64_##op##_return(long i, atomic64_t *v)) \unsigned long tmp; \ \ asm volatile("// atomic64_" #op "_return\n" \ +" prfm pstl1strm, %2\n" \ "1: ldxr %0, %2\n" \ " " #asm_op " %0, %0, %3\n" \ " stlxr %w1, %0, %2\n" \
@@ -164,6 +169,7 @@ __LL_SC_PREFIX(atomic64_cmpxchg(atomic64_t *ptr, long old, long new))unsigned long res; asmvolatile ("// atomic64_cmpxchg\n" +" prfm pstl1strm, %2\n" "1: ldxr %1, %2\n" " eor %0, %1, %3\n" " cbnz %w0, 2f\n"
@@ -186,6 +192,7 @@ __LL_SC_PREFIX(atomic64_dec_if_positive(atomic64_t *v))unsigned long tmp; asmvolatile ("// atomic64_dec_if_positive\n" +" prfm pstl1strm, %2\n" "1: ldxr %0, %2\n" " subs %0, %0, #1\n" " b.mi 2f\n"
@@ -210,6 +217,7 @@ __LL_SC_PREFIX(__cmpxchg_case_##name(volatile void *ptr, \unsigned long tmp, oldval; \ \ asm volatile( \ + " prfm pstl1strm, %2\n" \ "1: ldxr" #sz "\t%" #w "[oldval], %[v]\n" \ " eor %" #w "[tmp], %" #w "[oldval], %" #w "[old]\n" \ " cbnz %" #w "[tmp], 2f\n" \
@@ -249,6 +257,7 @@ __LL_SC_PREFIX(__cmpxchg_double##name(unsigned long old1, \unsigned long tmp, ret; \ \ asm volatile("// __cmpxchg_double" #name "\n" \ + " prfm pstl1strm, %2\n" \ "1: ldxp %0, %1, %2\n" \ " eor %0, %0, %3\n" \ " eor %1, %1, %4\n" \
arch/arm64/include/asm/cmpxchg.h +8 -0
@@ -33,12 +33,14 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizecase 1 : asm volatile(ARM64_LSE_ATOMIC_INSN(/* LL/SC */ +" prfm pstl1strm, %2\n" "1: ldxrb %w0, %2\n" " stlxrb %w1, %w3, %2\n" " cbnz %w1, 1b\n" " dmb ish",/* LSE atomics */ " nop\n" +" nop\n" " swpalb %w3, %w0, %2\n" " nop\n" " nop")
@@ -49,12 +51,14 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizecase 2 : asm volatile(ARM64_LSE_ATOMIC_INSN(/* LL/SC */ +" prfm pstl1strm, %2\n" "1: ldxrh %w0, %2\n" " stlxrh %w1, %w3, %2\n" " cbnz %w1, 1b\n" " dmb ish",/* LSE atomics */ " nop\n" +" nop\n" " swpalh %w3, %w0, %2\n" " nop\n" " nop")
@@ -65,12 +69,14 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizecase 4 : asm volatile(ARM64_LSE_ATOMIC_INSN(/* LL/SC */ +" prfm pstl1strm, %2\n" "1: ldxr %w0, %2\n" " stlxr %w1, %w3, %2\n" " cbnz %w1, 1b\n" " dmb ish",/* LSE atomics */ " nop\n" +" nop\n" " swpal %w3, %w0, %2\n" " nop\n" " nop")
@@ -81,12 +87,14 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int sizecase 8 : asm volatile(ARM64_LSE_ATOMIC_INSN(/* LL/SC */ +" prfm pstl1strm, %2\n" "1: ldxr %0, %2\n" " stlxr %w1, %3, %2\n" " cbnz %w1, 1b\n" " dmb ish",/* LSE atomics */ " nop\n" +" nop\n" " swpal %3, %0, %2\n" " nop\n" " nop")
arch/arm64/include/asm/futex.h +2 -0
@@ -24,6 +24,7 @@#define __futex_atomic_op (insn, ret, oldval, uaddr, tmp, oparg) \ asm volatile( \ +" prfm pstl1strm, %2\n" \ "1: ldxr %w1, %2\n" \ insn "\n" \ "2: stlxr %w3, %w0, %2\n" \
@@ -112,6 +113,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,return -EFAULT ; asmvolatile ("// futex_atomic_cmpxchg_inatomic\n" +" prfm pstl1strm, %2\n" "1: ldxr %w1, %2\n" " sub %w3, %w1, %w4\n" " cbnz %w3, 3f\n"
arch/arm64/lib/bitops.S +2 -0
@@ -31,6 +31,7 @@ ENTRY( \name ) eor w0, w0, w3 // Clear low bits mov x2, #1 add x1, x1, x0, lsr #3 // Get word offset +alt_lse " prfm pstl1strm, [x1]", "nop" lsl x3, x2, x3 // Create mask alt_lse "1: ldxr x2, [x1]", "\lse x3, [x1]"
@@ -48,6 +49,7 @@ ENTRY( \name ) eor w0, w0, w3 // Clear low bits mov x2, #1 add x1, x1, x0, lsr #3 // Get word offset +alt_lse " prfm pstl1strm, [x1]", "nop" lsl x4, x2, x3 // Create mask alt_lse "1: ldxr x2, [x1]", "\lse x4, x2, [x1]"
Signature · 1 line
2.1.4
PATCH 16/18 arm64: atomics: implement atomic{, 64}_cmpxchg using cmpxchg
In reply to Will DeaconWe don't need duplicate cmpxchg implementations, so use cmpxchg to implement atomic{,64}_cmpxchg, like we do for xchg already.
--- arch/arm64/include/asm/atomic.h | 2 ++ arch/arm64/include/asm/atomic_ll_sc.h | 46 ----------------------------------- arch/arm64/include/asm/atomic_lse.h | 35 -------------------------- 3 files changed, 2 insertions(+), 81 deletions(-)
arch/arm64/include/asm/atomic.h +2 -0
@@ -56,6 +56,7 @@#define atomic_read (v) READ_ONCE((v)->counter)#define atomic_set (v, i) (((v)->counter) = (i))#define atomic_xchg (v, new) xchg(&((v)->counter), (new)) +#define atomic_cmpxchg (v, old, new) cmpxchg(&((v)->counter), (old), (new))#define atomic_inc (v) atomic_add(1, (v))#define atomic_dec (v) atomic_sub(1, (v))
@@ -74,6 +75,7 @@#define atomic64_read atomic_read#define atomic64_set atomic_set#define atomic64_xchg atomic_xchg +#define atomic64_cmpxchg atomic_cmpxchg#define atomic64_inc (v) atomic64_add(1, (v))#define atomic64_dec (v) atomic64_sub(1, (v))
arch/arm64/include/asm/atomic_ll_sc.h +0 -46
58 lines not on this page · raw message
arch/arm64/include/asm/atomic_lse.h +0 -35
@@ -92,24 +92,6 @@ static inline int atomic_sub_return(int i, atomic_t *v)return w0; } -static inline int atomic_cmpxchg (atomic_t *ptr,int old,int new) -{ -unsigned long tmp; - register unsigned long x0 asm ("x0") = (unsigned long)ptr; - register int w1 asm ("w1") = old; - register int w2 asm ("w2") = new; - - asm volatile(ARM64_LSE_ATOMIC_INSN(__LL_SC_ATOMIC(cmpxchg, %[tmp]), -" mov %w[tmp], %w[old]\n" -" casal %w[tmp], %w[new], %[v]\n" -" mov %w[ret], %w[tmp]" ) - : [tmp] "=&r" (tmp), [ret] "+r" (x0), [v] "+Q" (ptr->counter) - : [old] "r" (w1), [new] "r" (w2) - : "memory"); - - return x0; -} -#undef __LL_SC_ATOMIC#define __LL_SC_ATOMIC64 (op, tmp) \
@@ -178,23 +160,6 @@ static inline long atomic64_sub_return(long i, atomic64_t *v)return x0; } -static inline long atomic64_cmpxchg (atomic64_t *ptr,long old,long new) -{ -unsigned long tmp; - register unsigned long x0 asm ("x0") = (unsigned long)ptr; - register long x1 asm ("x1") = old; - register long x2 asm ("x2") = new; - - asm volatile(ARM64_LSE_ATOMIC_INSN(__LL_SC_ATOMIC64(cmpxchg, %[tmp]), -" mov %[tmp], %[old]\n" -" casal %[tmp], %[new], %[v]\n" -" mov %[ret], %[tmp]" ) - : [tmp] "=&r" (tmp), [ret] "+r" (x0), [v] "+Q" (ptr->counter) - : [old] "r" (x1), [new] "r" (x2) - : "memory"); - - return x0; -}static inline long atomic64_dec_if_positive (atomic64_t *v) {
Signature · 1 line
2.1.4
PATCH 17/18 arm64: atomic64_dec_if_positive: fix incorrect branch condition
In reply to Will DeaconIf we attempt to atomic64_dec_if_positive on INT_MIN, we will underflow and incorrectly decide that the original parameter was positive.
This patches fixes the broken condition code so that we handle this corner case correctly.
--- arch/arm64/include/asm/atomic_ll_sc.h | 2 +- arch/arm64/include/asm/atomic_lse.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/atomic_ll_sc.h +1 -1
@@ -149,7 +149,7 @@ __LL_SC_PREFIX(atomic64_dec_if_positive(atomic64_t *v))" prfm pstl1strm, %2\n" "1: ldxr %0, %2\n" " subs %0, %0, #1\n" -" b.mi 2f\n" +" b.lt 2f\n" " stlxr %w1, %0, %2\n" " cbnz %w1, 1b\n" " dmb ish\n"
arch/arm64/include/asm/atomic_lse.h +1 -1
@@ -176,7 +176,7 @@ static inline long atomic64_dec_if_positive(atomic64_t *v)/* LSE atomics */ "1: ldr %[tmp], %[v]\n" " subs %[ret], %[tmp], #1\n" -" b.mi 2f\n" +" b.lt 2f\n" " casal %[tmp], %[ret], %[v]\n" " sub %[tmp], %[tmp], #1\n" " sub %[tmp], %[tmp], %[ret]\n"
Signature · 1 line
2.1.4
PATCH 18/18 arm64: kconfig: select HAVE_CMPXCHG_LOCAL
In reply to Will DeaconWe implement an optimised cmpxchg_local macro, so let the kernel know.
--- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+)
arch/arm64/Kconfig +1 -0
@@ -53,6 +53,7 @@ config ARM64 select HAVE_C_RECORDMCOUNT select HAVE_CC_STACKPROTECTOR select HAVE_CMPXCHG_DOUBLE + select HAVE_CMPXCHG_LOCAL select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_API_DEBUG
Signature · 1 line
2.1.4