Skip to content

Commit 6d2779e

Browse files
mrutland-armPeter Zijlstra
authored andcommitted
locking/atomic: scripts: fix fallback ifdeffery
Since commit: 9257959 ("locking/atomic: scripts: restructure fallback ifdeffery") The ordering fallbacks for atomic*_read_acquire() and atomic*_set_release() erroneously fall back to the implictly relaxed atomic*_read() and atomic*_set() variants respectively, without any additional barriers. This loses the ACQUIRE and RELEASE ordering semantics, which can result in a wide variety of problems, even on strongly-ordered architectures where the implementation of atomic*_read() and/or atomic*_set() allows the compiler to reorder those relative to other accesses. In practice this has been observed to break bit spinlocks on arm64, resulting in dentry cache corruption. The fallback logic was intended to allow ACQUIRE/RELEASE/RELAXED ops to be defined in terms of FULL ops, but where an op had RELAXED ordering by default, this unintentionally permitted the ACQUIRE/RELEASE ops to be defined in terms of the implicitly RELAXED default. This patch corrects the logic to avoid falling back to implicitly RELAXED ops, resulting in the same behaviour as prior to commit 9257959. I've verified the resulting assembly on arm64 by generating outlined wrappers of the atomics. Prior to this patch the compiler generates sequences using relaxed load (LDR) and store (STR) instructions, e.g. | <outlined_atomic64_read_acquire>: | ldr x0, [x0] | ret | | <outlined_atomic64_set_release>: | str x1, [x0] | ret With this patch applied the compiler generates sequences using the intended load-acquire (LDAR) and store-release (STLR) instructions, e.g. | <outlined_atomic64_read_acquire>: | ldar x0, [x0] | ret | | <outlined_atomic64_set_release>: | stlr x1, [x0] | ret To make sure that there were no other victims of the ifdeffery rewrite, I generated outlined copies of all of the {atomic,atomic64,atomic_long} atomic operations before and after commit 9257959. A diff of the generated assembly on arm64 shows that only the read_acquire() and set_release() operations were changed, and only lost their intended ordering: | [mark@lakrids:~/src/linux]% diff -u \ | <(aarch64-linux-gnu-objdump -d before-9257959a6e5b4fca.o) | <(aarch64-linux-gnu-objdump -d after-9257959a6e5b4fca.o) | --- /proc/self/fd/11 2023-09-19 16:51:51.114779415 +0100 | +++ /proc/self/fd/16 2023-09-19 16:51:51.114779415 +0100 | @@ -1,5 +1,5 @@ | | -before-9257959a6e5b4fca.o: file format elf64-littleaarch64 | +after-9257959a6e5b4fca.o: file format elf64-littleaarch64 | | | Disassembly of section .text: | @@ -9,7 +9,7 @@ | 4: d65f03c0 ret | | 0000000000000008 <outlined_atomic_read_acquire>: | - 8: 88dffc00 ldar w0, [x0] | + 8: b9400000 ldr w0, [x0] | c: d65f03c0 ret | | 0000000000000010 <outlined_atomic_set>: | @@ -17,7 +17,7 @@ | 14: d65f03c0 ret | | 0000000000000018 <outlined_atomic_set_release>: | - 18: 889ffc01 stlr w1, [x0] | + 18: b9000001 str w1, [x0] | 1c: d65f03c0 ret | | 0000000000000020 <outlined_atomic_add>: | @@ -1230,7 +1230,7 @@ | 1070: d65f03c0 ret | | 0000000000001074 <outlined_atomic64_read_acquire>: | - 1074: c8dffc00 ldar x0, [x0] | + 1074: f9400000 ldr x0, [x0] | 1078: d65f03c0 ret | | 000000000000107c <outlined_atomic64_set>: | @@ -1238,7 +1238,7 @@ | 1080: d65f03c0 ret | | 0000000000001084 <outlined_atomic64_set_release>: | - 1084: c89ffc01 stlr x1, [x0] | + 1084: f9000001 str x1, [x0] | 1088: d65f03c0 ret | | 000000000000108c <outlined_atomic64_add>: | @@ -2427,7 +2427,7 @@ | 207c: d65f03c0 ret | | 0000000000002080 <outlined_atomic_long_read_acquire>: | - 2080: c8dffc00 ldar x0, [x0] | + 2080: f9400000 ldr x0, [x0] | 2084: d65f03c0 ret | | 0000000000002088 <outlined_atomic_long_set>: | @@ -2435,7 +2435,7 @@ | 208c: d65f03c0 ret | | 0000000000002090 <outlined_atomic_long_set_release>: | - 2090: c89ffc01 stlr x1, [x0] | + 2090: f9000001 str x1, [x0] | 2094: d65f03c0 ret | | 0000000000002098 <outlined_atomic_long_add>: I've build tested this with a variety of configs for alpha, arm, arm64, csky, i386, m68k, microblaze, mips, nios2, openrisc, powerpc, riscv, s390, sh, sparc, x86_64, and xtensa, for which I've seen no issues. I was unable to build test for ia64 and parisc due to existing build breakage in v6.6-rc2. Fixes: 9257959 ("locking/atomic: scripts: restructure fallback ifdeffery") Reported-by: Ming Lei <ming.lei@redhat.com> Reported-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Tested-by: Baokun Li <libaokun1@huawei.com> Link: https://lkml.kernel.org/r/20230919171430.2697727-1-mark.rutland@arm.com
1 parent ce9ecca commit 6d2779e

File tree

2 files changed

+2
-10
lines changed

2 files changed

+2
-10
lines changed

include/linux/atomic/atomic-arch-fallback.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,6 @@ raw_atomic_read_acquire(const atomic_t *v)
459459
{
460460
#if defined(arch_atomic_read_acquire)
461461
return arch_atomic_read_acquire(v);
462-
#elif defined(arch_atomic_read)
463-
return arch_atomic_read(v);
464462
#else
465463
int ret;
466464

@@ -508,8 +506,6 @@ raw_atomic_set_release(atomic_t *v, int i)
508506
{
509507
#if defined(arch_atomic_set_release)
510508
arch_atomic_set_release(v, i);
511-
#elif defined(arch_atomic_set)
512-
arch_atomic_set(v, i);
513509
#else
514510
if (__native_word(atomic_t)) {
515511
smp_store_release(&(v)->counter, i);
@@ -2575,8 +2571,6 @@ raw_atomic64_read_acquire(const atomic64_t *v)
25752571
{
25762572
#if defined(arch_atomic64_read_acquire)
25772573
return arch_atomic64_read_acquire(v);
2578-
#elif defined(arch_atomic64_read)
2579-
return arch_atomic64_read(v);
25802574
#else
25812575
s64 ret;
25822576

@@ -2624,8 +2618,6 @@ raw_atomic64_set_release(atomic64_t *v, s64 i)
26242618
{
26252619
#if defined(arch_atomic64_set_release)
26262620
arch_atomic64_set_release(v, i);
2627-
#elif defined(arch_atomic64_set)
2628-
arch_atomic64_set(v, i);
26292621
#else
26302622
if (__native_word(atomic64_t)) {
26312623
smp_store_release(&(v)->counter, i);
@@ -4657,4 +4649,4 @@ raw_atomic64_dec_if_positive(atomic64_t *v)
46574649
}
46584650

46594651
#endif /* _LINUX_ATOMIC_FALLBACK_H */
4660-
// 202b45c7db600ce36198eb1f1fc2c2d5268ace2d
4652+
// 2fdd6702823fa842f9cea57a002e6e4476ae780c

scripts/atomic/gen-atomic-fallback.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ gen_proto_order_variant()
102102
fi
103103

104104
# Allow ACQUIRE/RELEASE/RELAXED ops to be defined in terms of FULL ops
105-
if [ ! -z "${order}" ]; then
105+
if [ ! -z "${order}" ] && ! meta_is_implicitly_relaxed "${meta}"; then
106106
printf "#elif defined(arch_${basename})\n"
107107
printf "\t${retstmt}arch_${basename}(${args});\n"
108108
fi

0 commit comments

Comments
 (0)