Skip to content

Commit dc1b4df

Browse files
mrutland-armPeter Zijlstra
authored andcommitted
atomics: Fix atomic64_{read_acquire,set_release} fallbacks
Arnd reports that on 32-bit architectures, the fallbacks for atomic64_read_acquire() and atomic64_set_release() are broken as they use smp_load_acquire() and smp_store_release() respectively, which do not work on types larger than the native word size. Since those contain compiletime_assert_atomic_type(), any attempt to use those fallbacks will result in a build-time error. e.g. with the following added to arch/arm/kernel/setup.c: | void test_atomic64(atomic64_t *v) | { | atomic64_set_release(v, 5); | atomic64_read_acquire(v); | } The compiler will complain as follows: | In file included from <command-line>: | In function 'arch_atomic64_set_release', | inlined from 'test_atomic64' at ./include/linux/atomic/atomic-instrumented.h:669:2: | ././include/linux/compiler_types.h:346:38: error: call to '__compiletime_assert_9' declared with attribute error: Need native word sized stores/loads for atomicity. | 346 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | | ^ | ././include/linux/compiler_types.h:327:4: note: in definition of macro '__compiletime_assert' | 327 | prefix ## suffix(); \ | | ^~~~~~ | ././include/linux/compiler_types.h:346:2: note: in expansion of macro '_compiletime_assert' | 346 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | | ^~~~~~~~~~~~~~~~~~~ | ././include/linux/compiler_types.h:349:2: note: in expansion of macro 'compiletime_assert' | 349 | compiletime_assert(__native_word(t), \ | | ^~~~~~~~~~~~~~~~~~ | ./include/asm-generic/barrier.h:133:2: note: in expansion of macro 'compiletime_assert_atomic_type' | 133 | compiletime_assert_atomic_type(*p); \ | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ./include/asm-generic/barrier.h:164:55: note: in expansion of macro '__smp_store_release' | 164 | #define smp_store_release(p, v) do { kcsan_release(); __smp_store_release(p, v); } while (0) | | ^~~~~~~~~~~~~~~~~~~ | ./include/linux/atomic/atomic-arch-fallback.h:1270:2: note: in expansion of macro 'smp_store_release' | 1270 | smp_store_release(&(v)->counter, i); | | ^~~~~~~~~~~~~~~~~ | make[2]: *** [scripts/Makefile.build:288: arch/arm/kernel/setup.o] Error 1 | make[1]: *** [scripts/Makefile.build:550: arch/arm/kernel] Error 2 | make: *** [Makefile:1831: arch/arm] Error 2 Fix this by only using smp_load_acquire() and smp_store_release() for native atomic types, and otherwise falling back to the regular barriers necessary for acquire/release semantics, as we do in the more generic acquire and release fallbacks. Since the fallback templates are used to generate the atomic64_*() and atomic_*() operations, the __native_word() check is added to both. For the atomic_*() operations, which are always 32-bit, the __native_word() check is redundant but not harmful, as it is always true. For the example above this works as expected on 32-bit, e.g. for arm multi_v7_defconfig: | <test_atomic64>: | push {r4, r5} | dmb ish | pldw [r0] | mov r2, #5 | mov r3, #0 | ldrexd r4, [r0] | strexd r4, r2, [r0] | teq r4, #0 | bne 484 <test_atomic64+0x14> | ldrexd r2, [r0] | dmb ish | pop {r4, r5} | bx lr ... and also on 64-bit, e.g. for arm64 defconfig: | <test_atomic64>: | bti c | paciasp | mov x1, #0x5 | stlr x1, [x0] | ldar x0, [x0] | autiasp | ret Reported-by: Arnd Bergmann <[email protected]> Signed-off-by: Mark Rutland <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Ard Biesheuvel <[email protected]> Reviewed-by: Boqun Feng <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent c441e93 commit dc1b4df

File tree

3 files changed

+49
-7
lines changed

3 files changed

+49
-7
lines changed

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

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,16 @@
151151
static __always_inline int
152152
arch_atomic_read_acquire(const atomic_t *v)
153153
{
154-
return smp_load_acquire(&(v)->counter);
154+
int ret;
155+
156+
if (__native_word(atomic_t)) {
157+
ret = smp_load_acquire(&(v)->counter);
158+
} else {
159+
ret = arch_atomic_read(v);
160+
__atomic_acquire_fence();
161+
}
162+
163+
return ret;
155164
}
156165
#define arch_atomic_read_acquire arch_atomic_read_acquire
157166
#endif
@@ -160,7 +169,12 @@ arch_atomic_read_acquire(const atomic_t *v)
160169
static __always_inline void
161170
arch_atomic_set_release(atomic_t *v, int i)
162171
{
163-
smp_store_release(&(v)->counter, i);
172+
if (__native_word(atomic_t)) {
173+
smp_store_release(&(v)->counter, i);
174+
} else {
175+
__atomic_release_fence();
176+
arch_atomic_set(v, i);
177+
}
164178
}
165179
#define arch_atomic_set_release arch_atomic_set_release
166180
#endif
@@ -1258,7 +1272,16 @@ arch_atomic_dec_if_positive(atomic_t *v)
12581272
static __always_inline s64
12591273
arch_atomic64_read_acquire(const atomic64_t *v)
12601274
{
1261-
return smp_load_acquire(&(v)->counter);
1275+
s64 ret;
1276+
1277+
if (__native_word(atomic64_t)) {
1278+
ret = smp_load_acquire(&(v)->counter);
1279+
} else {
1280+
ret = arch_atomic64_read(v);
1281+
__atomic_acquire_fence();
1282+
}
1283+
1284+
return ret;
12621285
}
12631286
#define arch_atomic64_read_acquire arch_atomic64_read_acquire
12641287
#endif
@@ -1267,7 +1290,12 @@ arch_atomic64_read_acquire(const atomic64_t *v)
12671290
static __always_inline void
12681291
arch_atomic64_set_release(atomic64_t *v, s64 i)
12691292
{
1270-
smp_store_release(&(v)->counter, i);
1293+
if (__native_word(atomic64_t)) {
1294+
smp_store_release(&(v)->counter, i);
1295+
} else {
1296+
__atomic_release_fence();
1297+
arch_atomic64_set(v, i);
1298+
}
12711299
}
12721300
#define arch_atomic64_set_release arch_atomic64_set_release
12731301
#endif
@@ -2358,4 +2386,4 @@ arch_atomic64_dec_if_positive(atomic64_t *v)
23582386
#endif
23592387

23602388
#endif /* _LINUX_ATOMIC_FALLBACK_H */
2361-
// cca554917d7ea73d5e3e7397dd70c484cad9b2c4
2389+
// 8e2cc06bc0d2c0967d2f8424762bd48555ee40ae

scripts/atomic/fallbacks/read_acquire

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ cat <<EOF
22
static __always_inline ${ret}
33
arch_${atomic}_read_acquire(const ${atomic}_t *v)
44
{
5-
return smp_load_acquire(&(v)->counter);
5+
${int} ret;
6+
7+
if (__native_word(${atomic}_t)) {
8+
ret = smp_load_acquire(&(v)->counter);
9+
} else {
10+
ret = arch_${atomic}_read(v);
11+
__atomic_acquire_fence();
12+
}
13+
14+
return ret;
615
}
716
EOF

scripts/atomic/fallbacks/set_release

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ cat <<EOF
22
static __always_inline void
33
arch_${atomic}_set_release(${atomic}_t *v, ${int} i)
44
{
5-
smp_store_release(&(v)->counter, i);
5+
if (__native_word(${atomic}_t)) {
6+
smp_store_release(&(v)->counter, i);
7+
} else {
8+
__atomic_release_fence();
9+
arch_${atomic}_set(v, i);
10+
}
611
}
712
EOF

0 commit comments

Comments
 (0)