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>,
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 v3 06/12] READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE()
Date: Wed, 15 Apr 2020 17:52:12 +0100 [thread overview]
Message-ID: <20200415165218.20251-7-will@kernel.org> (raw)
In-Reply-To: <20200415165218.20251-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 | 104 ++++++++++-----------------------------
1 file changed, 25 insertions(+), 79 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 034b0a644efc..0bf6c04b3f8f 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,35 @@ 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) \
+/*
+ * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
+ * to hide memory access from KASAN.
+ */
+#define READ_ONCE_NOCHECK(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 = *(volatile typeof(x) *)&(x); \
+ smp_read_barrier_depends(); \
+ __x; \
})
-#define READ_ONCE(x) __READ_ONCE(x, 1)
+#define READ_ONCE(x) READ_ONCE_NOCHECK(x)
+
+#define WRITE_ONCE(x, val) \
+do { \
+ *(volatile typeof(x) *)&(x) = (val); \
+} while (0)
+
+#ifdef CONFIG_KASAN
/*
- * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
- * to hide memory access from 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 READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
+# 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_word_at_a_time(const void *addr)
@@ -281,14 +235,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.0.110.g2183baf09c-goog
next prev parent reply other threads:[~2020-04-15 16:53 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-15 16:52 [PATCH v3 00/12] Rework READ_ONCE() to improve codegen Will Deacon
2020-04-15 16:52 ` [PATCH v3 01/12] compiler/gcc: Emit build-time warning for GCC prior to version 4.8 Will Deacon
2020-04-15 17:20 ` Masahiro Yamada
2020-04-15 16:52 ` [PATCH v3 02/12] netfilter: Avoid assigning 'const' pointer to non-const pointer Will Deacon
2020-04-15 16:52 ` [PATCH v3 03/12] net: tls: " Will Deacon
2020-04-15 16:52 ` [PATCH v3 04/12] fault_inject: Don't rely on "return value" from WRITE_ONCE() Will Deacon
2020-04-15 16:52 ` [PATCH v3 05/12] arm64: csum: Disable KASAN for do_csum() Will Deacon
2020-04-15 17:28 ` Mark Rutland
2020-04-15 18:42 ` Arnd Bergmann
2020-04-15 19:43 ` Will Deacon
2020-04-15 20:10 ` Will Deacon
2020-04-15 19:26 ` Will Deacon
2020-04-16 9:31 ` Mark Rutland
2020-04-16 11:53 ` Will Deacon
2020-04-16 12:11 ` Mark Rutland
2020-04-15 19:26 ` Robin Murphy
2020-04-15 16:52 ` Will Deacon [this message]
2020-04-15 16:52 ` [PATCH v3 07/12] READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses Will Deacon
2020-04-15 16:52 ` [PATCH v3 08/12] READ_ONCE: Drop pointer qualifiers when reading from scalar types Will Deacon
2020-04-15 16:52 ` [PATCH v3 09/12] locking/barriers: Use '__unqual_scalar_typeof' for load-acquire macros Will Deacon
2020-04-15 16:52 ` [PATCH v3 10/12] arm64: barrier: Use '__unqual_scalar_typeof' for acquire/release macros Will Deacon
2020-04-15 16:52 ` [PATCH v3 11/12] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8 Will Deacon
2020-04-15 18:37 ` Arnd Bergmann
2020-04-15 16:52 ` [PATCH v3 12/12] gcov: Remove old GCC 3.4 support Will Deacon
2020-04-16 12:30 ` [PATCH v3 00/12] Rework READ_ONCE() to improve codegen Christian Borntraeger
2020-04-16 12:48 ` 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=20200415165218.20251-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=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.