From: Will Deacon <will@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: linux-arch@vger.kernel.org, kernel-team@android.com,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Peter Zijlstra <peterz@infradead.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Segher Boessenkool <segher@kernel.crashing.org>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
Arnd Bergmann <arnd@arndb.de>,
Peter Oberparleiter <oberpar@linux.ibm.com>,
Masahiro Yamada <masahiroy@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>
Subject: [PATCH v4 06/11] READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE()
Date: Tue, 21 Apr 2020 16:15:32 +0100 [thread overview]
Message-ID: <20200421151537.19241-7-will@kernel.org> (raw)
In-Reply-To: <20200421151537.19241-1-will@kernel.org>
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 230fa253df63 ("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 <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
include/linux/compiler.h | 118 +++++++++++++--------------------------
1 file changed, 39 insertions(+), 79 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 034b0a644efc..338111a448d0 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -177,60 +177,6 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
# define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__)
#endif
-#include <uapi/linux/types.h>
-
-#define __READ_ONCE_SIZE \
-({ \
- switch (size) { \
- case 1: *(__u8 *)res = *(volatile __u8 *)p; break; \
- case 2: *(__u16 *)res = *(volatile __u16 *)p; break; \
- case 4: *(__u32 *)res = *(volatile __u32 *)p; break; \
- case 8: *(__u64 *)res = *(volatile __u64 *)p; break; \
- default: \
- barrier(); \
- __builtin_memcpy((void *)res, (const void *)p, size); \
- barrier(); \
- } \
-})
-
-static __always_inline
-void __read_once_size(const volatile void *p, void *res, int size)
-{
- __READ_ONCE_SIZE;
-}
-
-#ifdef CONFIG_KASAN
-/*
- * We can't declare function 'inline' because __no_sanitize_address confilcts
- * with inlining. Attempt to inline it may cause a build failure.
- * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
- * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
- */
-# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
-#else
-# define __no_kasan_or_inline __always_inline
-#endif
-
-static __no_kasan_or_inline
-void __read_once_size_nocheck(const volatile void *p, void *res, int size)
-{
- __READ_ONCE_SIZE;
-}
-
-static __always_inline void __write_once_size(volatile void *p, void *res, int size)
-{
- switch (size) {
- case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
- case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
- case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
- case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
- default:
- barrier();
- __builtin_memcpy((void *)p, (const void *)res, size);
- barrier();
- }
-}
-
/*
* Prevent the compiler from merging or refetching reads or writes. The
* 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
* statements.
*
* These two macros will also work on aggregate data types like structs or
- * unions. If the size of the accessed data type exceeds the word size of
- * the machine (e.g., 32 bits or 64 bits) READ_ONCE() and WRITE_ONCE() will
- * fall back to memcpy(). There's at least two memcpy()s: one for the
- * __builtin_memcpy() and then one for the macro doing the copy of variable
- * - '__u' allocated on the stack.
+ * unions.
*
* Their two major use cases are: (1) Mediating communication between
* 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
#include <asm/barrier.h>
#include <linux/kasan-checks.h>
-#define __READ_ONCE(x, check) \
+#define __READ_ONCE(x) (*(volatile typeof(x) *)&(x))
+
+#define READ_ONCE(x) \
({ \
- union { typeof(x) __val; char __c[1]; } __u; \
- if (check) \
- __read_once_size(&(x), __u.__c, sizeof(x)); \
- else \
- __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
- smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
- __u.__val; \
+ typeof(x) __x = __READ_ONCE(x); \
+ smp_read_barrier_depends(); \
+ __x; \
})
-#define READ_ONCE(x) __READ_ONCE(x, 1)
+
+#define WRITE_ONCE(x, val) \
+do { \
+ *(volatile typeof(x) *)&(x) = (val); \
+} while (0)
+
+#ifdef CONFIG_KASAN
+/*
+ * We can't declare function 'inline' because __no_sanitize_address conflicts
+ * with inlining. Attempt to inline it may cause a build failure.
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
+ */
+# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
+#else
+# define __no_kasan_or_inline __always_inline
+#endif
+
+static __no_kasan_or_inline
+unsigned long __read_once_word_nocheck(const void *addr)
+{
+ return __READ_ONCE(*(unsigned long *)addr);
+}
/*
- * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
- * to hide memory access from KASAN.
+ * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need to load a
+ * word from memory atomically but without telling KASAN. This is usually
+ * used by unwinding code when walking the stack of a running process.
*/
-#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
+#define READ_ONCE_NOCHECK(x) \
+({ \
+ unsigned long __x = __read_once_word_nocheck(&(x)); \
+ smp_read_barrier_depends(); \
+ __x; \
+})
static __no_kasan_or_inline
unsigned long read_word_at_a_time(const void *addr)
@@ -281,14 +249,6 @@ unsigned long read_word_at_a_time(const void *addr)
return *(unsigned long *)addr;
}
-#define WRITE_ONCE(x, val) \
-({ \
- union { typeof(x) __val; char __c[1]; } __u = \
- { .__val = (__force typeof(x)) (val) }; \
- __write_once_size(&(x), __u.__c, sizeof(x)); \
- __u.__val; \
-})
-
#endif /* __KERNEL__ */
/*
--
2.26.1.301.g55bc3eb7cb9-goog
next prev parent reply other threads:[~2020-04-21 15:15 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-21 15:15 [PATCH v4 00/11] Rework READ_ONCE() to improve codegen Will Deacon
2020-04-21 15:15 ` [PATCH v4 01/11] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8 Will Deacon
2020-04-21 17:15 ` Masahiro Yamada
2020-04-21 15:15 ` [PATCH v4 02/11] netfilter: Avoid assigning 'const' pointer to non-const pointer Will Deacon
2020-04-21 15:15 ` [PATCH v4 03/11] net: tls: " Will Deacon
2020-04-21 15:15 ` [PATCH v4 04/11] fault_inject: Don't rely on "return value" from WRITE_ONCE() Will Deacon
2020-04-21 15:15 ` [PATCH v4 05/11] arm64: csum: Disable KASAN for do_csum() Will Deacon
2020-04-22 9:49 ` Mark Rutland
2020-04-22 10:41 ` Will Deacon
2020-04-22 11:01 ` Robin Murphy
2020-04-24 9:41 ` David Laight
2020-04-24 11:00 ` Robin Murphy
2020-04-24 13:04 ` David Laight
2020-04-21 15:15 ` Will Deacon [this message]
2020-04-22 9:51 ` [PATCH v4 06/11] READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE() Mark Rutland
2020-04-21 15:15 ` [PATCH v4 07/11] READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses Will Deacon
2020-04-24 16:31 ` Jann Horn
2020-04-24 17:11 ` Will Deacon
2020-04-24 17:43 ` Peter Zijlstra
2020-04-21 15:15 ` [PATCH v4 08/11] READ_ONCE: Drop pointer qualifiers when reading from scalar types Will Deacon
2020-04-22 10:25 ` Rasmus Villemoes
2020-04-22 11:48 ` Segher Boessenkool
2020-04-22 13:11 ` Will Deacon
2020-04-22 14:54 ` Will Deacon
2020-04-21 15:15 ` [PATCH v4 09/11] locking/barriers: Use '__unqual_scalar_typeof' for load-acquire macros Will Deacon
2020-04-21 15:15 ` [PATCH v4 10/11] arm64: barrier: Use '__unqual_scalar_typeof' for acquire/release macros Will Deacon
2020-04-21 15:15 ` [PATCH v4 11/11] gcov: Remove old GCC 3.4 support Will Deacon
2020-04-21 17:19 ` Masahiro Yamada
2020-04-21 18:42 ` [PATCH v4 00/11] Rework READ_ONCE() to improve codegen Linus Torvalds
2020-04-22 8:18 ` Will Deacon
2020-04-22 11:37 ` Peter Zijlstra
2020-04-22 12:26 ` Will Deacon
2020-04-24 13:42 ` Will Deacon
2020-04-24 15:54 ` Marco Elver
2020-04-24 16:52 ` Will Deacon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200421151537.19241-7-will@kernel.org \
--to=will@kernel.org \
--cc=arnd@arndb.de \
--cc=borntraeger@de.ibm.com \
--cc=kernel-team@android.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.vanoostenryck@gmail.com \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=ndesaulniers@google.com \
--cc=oberpar@linux.ibm.com \
--cc=peterz@infradead.org \
--cc=segher@kernel.crashing.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.