Skip to content

Commit a5460b5

Browse files
committed
READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE()
The implementations of {READ,WRITE}_ONCE() suffer from a significant amount of indirection and complexity due to a historic GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 which was originally worked around by 230fa25 ("kernel: Provide READ_ONCE and ASSIGN_ONCE"). Since GCC 4.8 is fairly vintage at this point and we emit a warning if we detect it during the build, return {READ,WRITE}_ONCE() to their former glory with an implementation that is easier to understand and, crucially, more amenable to optimisation. A side effect of this simplification is that WRITE_ONCE() no longer returns a value, but nobody seems to be relying on that and the new behaviour is aligned with smp_store_release(). Suggested-by: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Michael Ellerman <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Christian Borntraeger <[email protected]> Signed-off-by: Will Deacon <[email protected]>
1 parent c6a771d commit a5460b5

File tree

1 file changed

+39
-79
lines changed

1 file changed

+39
-79
lines changed

include/linux/compiler.h

Lines changed: 39 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -177,60 +177,6 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
177177
# define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__)
178178
#endif
179179

180-
#include <uapi/linux/types.h>
181-
182-
#define __READ_ONCE_SIZE \
183-
({ \
184-
switch (size) { \
185-
case 1: *(__u8 *)res = *(volatile __u8 *)p; break; \
186-
case 2: *(__u16 *)res = *(volatile __u16 *)p; break; \
187-
case 4: *(__u32 *)res = *(volatile __u32 *)p; break; \
188-
case 8: *(__u64 *)res = *(volatile __u64 *)p; break; \
189-
default: \
190-
barrier(); \
191-
__builtin_memcpy((void *)res, (const void *)p, size); \
192-
barrier(); \
193-
} \
194-
})
195-
196-
static __always_inline
197-
void __read_once_size(const volatile void *p, void *res, int size)
198-
{
199-
__READ_ONCE_SIZE;
200-
}
201-
202-
#ifdef CONFIG_KASAN
203-
/*
204-
* We can't declare function 'inline' because __no_sanitize_address confilcts
205-
* with inlining. Attempt to inline it may cause a build failure.
206-
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
207-
* '__maybe_unused' allows us to avoid defined-but-not-used warnings.
208-
*/
209-
# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
210-
#else
211-
# define __no_kasan_or_inline __always_inline
212-
#endif
213-
214-
static __no_kasan_or_inline
215-
void __read_once_size_nocheck(const volatile void *p, void *res, int size)
216-
{
217-
__READ_ONCE_SIZE;
218-
}
219-
220-
static __always_inline void __write_once_size(volatile void *p, void *res, int size)
221-
{
222-
switch (size) {
223-
case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
224-
case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
225-
case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
226-
case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
227-
default:
228-
barrier();
229-
__builtin_memcpy((void *)p, (const void *)res, size);
230-
barrier();
231-
}
232-
}
233-
234180
/*
235181
* Prevent the compiler from merging or refetching reads or writes. The
236182
* compiler is also forbidden from reordering successive instances of
@@ -240,11 +186,7 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
240186
* statements.
241187
*
242188
* These two macros will also work on aggregate data types like structs or
243-
* unions. If the size of the accessed data type exceeds the word size of
244-
* the machine (e.g., 32 bits or 64 bits) READ_ONCE() and WRITE_ONCE() will
245-
* fall back to memcpy(). There's at least two memcpy()s: one for the
246-
* __builtin_memcpy() and then one for the macro doing the copy of variable
247-
* - '__u' allocated on the stack.
189+
* unions.
248190
*
249191
* Their two major use cases are: (1) Mediating communication between
250192
* process-level code and irq/NMI handlers, all running on the same CPU,
@@ -256,23 +198,49 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
256198
#include <asm/barrier.h>
257199
#include <linux/kasan-checks.h>
258200

259-
#define __READ_ONCE(x, check) \
201+
#define __READ_ONCE(x) (*(volatile typeof(x) *)&(x))
202+
203+
#define READ_ONCE(x) \
260204
({ \
261-
union { typeof(x) __val; char __c[1]; } __u; \
262-
if (check) \
263-
__read_once_size(&(x), __u.__c, sizeof(x)); \
264-
else \
265-
__read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
266-
smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
267-
__u.__val; \
205+
typeof(x) __x = __READ_ONCE(x); \
206+
smp_read_barrier_depends(); \
207+
__x; \
268208
})
269-
#define READ_ONCE(x) __READ_ONCE(x, 1)
209+
210+
#define WRITE_ONCE(x, val) \
211+
do { \
212+
*(volatile typeof(x) *)&(x) = (val); \
213+
} while (0)
214+
215+
#ifdef CONFIG_KASAN
216+
/*
217+
* We can't declare function 'inline' because __no_sanitize_address conflicts
218+
* with inlining. Attempt to inline it may cause a build failure.
219+
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
220+
* '__maybe_unused' allows us to avoid defined-but-not-used warnings.
221+
*/
222+
# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
223+
#else
224+
# define __no_kasan_or_inline __always_inline
225+
#endif
226+
227+
static __no_kasan_or_inline
228+
unsigned long __read_once_word_nocheck(const void *addr)
229+
{
230+
return __READ_ONCE(*(unsigned long *)addr);
231+
}
270232

271233
/*
272-
* Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
273-
* to hide memory access from KASAN.
234+
* Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need to load a
235+
* word from memory atomically but without telling KASAN. This is usually
236+
* used by unwinding code when walking the stack of a running process.
274237
*/
275-
#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
238+
#define READ_ONCE_NOCHECK(x) \
239+
({ \
240+
unsigned long __x = __read_once_word_nocheck(&(x)); \
241+
smp_read_barrier_depends(); \
242+
__x; \
243+
})
276244

277245
static __no_kasan_or_inline
278246
unsigned long read_word_at_a_time(const void *addr)
@@ -281,14 +249,6 @@ unsigned long read_word_at_a_time(const void *addr)
281249
return *(unsigned long *)addr;
282250
}
283251

284-
#define WRITE_ONCE(x, val) \
285-
({ \
286-
union { typeof(x) __val; char __c[1]; } __u = \
287-
{ .__val = (__force typeof(x)) (val) }; \
288-
__write_once_size(&(x), __u.__c, sizeof(x)); \
289-
__u.__val; \
290-
})
291-
292252
#endif /* __KERNEL__ */
293253

294254
/*

0 commit comments

Comments
 (0)