[PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Will Deacon]

Hi folks,

This is version two of the patches I previously posted as an RFC here:

https://lore.kernel.org/lkml/20200110165636.28035-1-will@kernel.org

Changes since then include:

  * Adopted a less vomit-inducing series of macros for __unqual_scalar_typeof

  * Cast to 'const' in READ_ONCE() to prevent assignment to the resulting
    expression

  * Only warn once at build-time if GCC prior to 4.8 is detected...

  * ... and then raise the minimum GCC version to 4.8, with an error for
    older versions of the compiler

  * Remove some dead gcov code so that the resulting diffstat can distract
    from those less vomit-inducing macros I mentioned earlier on

Failing the build for older compilers is always a contentious topic, so
I've done that as a separate couple of patches on the end in case we end
up dropping or reverting them.

Cheers,

Will

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>

--->8

Will Deacon (10):
  compiler/gcc: Emit build-time warning for GCC prior to version 4.8
  netfilter: Avoid assigning 'const' pointer to non-const pointer
  fault_inject: Don't rely on "return value" from WRITE_ONCE()
  READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE()
  READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses
  READ_ONCE: Drop pointer qualifiers when reading from scalar types
  locking/barriers: Use '__unqual_scalar_typeof' for load-acquire macros
  arm64: barrier: Use '__unqual_scalar_typeof' for acquire/release
    macros
  compiler/gcc: Raise minimum GCC version for kernel builds to 4.8
  gcov: Remove old GCC 3.4 support

 Documentation/process/changes.rst |   2 +-
 arch/arm/crypto/Kconfig           |  12 +-
 arch/arm64/include/asm/barrier.h  |  16 +-
 crypto/Kconfig                    |   1 -
 drivers/xen/time.c                |   2 +-
 include/asm-generic/barrier.h     |  16 +-
 include/linux/compiler-gcc.h      |   5 +-
 include/linux/compiler.h          | 129 +++----
 include/linux/compiler_types.h    |  21 ++
 init/Kconfig                      |   5 +-
 kernel/gcov/Kconfig               |  24 --
 kernel/gcov/Makefile              |   3 +-
 kernel/gcov/gcc_3_4.c             | 573 ------------------------------
 lib/fault-inject.c                |   4 +-
 net/netfilter/core.c              |   2 +-
 net/xdp/xsk_queue.h               |   2 +-
 scripts/Kconfig.include           |   3 +
 scripts/gcc-plugins/Kconfig       |   4 +-
 18 files changed, 111 insertions(+), 713 deletions(-)
 delete mode 100644 kernel/gcov/gcc_3_4.c

-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 01/10] compiler/gcc: Emit build-time warning for GCC prior to version 4.8

[Will Deacon]

Prior to version 4.8, GCC may miscompile READ_ONCE() by erroneously
discarding the 'volatile' qualifier:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

We've been working around this using some nasty hacks which make
READ_ONCE() both horribly complicated and also prevent us from enforcing
that it is only used on scalar types. Since GCC 4.8 is pretty old for
kernel builds now, emit a warning if we detect it during the build.

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>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 init/Kconfig            | 4 ++--
 scripts/Kconfig.include | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index a34064a031a5..bdc2f1b1667b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -10,11 +10,11 @@ config DEFCONFIG_LIST
 	default "arch/$(ARCH)/defconfig"
 
 config CC_IS_GCC
-	def_bool $(success,$(CC) --version | head -n 1 | grep -q gcc)
+	def_bool $(cc-is-gcc)
 
 config GCC_VERSION
 	int
-	default $(shell,$(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC
+	default $(gcc-version) if CC_IS_GCC
 	default 0
 
 config CC_IS_CLANG
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index d4adfbe42690..4e645a798b56 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -40,3 +40,9 @@ $(error-if,$(success, $(LD) -v | grep -q gold), gold linker '$(LD)' not supporte
 
 # gcc version including patch level
 gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
+
+# Return y if the compiler is GCC, n otherwise
+cc-is-gcc := $(success,$(CC) --version | head -n 1 | grep -q gcc)
+
+# Warn if the compiler is GCC prior to 4.8
+$(warning-if,$(if-success,[ $(gcc-version) -lt 40800 ],$(cc-is-gcc),n),"Your compiler is old and may miscompile the kernel due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 - please upgrade it.")
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Will Deacon]

nf_remove_net_hook() uses WRITE_ONCE() to assign a 'const pointer to a
'non-const' pointer. Cleanups to the implementation of WRITE_ONCE() mean
that this will give rise to a compiler warning, just like a plain old
assignment would do:

  | In file included from ./include/linux/export.h:43,
  |                  from ./include/linux/linkage.h:7,
  |                  from ./include/linux/kernel.h:8,
  |                  from net/netfilter/core.c:9:
  | net/netfilter/core.c: In function ‘nf_remove_net_hook’:
  | ./include/linux/compiler.h:216:30: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  |   *(volatile typeof(x) *)&(x) = (val);  \
  |                               ^
  | net/netfilter/core.c:379:3: note: in expansion of macro ‘WRITE_ONCE’
  |    WRITE_ONCE(orig_ops[i], &dummy_ops);
  |    ^~~~~~~~~~

Follow the pattern used elsewhere in this file and add a cast to 'void *'
to squash the warning.

Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Will Deacon <will@kernel.org>
---
 net/netfilter/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 78f046ec506f..3ac7c8c1548d 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -376,7 +376,7 @@ static bool nf_remove_net_hook(struct nf_hook_entries *old,
 		if (orig_ops[i] != unreg)
 			continue;
 		WRITE_ONCE(old->hooks[i].hook, accept_all);
-		WRITE_ONCE(orig_ops[i], &dummy_ops);
+		WRITE_ONCE(orig_ops[i], (void *)&dummy_ops);
 		return true;
 	}
 
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Nick Desaulniers]

On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
>
> nf_remove_net_hook() uses WRITE_ONCE() to assign a 'const pointer to a
> 'non-const' pointer. Cleanups to the implementation of WRITE_ONCE() mean
> that this will give rise to a compiler warning, just like a plain old
> assignment would do:
>
>   | In file included from ./include/linux/export.h:43,
>   |                  from ./include/linux/linkage.h:7,
>   |                  from ./include/linux/kernel.h:8,
>   |                  from net/netfilter/core.c:9:
>   | net/netfilter/core.c: In function ‘nf_remove_net_hook’:
>   | ./include/linux/compiler.h:216:30: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>   |   *(volatile typeof(x) *)&(x) = (val);  \
>   |                               ^
>   | net/netfilter/core.c:379:3: note: in expansion of macro ‘WRITE_ONCE’
>   |    WRITE_ONCE(orig_ops[i], &dummy_ops);
>   |    ^~~~~~~~~~
>
> Follow the pattern used elsewhere in this file and add a cast to 'void *'
> to squash the warning.
>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  net/netfilter/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 78f046ec506f..3ac7c8c1548d 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -376,7 +376,7 @@ static bool nf_remove_net_hook(struct nf_hook_entries *old,
>                 if (orig_ops[i] != unreg)
>                         continue;
>                 WRITE_ONCE(old->hooks[i].hook, accept_all);
> -               WRITE_ONCE(orig_ops[i], &dummy_ops);
> +               WRITE_ONCE(orig_ops[i], (void *)&dummy_ops);

Good thing it's the variable being modified was not declared const; I
get spooked when I see -Wdiscarded-qualifiers because of Section
6.7.3.6 of the ISO C11 draft spec:

```
If an attempt is made to modify an object defined with a
const-qualified type through use
of an lvalue with non-const-qualified type, the behavior is undefined.
If an attempt is
made to refer to an object defined with a volatile-qualified type
through use of an lvalue
with non-volatile-qualified type, the behavior is undefined.133)

133) This applies to those objects that behave as if they were defined
with qualified types, even if they are
never actually defined as objects in the program (such as an object at
a memory-mapped input/output
address).
```

Which is about the modification of a const-declared variable (explicit
UB which Clang actively exploits), and doesn't apply in this case.  I
agree that this is the best way to fix this due to the use of typeof()
and it's semantics of dropping qualifiers; declaring `dummy_ops` as
non-const would be another, but that is worse IMO.  Thanks for the
patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>                 return true;
>         }
>
> --
> 2.25.0.341.g760bfbb309-goog
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Peter Zijlstra]

On Thu, Jan 23, 2020 at 11:07:59AM -0800, Nick Desaulniers wrote:

> Good thing it's the variable being modified was not declared const; I
> get spooked when I see -Wdiscarded-qualifiers because of Section
> 6.7.3.6 of the ISO C11 draft spec:
> 
> ```
> If an attempt is made to modify an object defined with a
> const-qualified type through use
> of an lvalue with non-const-qualified type, the behavior is undefined.
> If an attempt is
> made to refer to an object defined with a volatile-qualified type
> through use of an lvalue
> with non-volatile-qualified type, the behavior is undefined.133)
> 
> 133) This applies to those objects that behave as if they were defined
> with qualified types, even if they are
> never actually defined as objects in the program (such as an object at
> a memory-mapped input/output
> address).
> ```
> 
> Which is about the modification of a const-declared variable (explicit
> UB which Clang actively exploits), 

Just for curiosity's sake. What does clang actually do in that case?

Re: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Nick Desaulniers]

On Fri, Jan 24, 2020 at 12:24 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jan 23, 2020 at 11:07:59AM -0800, Nick Desaulniers wrote:
>
> > Good thing it's the variable being modified was not declared const; I
> > get spooked when I see -Wdiscarded-qualifiers because of Section
> > 6.7.3.6 of the ISO C11 draft spec:
> >
> > ```
> > If an attempt is made to modify an object defined with a
> > const-qualified type through use
> > of an lvalue with non-const-qualified type, the behavior is undefined.
> > If an attempt is
> > made to refer to an object defined with a volatile-qualified type
> > through use of an lvalue
> > with non-volatile-qualified type, the behavior is undefined.133)
> >
> > 133) This applies to those objects that behave as if they were defined
> > with qualified types, even if they are
> > never actually defined as objects in the program (such as an object at
> > a memory-mapped input/output
> > address).
> > ```
> >
> > Which is about the modification of a const-declared variable (explicit
> > UB which Clang actively exploits),
>
> Just for curiosity's sake. What does clang actually do in that case?
>

https://bugs.llvm.org/show_bug.cgi?id=42763
I was playing around with this in godbolt but couldn't quickly come up
with a simple reproducer.  IIRC, I've fixed maybe 3 instances of this
recently in code though.
-- 
Thanks,
~Nick Desaulniers

RE: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[David Laight]

From: Nick Desaulniers
> Sent: 24 January 2020 17:20
...
> > > Good thing it's the variable being modified was not declared const; I
> > > get spooked when I see -Wdiscarded-qualifiers because of Section
> > > 6.7.3.6 of the ISO C11 draft spec:
> > >
> > > ```
> > > If an attempt is made to modify an object defined with a const-qualified
> > > type through use of an lvalue with non-const-qualified type,
> > > the behavior is undefined.

Well some old systems had small integer constants at fixes addresses.
So 'const int one = 1;'  would be a reference to the global constant.
An assignment like '*(int *)&one = 2;' would change the value of the
system-wide 'one' constant'.

Pretty much 'undefined'.

But no excuse for the compiler just discarding the code.

I suspect that the code to remove 'const' needs to 'launder' the value
through a suitable integer type.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Linus Torvalds]

On Fri, Jan 24, 2020 at 12:25 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Just for curiosity's sake. What does clang actually do in that case?

This shouldn't necessarily be clang-specific. If the variable itself
is 'const', it might go into a read-only section. So trying to modify
it will quite possibly hit a SIGSEGV in user space (and in kernel
space cause an oops).

Note that that is different from a const pointer to something that
wasn't originally const. That's just a "error out at compile time if
somebody tries to write through it", but the const'ness can be cast
away, because all the 'const' really said was that the object can't be
modified through _that_ pointer, not in general.

(That also means that the compiler can't necessarily even optimize
multiple accesses through a const pointer away, because the object
might be modified through another pointer that aliases the const one -
you'd need to also mark it "restrict" to tell the compiler that no
other pointer will alias).

                 Linus

Re: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[Peter Zijlstra]

On Fri, Jan 24, 2020 at 09:36:54AM -0800, Linus Torvalds wrote:
> On Fri, Jan 24, 2020 at 12:25 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Just for curiosity's sake. What does clang actually do in that case?
> 
> This shouldn't necessarily be clang-specific. If the variable itself
> is 'const', it might go into a read-only section. So trying to modify
> it will quite possibly hit a SIGSEGV in user space (and in kernel
> space cause an oops).

Quite; but I worried clang would use the UB to omit the access entirely,
and therefore rob us of the well deserved crash or something.

Let me go read Nick's email tho, see what it actually does.

RE: [PATCH v2 02/10] netfilter: Avoid assigning 'const' pointer to non-const pointer

[David Laight]

From: Linus Torvalds
> Sent: 24 January 2020 17:37
...
> (That also means that the compiler can't necessarily even optimize
> multiple accesses through a const pointer away, because the object
> might be modified through another pointer that aliases the const one -
> you'd need to also mark it "restrict" to tell the compiler that no
> other pointer will alias).

I've seen gcc cache a value read through a 'const' parameter pointer
across a function call.
Can't remember which version though.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[PATCH v2 03/10] fault_inject: Don't rely on "return value" from WRITE_ONCE()

[Will Deacon]

It's a bit weird that WRITE_ONCE() evaluates to the value it stores and
it's different to smp_store_release(), which can't be used this way.

In preparation for preventing this in WRITE_ONCE(), change the fault
injection code to use a local variable instead.

Cc: Akinobu Mita <akinobu.mita@gmail.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 lib/fault-inject.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/fault-inject.c b/lib/fault-inject.c
index 8186ca84910b..ce12621b4275 100644
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -106,7 +106,9 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
 		unsigned int fail_nth = READ_ONCE(current->fail_nth);
 
 		if (fail_nth) {
-			if (!WRITE_ONCE(current->fail_nth, fail_nth - 1))
+			fail_nth--;
+			WRITE_ONCE(current->fail_nth, fail_nth);
+			if (!fail_nth)
 				goto fail;
 
 			return false;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 04/10] READ_ONCE: Simplify implementations of {READ,WRITE}_ONCE()

[Will Deacon]

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 5e88e7e33abe..44974d658f30 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.25.0.341.g760bfbb309-goog

[PATCH v2 05/10] READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses

[Will Deacon]

{READ,WRITE}_ONCE() cannot guarantee atomicity for arbitrary data sizes.
This can be surprising to callers that might incorrectly be expecting
atomicity for accesses to aggregate structures, although there are other
callers where tearing is actually permissable (e.g. if they are using
something akin to sequence locking to protect the access).

Linus sayeth:

  | We could also look at being stricter for the normal READ/WRITE_ONCE(),
  | and require that they are
  |
  | (a) regular integer types
  |
  | (b) fit in an atomic word
  |
  | We actually did (b) for a while, until we noticed that we do it on
  | loff_t's etc and relaxed the rules. But maybe we could have a
  | "non-atomic" version of READ/WRITE_ONCE() that is used for the
  | questionable cases?

The slight snag is that we also have to support 64-bit accesses on 32-bit
architectures, as these appear to be widespread and tend to work out ok
if either the architecture supports atomic 64-bit accesses (x86, armv7)
or if the variable being accesses represents a virtual address and
therefore only requires 32-bit atomicity in practice.

Take a step in that direction by introducing a variant of
'compiletime_assert_atomic_type()' and use it to check the pointer
argument to {READ,WRITE}_ONCE(). Expose __{READ,WRITE_ONCE}() variants
which are allowed to tear and convert the two broken callers over to the
new macros.

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>
Signed-off-by: Will Deacon <will@kernel.org>
---
 drivers/xen/time.c       |  2 +-
 include/linux/compiler.h | 37 +++++++++++++++++++++++++++++++++----
 net/xdp/xsk_queue.h      |  2 +-
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/time.c b/drivers/xen/time.c
index 0968859c29d0..108edbcbc040 100644
--- a/drivers/xen/time.c
+++ b/drivers/xen/time.c
@@ -64,7 +64,7 @@ static void xen_get_runstate_snapshot_cpu_delta(
 	do {
 		state_time = get64(&state->state_entry_time);
 		rmb();	/* Hypervisor might update data. */
-		*res = READ_ONCE(*state);
+		*res = __READ_ONCE(*state);
 		rmb();	/* Hypervisor might update data. */
 	} while (get64(&state->state_entry_time) != state_time ||
 		 (state_time & XEN_RUNSTATE_UPDATE));
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 44974d658f30..a7b2195f2655 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -198,24 +198,43 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 #include <asm/barrier.h>
 #include <linux/kasan-checks.h>
 
+/*
+ * Use __READ_ONCE() instead of READ_ONCE() if you do not require any
+ * atomicity or dependency ordering guarantees. Note that this may result
+ * in tears!
+ */
+#define __READ_ONCE(x)	(*(const volatile typeof(x) *)&(x))
+
+#define __READ_ONCE_SCALAR(x)						\
+({									\
+	typeof(x) __x = __READ_ONCE(x);					\
+	smp_read_barrier_depends();					\
+	__x;								\
+})
+
 /*
  * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
  * to hide memory access from KASAN.
  */
 #define READ_ONCE_NOCHECK(x)						\
 ({									\
-	typeof(x) __x = *(volatile typeof(x) *)&(x);			\
-	smp_read_barrier_depends();					\
-	__x;								\
+	compiletime_assert_rwonce_type(x);				\
+	__READ_ONCE_SCALAR(x);						\
 })
 
 #define READ_ONCE(x)	READ_ONCE_NOCHECK(x)
 
-#define WRITE_ONCE(x, val)				\
+#define __WRITE_ONCE(x, val)				\
 do {							\
 	*(volatile typeof(x) *)&(x) = (val);		\
 } while (0)
 
+#define WRITE_ONCE(x, val)				\
+do {							\
+	compiletime_assert_rwonce_type(x);		\
+	__WRITE_ONCE(x, val);				\
+} while (0)
+
 #ifdef CONFIG_KASAN
 /*
  * We can't declare function 'inline' because __no_sanitize_address conflicts
@@ -299,6 +318,16 @@ static inline void *offset_to_ptr(const int *off)
 	compiletime_assert(__native_word(t),				\
 		"Need native word sized stores/loads for atomicity.")
 
+/*
+ * Yes, this permits 64-bit accesses on 32-bit architectures. These will
+ * actually be atomic in many cases (namely x86), but for others we rely on
+ * the access being split into 2x32-bit accesses for a 32-bit quantity (e.g.
+ * a virtual address) and a strong prevailing wind.
+ */
+#define compiletime_assert_rwonce_type(t)					\
+	compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long),	\
+		"Unsupported access size for {READ,WRITE}_ONCE().")
+
 /* &a[0] degrades to a pointer: a different type from an array */
 #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
 
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index eddae4688862..2b55c1c7b2b6 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -304,7 +304,7 @@ static inline struct xdp_desc *xskq_validate_desc(struct xsk_queue *q,
 		struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
 		unsigned int idx = q->cons_tail & q->ring_mask;
 
-		*desc = READ_ONCE(ring->desc[idx]);
+		*desc = __READ_ONCE(ring->desc[idx]);
 		if (xskq_is_valid_desc(q, desc, umem))
 			return desc;
 
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v2 05/10] READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses

[Peter Zijlstra]

On Thu, Jan 23, 2020 at 03:33:36PM +0000, Will Deacon wrote:
> {READ,WRITE}_ONCE() cannot guarantee atomicity for arbitrary data sizes.
> This can be surprising to callers that might incorrectly be expecting
> atomicity for accesses to aggregate structures, although there are other
> callers where tearing is actually permissable (e.g. if they are using
> something akin to sequence locking to protect the access).
> 
> Linus sayeth:
> 
>   | We could also look at being stricter for the normal READ/WRITE_ONCE(),
>   | and require that they are
>   |
>   | (a) regular integer types
>   |
>   | (b) fit in an atomic word
>   |
>   | We actually did (b) for a while, until we noticed that we do it on
>   | loff_t's etc and relaxed the rules. But maybe we could have a
>   | "non-atomic" version of READ/WRITE_ONCE() that is used for the
>   | questionable cases?
> 
> The slight snag is that we also have to support 64-bit accesses on 32-bit
> architectures, as these appear to be widespread and tend to work out ok
> if either the architecture supports atomic 64-bit accesses (x86, armv7)
> or if the variable being accesses represents a virtual address and
> therefore only requires 32-bit atomicity in practice.
> 
> Take a step in that direction by introducing a variant of
> 'compiletime_assert_atomic_type()' and use it to check the pointer
> argument to {READ,WRITE}_ONCE(). Expose __{READ,WRITE_ONCE}() variants
> which are allowed to tear and convert the two broken callers over to the
> new macros.

The build robot is telling me we also need this for m68k; they have:

  arch/m68k/include/asm/page.h:typedef struct { unsigned long pmd[16]; } pmd_t;

Commit 688272809fcce seems to suggest the below is actually wrong tho.

---
diff --git a/mm/gup.c b/mm/gup.c
index 7646bf993b25..62885dad5444 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -320,7 +320,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma,
 	 * The READ_ONCE() will stabilize the pmdval in a register or
 	 * on the stack so that it will stop changing under the code.
 	 */
-	pmdval = READ_ONCE(*pmd);
+	pmdval = __READ_ONCE(*pmd);
 	if (pmd_none(pmdval))
 		return no_page_table(vma, flags);
 	if (pmd_huge(pmdval) && vma->vm_flags & VM_HUGETLB) {
@@ -345,7 +345,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma,
 				  !is_pmd_migration_entry(pmdval));
 		if (is_pmd_migration_entry(pmdval))
 			pmd_migration_entry_wait(mm, pmd);
-		pmdval = READ_ONCE(*pmd);
+		pmdval = __READ_ONCE(*pmd);
 		/*
 		 * MADV_DONTNEED may convert the pmd to null because
 		 * mmap_sem is held in read mode

Re: [PATCH v2 05/10] READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses

[Peter Zijlstra]

On Sat, Jan 25, 2020 at 09:27:46AM +0100, Peter Zijlstra wrote:
> On Thu, Jan 23, 2020 at 03:33:36PM +0000, Will Deacon wrote:
> > {READ,WRITE}_ONCE() cannot guarantee atomicity for arbitrary data sizes.
> > This can be surprising to callers that might incorrectly be expecting
> > atomicity for accesses to aggregate structures, although there are other
> > callers where tearing is actually permissable (e.g. if they are using
> > something akin to sequence locking to protect the access).
> > 
> > Linus sayeth:
> > 
> >   | We could also look at being stricter for the normal READ/WRITE_ONCE(),
> >   | and require that they are
> >   |
> >   | (a) regular integer types
> >   |
> >   | (b) fit in an atomic word
> >   |
> >   | We actually did (b) for a while, until we noticed that we do it on
> >   | loff_t's etc and relaxed the rules. But maybe we could have a
> >   | "non-atomic" version of READ/WRITE_ONCE() that is used for the
> >   | questionable cases?
> > 
> > The slight snag is that we also have to support 64-bit accesses on 32-bit
> > architectures, as these appear to be widespread and tend to work out ok
> > if either the architecture supports atomic 64-bit accesses (x86, armv7)
> > or if the variable being accesses represents a virtual address and
> > therefore only requires 32-bit atomicity in practice.
> > 
> > Take a step in that direction by introducing a variant of
> > 'compiletime_assert_atomic_type()' and use it to check the pointer
> > argument to {READ,WRITE}_ONCE(). Expose __{READ,WRITE_ONCE}() variants
> > which are allowed to tear and convert the two broken callers over to the
> > new macros.
> 
> The build robot is telling me we also need this for m68k; they have:
> 
>   arch/m68k/include/asm/page.h:typedef struct { unsigned long pmd[16]; } pmd_t;

Fixed that with these patches:

  https://lkml.kernel.org/r/20200129103941.304769381@infradead.org

[PATCH v2 06/10] READ_ONCE: Drop pointer qualifiers when reading from scalar types

[Will Deacon]

Passing a volatile-qualified pointer to READ_ONCE() is an absolute
trainwreck for code generation: the use of 'typeof()' to define a
temporary variable inside the macro means that the final evaluation in
macro scope ends up forcing a read back from the stack. When stack
protector is enabled (the default for arm64, at least), this causes
the compiler to vomit up all sorts of junk.

Unfortunately, dropping pointer qualifiers inside the macro poses quite
a challenge, especially since the pointed-to type is permitted to be an
aggregate, and this is relied upon by mm/ code accessing things like
'pmd_t'. Based on numerous hacks and discussions on the mailing list,
this is the best I've managed to come up with.

Introduce '__unqual_scalar_typeof()' which takes an expression and, if
the expression is an optionally qualified 8, 16, 32 or 64-bit scalar
type, evaluates to the unqualified type. Other input types, including
aggregates, remain unchanged. Hopefully READ_ONCE() on volatile aggregate
pointers isn't something we do on a fast-path.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Will Deacon <will@kernel.org>
---
 include/linux/compiler.h       |  6 +++---
 include/linux/compiler_types.h | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a7b2195f2655..994c35638584 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -203,13 +203,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
  * atomicity or dependency ordering guarantees. Note that this may result
  * in tears!
  */
-#define __READ_ONCE(x)	(*(const volatile typeof(x) *)&(x))
+#define __READ_ONCE(x)	(*(const volatile __unqual_scalar_typeof(x) *)&(x))
 
 #define __READ_ONCE_SCALAR(x)						\
 ({									\
-	typeof(x) __x = __READ_ONCE(x);					\
+	__unqual_scalar_typeof(x) __x = __READ_ONCE(x);			\
 	smp_read_barrier_depends();					\
-	__x;								\
+	(typeof(x))__x;							\
 })
 
 /*
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 72393a8c1a6c..58361f2d3b98 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -219,6 +219,27 @@ struct ftrace_likely_data {
 /* Are two types/vars the same type (ignoring qualifiers)? */
 #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
 
+/*
+ * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving
+ *			       non-scalar types unchanged.
+ *
+ * We build this out of a couple of helper macros in a vain attempt to
+ * help you keep your lunch down while reading it.
+ */
+#define __pick_scalar_type(x, type, otherwise)					\
+	__builtin_choose_expr(__same_type(x, type), (type)0, otherwise)
+
+#define __pick_integer_type(x, type, otherwise)					\
+	__pick_scalar_type(x, unsigned type,					\
+		__pick_scalar_type(x, signed type, otherwise))
+
+#define __unqual_scalar_typeof(x) typeof(					\
+	__pick_integer_type(x, char,						\
+		__pick_integer_type(x, short,					\
+			__pick_integer_type(x, int,				\
+				__pick_integer_type(x, long,			\
+					__pick_integer_type(x, long long, x))))))
+
 /* Is this type a native word size -- useful for atomic operations */
 #define __native_word(t) \
 	(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 07/10] locking/barriers: Use '__unqual_scalar_typeof' for load-acquire macros

[Will Deacon]

Passing volatile-qualified pointers to the asm-generic implementations of
the load-acquire macros results in a re-load from the stack due to the
temporary result variable inheriting the volatile semantics thanks to the
use of 'typeof()'.

Define these temporary variables using 'unqual_scalar_typeof' to drop
the volatile qualifier in the case that they are scalar types.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Will Deacon <will@kernel.org>
---
 include/asm-generic/barrier.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 85b28eb80b11..2eacaf7d62f6 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -128,10 +128,10 @@ do {									\
 #ifndef __smp_load_acquire
 #define __smp_load_acquire(p)						\
 ({									\
-	typeof(*p) ___p1 = READ_ONCE(*p);				\
+	__unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p);		\
 	compiletime_assert_atomic_type(*p);				\
 	__smp_mb();							\
-	___p1;								\
+	(typeof(*p))___p1;						\
 })
 #endif
 
@@ -183,10 +183,10 @@ do {									\
 #ifndef smp_load_acquire
 #define smp_load_acquire(p)						\
 ({									\
-	typeof(*p) ___p1 = READ_ONCE(*p);				\
+	__unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p);		\
 	compiletime_assert_atomic_type(*p);				\
 	barrier();							\
-	___p1;								\
+	(typeof(*p))___p1;						\
 })
 #endif
 
@@ -229,14 +229,14 @@ do {									\
 #ifndef smp_cond_load_relaxed
 #define smp_cond_load_relaxed(ptr, cond_expr) ({		\
 	typeof(ptr) __PTR = (ptr);				\
-	typeof(*ptr) VAL;					\
+	__unqual_scalar_typeof(*ptr) VAL;			\
 	for (;;) {						\
 		VAL = READ_ONCE(*__PTR);			\
 		if (cond_expr)					\
 			break;					\
 		cpu_relax();					\
 	}							\
-	VAL;							\
+	(typeof(*ptr))VAL;					\
 })
 #endif
 
@@ -250,10 +250,10 @@ do {									\
  */
 #ifndef smp_cond_load_acquire
 #define smp_cond_load_acquire(ptr, cond_expr) ({		\
-	typeof(*ptr) _val;					\
+	__unqual_scalar_typeof(*ptr) _val;			\
 	_val = smp_cond_load_relaxed(ptr, cond_expr);		\
 	smp_acquire__after_ctrl_dep();				\
-	_val;							\
+	(typeof(*ptr))_val;					\
 })
 #endif
 
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 08/10] arm64: barrier: Use '__unqual_scalar_typeof' for acquire/release macros

[Will Deacon]

Passing volatile-qualified pointers to the arm64 implementations of the
load-acquire/store-release macros results in a re-load from the stack
and a bunch of associated stack-protector churn due to the temporary
result variable inheriting the volatile semantics thanks to the use of
'typeof()'.

Define these temporary variables using 'unqual_scalar_typeof' to drop
the volatile qualifier in the case that they are scalar types.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/barrier.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 7d9cc5ec4971..fb4c27506ef4 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -76,8 +76,8 @@ static inline unsigned long array_index_mask_nospec(unsigned long idx,
 #define __smp_store_release(p, v)					\
 do {									\
 	typeof(p) __p = (p);						\
-	union { typeof(*p) __val; char __c[1]; } __u =			\
-		{ .__val = (__force typeof(*p)) (v) };			\
+	union { __unqual_scalar_typeof(*p) __val; char __c[1]; } __u =	\
+		{ .__val = (__force __unqual_scalar_typeof(*p)) (v) };	\
 	compiletime_assert_atomic_type(*p);				\
 	kasan_check_write(__p, sizeof(*p));				\
 	switch (sizeof(*p)) {						\
@@ -110,7 +110,7 @@ do {									\
 
 #define __smp_load_acquire(p)						\
 ({									\
-	union { typeof(*p) __val; char __c[1]; } __u;			\
+	union { __unqual_scalar_typeof(*p) __val; char __c[1]; } __u;	\
 	typeof(p) __p = (p);						\
 	compiletime_assert_atomic_type(*p);				\
 	kasan_check_read(__p, sizeof(*p));				\
@@ -136,33 +136,33 @@ do {									\
 			: "Q" (*__p) : "memory");			\
 		break;							\
 	}								\
-	__u.__val;							\
+	(typeof(*p))__u.__val;						\
 })
 
 #define smp_cond_load_relaxed(ptr, cond_expr)				\
 ({									\
 	typeof(ptr) __PTR = (ptr);					\
-	typeof(*ptr) VAL;						\
+	__unqual_scalar_typeof(*ptr) VAL;				\
 	for (;;) {							\
 		VAL = READ_ONCE(*__PTR);				\
 		if (cond_expr)						\
 			break;						\
 		__cmpwait_relaxed(__PTR, VAL);				\
 	}								\
-	VAL;								\
+	(typeof(*ptr))VAL;						\
 })
 
 #define smp_cond_load_acquire(ptr, cond_expr)				\
 ({									\
 	typeof(ptr) __PTR = (ptr);					\
-	typeof(*ptr) VAL;						\
+	__unqual_scalar_typeof(*ptr) VAL;				\
 	for (;;) {							\
 		VAL = smp_load_acquire(__PTR);				\
 		if (cond_expr)						\
 			break;						\
 		__cmpwait_relaxed(__PTR, VAL);				\
 	}								\
-	VAL;								\
+	(typeof(*ptr))VAL;						\
 })
 
 #include <asm-generic/barrier.h>
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Will Deacon]

It is very rare to see versions of GCC prior to 4.8 being used to build
the mainline kernel. These old compilers are also know to have codegen
issues which can lead to silent miscompilation:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

Raise the minimum GCC version for kernel build to 4.8 and remove some
tautological Kconfig dependencies as a consequence.

Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 Documentation/process/changes.rst |  2 +-
 arch/arm/crypto/Kconfig           | 12 ++++++------
 crypto/Kconfig                    |  1 -
 include/linux/compiler-gcc.h      |  5 ++---
 init/Kconfig                      |  1 -
 scripts/Kconfig.include           |  3 ---
 scripts/gcc-plugins/Kconfig       |  4 +---
 7 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index 2284f2221f02..f2cbfa901cc8 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
 ====================== ===============  ========================================
         Program        Minimal version       Command to check the version
 ====================== ===============  ========================================
-GNU C                  4.6              gcc --version
+GNU C                  4.8              gcc --version
 GNU make               3.81             make --version
 binutils               2.21             ld -v
 flex                   2.5.35           flex --version
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 2674de6ada1f..c9bf2df85cb9 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -30,7 +30,7 @@ config CRYPTO_SHA1_ARM_NEON
 
 config CRYPTO_SHA1_ARM_CE
 	tristate "SHA1 digest algorithm (ARM v8 Crypto Extensions)"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	select CRYPTO_SHA1_ARM
 	select CRYPTO_HASH
 	help
@@ -39,7 +39,7 @@ config CRYPTO_SHA1_ARM_CE
 
 config CRYPTO_SHA2_ARM_CE
 	tristate "SHA-224/256 digest algorithm (ARM v8 Crypto Extensions)"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	select CRYPTO_SHA256_ARM
 	select CRYPTO_HASH
 	help
@@ -96,7 +96,7 @@ config CRYPTO_AES_ARM_BS
 
 config CRYPTO_AES_ARM_CE
 	tristate "Accelerated AES using ARMv8 Crypto Extensions"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	select CRYPTO_SKCIPHER
 	select CRYPTO_LIB_AES
 	select CRYPTO_SIMD
@@ -106,7 +106,7 @@ config CRYPTO_AES_ARM_CE
 
 config CRYPTO_GHASH_ARM_CE
 	tristate "PMULL-accelerated GHASH using NEON/ARMv8 Crypto Extensions"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	select CRYPTO_HASH
 	select CRYPTO_CRYPTD
 	select CRYPTO_GF128MUL
@@ -118,13 +118,13 @@ config CRYPTO_GHASH_ARM_CE
 
 config CRYPTO_CRCT10DIF_ARM_CE
 	tristate "CRCT10DIF digest algorithm using PMULL instructions"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	depends on CRC_T10DIF
 	select CRYPTO_HASH
 
 config CRYPTO_CRC32_ARM_CE
 	tristate "CRC32(C) digest algorithm using CRC and/or PMULL instructions"
-	depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
+	depends on KERNEL_MODE_NEON
 	depends on CRC32
 	select CRYPTO_HASH
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 5575d48473bd..bd8540f56efc 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -320,7 +320,6 @@ config CRYPTO_AEGIS128
 config CRYPTO_AEGIS128_SIMD
 	bool "Support SIMD acceleration for AEGIS-128"
 	depends on CRYPTO_AEGIS128 && ((ARM || ARM64) && KERNEL_MODE_NEON)
-	depends on !ARM || CC_IS_CLANG || GCC_VERSION >= 40800
 	default y
 
 config CRYPTO_AEGIS128_AESNI_SSE2
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index d7ee4c6bad48..e2f725273261 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -10,7 +10,8 @@
 		     + __GNUC_MINOR__ * 100	\
 		     + __GNUC_PATCHLEVEL__)
 
-#if GCC_VERSION < 40600
+/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
+#if GCC_VERSION < 40800
 # error Sorry, your compiler is too old - please upgrade it.
 #endif
 
@@ -126,9 +127,7 @@
 #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP) && !defined(__CHECKER__)
 #define __HAVE_BUILTIN_BSWAP32__
 #define __HAVE_BUILTIN_BSWAP64__
-#if GCC_VERSION >= 40800
 #define __HAVE_BUILTIN_BSWAP16__
-#endif
 #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP && !__CHECKER__ */
 
 #if GCC_VERSION >= 70000
diff --git a/init/Kconfig b/init/Kconfig
index bdc2f1b1667b..46729aa2ca0b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1257,7 +1257,6 @@ config LD_DEAD_CODE_DATA_ELIMINATION
 	bool "Dead code and data elimination (EXPERIMENTAL)"
 	depends on HAVE_LD_DEAD_CODE_DATA_ELIMINATION
 	depends on EXPERT
-	depends on !(FUNCTION_TRACER && CC_IS_GCC && GCC_VERSION < 40800)
 	depends on $(cc-option,-ffunction-sections -fdata-sections)
 	depends on $(ld-option,--gc-sections)
 	help
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 4e645a798b56..5b7dc6635c4e 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -43,6 +43,3 @@ gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
 
 # Return y if the compiler is GCC, n otherwise
 cc-is-gcc := $(success,$(CC) --version | head -n 1 | grep -q gcc)
-
-# Warn if the compiler is GCC prior to 4.8
-$(warning-if,$(if-success,[ $(gcc-version) -lt 40800 ],$(cc-is-gcc),n),"Your compiler is old and may miscompile the kernel due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 - please upgrade it.")
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index e3569543bdac..a0f669cd224f 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -1,9 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
-preferred-plugin-hostcc := $(if-success,[ $(gcc-version) -ge 40800 ],$(HOSTCXX),$(HOSTCC))
-
 config PLUGIN_HOSTCC
 	string
-	default "$(shell,$(srctree)/scripts/gcc-plugin.sh "$(preferred-plugin-hostcc)" "$(HOSTCXX)" "$(CC)")" if CC_IS_GCC
+	default "$(shell,$(srctree)/scripts/gcc-plugin.sh "$(HOSTCXX)" "$(HOSTCXX)" "$(CC)")" if CC_IS_GCC
 	help
 	  Host compiler used to build GCC plugins.  This can be $(HOSTCXX),
 	  $(HOSTCC), or a null string if GCC plugin is unsupported.
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Nick Desaulniers]

On Thu, Jan 23, 2020 at 7:34 AM Will Deacon <will@kernel.org> wrote:
>
> It is very rare to see versions of GCC prior to 4.8 being used to build
> the mainline kernel. These old compilers are also know to have codegen
> issues which can lead to silent miscompilation:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>
> Raise the minimum GCC version for kernel build to 4.8 and remove some
> tautological Kconfig dependencies as a consequence.
>
> Cc: Nick Desaulniers <ndesaulniers@google.com>

Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
I wouldn't mind if this patch preceded the earlier one in the series
adding the warning, should the series require a v2 and if folks are
generally ok with bumping the min version.

> Cc: Arnd Bergmann <arnd@arndb.de>

Arnd had previously mentioned that one of the older RHEL releases
still supported was using a version of GCC < 4.8.  I don't know enough
about RHEL to know if packages are available to use newer compilers on
that distribution?  Or if it's a common version that kernel developers
are using?  It feels like eventually a workaround for a known compiler
bug becomes too burdensome to maintain, at which point we bump the
minimum version required.  In the future, it may be worthwhile for us
to discuss kernel toolchain upgrade process, and potentially document
it.

> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  Documentation/process/changes.rst |  2 +-
>  arch/arm/crypto/Kconfig           | 12 ++++++------
>  crypto/Kconfig                    |  1 -
>  include/linux/compiler-gcc.h      |  5 ++---
>  init/Kconfig                      |  1 -
>  scripts/Kconfig.include           |  3 ---
>  scripts/gcc-plugins/Kconfig       |  4 +---
>  7 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> index 2284f2221f02..f2cbfa901cc8 100644
> --- a/Documentation/process/changes.rst
> +++ b/Documentation/process/changes.rst
> @@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
>  ====================== ===============  ========================================
>          Program        Minimal version       Command to check the version
>  ====================== ===============  ========================================
> -GNU C                  4.6              gcc --version
> +GNU C                  4.8              gcc --version
>  GNU make               3.81             make --version
>  binutils               2.21             ld -v
>  flex                   2.5.35           flex --version
> diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
> index 2674de6ada1f..c9bf2df85cb9 100644
> --- a/arch/arm/crypto/Kconfig
> +++ b/arch/arm/crypto/Kconfig
> @@ -30,7 +30,7 @@ config CRYPTO_SHA1_ARM_NEON
>
>  config CRYPTO_SHA1_ARM_CE
>         tristate "SHA1 digest algorithm (ARM v8 Crypto Extensions)"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         select CRYPTO_SHA1_ARM
>         select CRYPTO_HASH
>         help
> @@ -39,7 +39,7 @@ config CRYPTO_SHA1_ARM_CE
>
>  config CRYPTO_SHA2_ARM_CE
>         tristate "SHA-224/256 digest algorithm (ARM v8 Crypto Extensions)"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         select CRYPTO_SHA256_ARM
>         select CRYPTO_HASH
>         help
> @@ -96,7 +96,7 @@ config CRYPTO_AES_ARM_BS
>
>  config CRYPTO_AES_ARM_CE
>         tristate "Accelerated AES using ARMv8 Crypto Extensions"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         select CRYPTO_SKCIPHER
>         select CRYPTO_LIB_AES
>         select CRYPTO_SIMD
> @@ -106,7 +106,7 @@ config CRYPTO_AES_ARM_CE
>
>  config CRYPTO_GHASH_ARM_CE
>         tristate "PMULL-accelerated GHASH using NEON/ARMv8 Crypto Extensions"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         select CRYPTO_HASH
>         select CRYPTO_CRYPTD
>         select CRYPTO_GF128MUL
> @@ -118,13 +118,13 @@ config CRYPTO_GHASH_ARM_CE
>
>  config CRYPTO_CRCT10DIF_ARM_CE
>         tristate "CRCT10DIF digest algorithm using PMULL instructions"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         depends on CRC_T10DIF
>         select CRYPTO_HASH
>
>  config CRYPTO_CRC32_ARM_CE
>         tristate "CRC32(C) digest algorithm using CRC and/or PMULL instructions"
> -       depends on KERNEL_MODE_NEON && (CC_IS_CLANG || GCC_VERSION >= 40800)
> +       depends on KERNEL_MODE_NEON
>         depends on CRC32
>         select CRYPTO_HASH
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 5575d48473bd..bd8540f56efc 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -320,7 +320,6 @@ config CRYPTO_AEGIS128
>  config CRYPTO_AEGIS128_SIMD
>         bool "Support SIMD acceleration for AEGIS-128"
>         depends on CRYPTO_AEGIS128 && ((ARM || ARM64) && KERNEL_MODE_NEON)
> -       depends on !ARM || CC_IS_CLANG || GCC_VERSION >= 40800
>         default y
>
>  config CRYPTO_AEGIS128_AESNI_SSE2
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index d7ee4c6bad48..e2f725273261 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -10,7 +10,8 @@
>                      + __GNUC_MINOR__ * 100     \
>                      + __GNUC_PATCHLEVEL__)
>
> -#if GCC_VERSION < 40600
> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
> +#if GCC_VERSION < 40800
>  # error Sorry, your compiler is too old - please upgrade it.
>  #endif
>
> @@ -126,9 +127,7 @@
>  #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP) && !defined(__CHECKER__)
>  #define __HAVE_BUILTIN_BSWAP32__
>  #define __HAVE_BUILTIN_BSWAP64__
> -#if GCC_VERSION >= 40800
>  #define __HAVE_BUILTIN_BSWAP16__
> -#endif
>  #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP && !__CHECKER__ */
>
>  #if GCC_VERSION >= 70000
> diff --git a/init/Kconfig b/init/Kconfig
> index bdc2f1b1667b..46729aa2ca0b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1257,7 +1257,6 @@ config LD_DEAD_CODE_DATA_ELIMINATION
>         bool "Dead code and data elimination (EXPERIMENTAL)"
>         depends on HAVE_LD_DEAD_CODE_DATA_ELIMINATION
>         depends on EXPERT
> -       depends on !(FUNCTION_TRACER && CC_IS_GCC && GCC_VERSION < 40800)
>         depends on $(cc-option,-ffunction-sections -fdata-sections)
>         depends on $(ld-option,--gc-sections)
>         help
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 4e645a798b56..5b7dc6635c4e 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -43,6 +43,3 @@ gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
>
>  # Return y if the compiler is GCC, n otherwise
>  cc-is-gcc := $(success,$(CC) --version | head -n 1 | grep -q gcc)
> -
> -# Warn if the compiler is GCC prior to 4.8
> -$(warning-if,$(if-success,[ $(gcc-version) -lt 40800 ],$(cc-is-gcc),n),"Your compiler is old and may miscompile the kernel due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 - please upgrade it.")
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index e3569543bdac..a0f669cd224f 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -1,9 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -preferred-plugin-hostcc := $(if-success,[ $(gcc-version) -ge 40800 ],$(HOSTCXX),$(HOSTCC))
> -
>  config PLUGIN_HOSTCC
>         string
> -       default "$(shell,$(srctree)/scripts/gcc-plugin.sh "$(preferred-plugin-hostcc)" "$(HOSTCXX)" "$(CC)")" if CC_IS_GCC
> +       default "$(shell,$(srctree)/scripts/gcc-plugin.sh "$(HOSTCXX)" "$(HOSTCXX)" "$(CC)")" if CC_IS_GCC
>         help
>           Host compiler used to build GCC plugins.  This can be $(HOSTCXX),
>           $(HOSTCC), or a null string if GCC plugin is unsupported.
> --
> 2.25.0.341.g760bfbb309-goog
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Peter Zijlstra]

On Thu, Jan 23, 2020 at 10:36:37AM -0800, Nick Desaulniers wrote:
> On Thu, Jan 23, 2020 at 7:34 AM Will Deacon <will@kernel.org> wrote:
> >
> > It is very rare to see versions of GCC prior to 4.8 being used to build
> > the mainline kernel. These old compilers are also know to have codegen
> > issues which can lead to silent miscompilation:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> >
> > Raise the minimum GCC version for kernel build to 4.8 and remove some
> > tautological Kconfig dependencies as a consequence.
> >
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> 
> Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> I wouldn't mind if this patch preceded the earlier one in the series
> adding the warning, should the series require a v2 and if folks are
> generally ok with bumping the min version.

If I hadn't actually read your reply, I would have never spotted that
reviewed-by tag, hidden in a blob of text like that.

Adding some whitespace before and after, such that it stands out a
little more, might avoid such issues.

Re: [PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Nick Desaulniers]

On Fri, Jan 24, 2020 at 12:26 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jan 23, 2020 at 10:36:37AM -0800, Nick Desaulniers wrote:
> > On Thu, Jan 23, 2020 at 7:34 AM Will Deacon <will@kernel.org> wrote:
> > >
> > > It is very rare to see versions of GCC prior to 4.8 being used to build
> > > the mainline kernel. These old compilers are also know to have codegen
> > > issues which can lead to silent miscompilation:
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> > >
> > > Raise the minimum GCC version for kernel build to 4.8 and remove some
> > > tautological Kconfig dependencies as a consequence.
> > >
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> >
> > Thanks for the patch.
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > I wouldn't mind if this patch preceded the earlier one in the series
> > adding the warning, should the series require a v2 and if folks are
> > generally ok with bumping the min version.
>
> If I hadn't actually read your reply, I would have never spotted that
> reviewed-by tag, hidden in a blob of text like that.
>
> Adding some whitespace before and after, such that it stands out a
> little more, might avoid such issues.

Ack. Do maintainers have tools for fetching patch series and
automating collecting Reviewed-by tags, or is it all extremely manual?

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Peter Zijlstra]

On Fri, Jan 24, 2020 at 09:05:27AM -0800, Nick Desaulniers wrote:

> Ack. Do maintainers have tools for fetching patch series and
> automating collecting Reviewed-by tags, or is it all extremely manual?

Both; I'm currently still doing it manually, but I know that for example
Thomas's scripts are a lot smarter and do it for him.

I'm meaning to switch to his scripts, but I suppose there's some inertia
at play :-)

Re: [PATCH v2 09/10] compiler/gcc: Raise minimum GCC version for kernel builds to 4.8

[Michael Ellerman]

Nick Desaulniers <ndesaulniers@google.com> writes:
> On Fri, Jan 24, 2020 at 12:26 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> On Thu, Jan 23, 2020 at 10:36:37AM -0800, Nick Desaulniers wrote:
>> > On Thu, Jan 23, 2020 at 7:34 AM Will Deacon <will@kernel.org> wrote:
>> > > It is very rare to see versions of GCC prior to 4.8 being used to build
>> > > the mainline kernel. These old compilers are also know to have codegen
>> > > issues which can lead to silent miscompilation:
>> > >
>> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
>> > >
>> > > Raise the minimum GCC version for kernel build to 4.8 and remove some
>> > > tautological Kconfig dependencies as a consequence.
>> > >
>> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
>> >
>> > Thanks for the patch.
>> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>> > I wouldn't mind if this patch preceded the earlier one in the series
>> > adding the warning, should the series require a v2 and if folks are
>> > generally ok with bumping the min version.
>>
>> If I hadn't actually read your reply, I would have never spotted that
>> reviewed-by tag, hidden in a blob of text like that.
>>
>> Adding some whitespace before and after, such that it stands out a
>> little more, might avoid such issues.
>
> Ack. Do maintainers have tools for fetching patch series and
> automating collecting Reviewed-by tags, or is it all extremely manual?

Patchwork collects them for you. But not all maintainers use patchwork,
it's a bit new and trendy ;)

cheers

[PATCH v2 10/10] gcov: Remove old GCC 3.4 support

[Will Deacon]

The kernel requires at least GCC 4.8 in order to build, and so there is
no need to cater for the pre-4.7 gcov format.

Remove the obsolete code.

Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/gcov/Kconfig   |  24 --
 kernel/gcov/Makefile  |   3 +-
 kernel/gcov/gcc_3_4.c | 573 ------------------------------------------
 3 files changed, 1 insertion(+), 599 deletions(-)
 delete mode 100644 kernel/gcov/gcc_3_4.c

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 060e8e726755..8df7fad82df8 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -51,28 +51,4 @@ config GCOV_PROFILE_ALL
 	larger and run slower. Also be sure to exclude files from profiling
 	which are not linked to the kernel image to prevent linker errors.
 
-choice
-	prompt "Specify GCOV format"
-	depends on GCOV_KERNEL
-	depends on CC_IS_GCC
-	---help---
-	The gcov format is usually determined by the GCC version, and the
-	default is chosen according to your GCC version. However, there are
-	exceptions where format changes are integrated in lower-version GCCs.
-	In such a case, change this option to adjust the format used in the
-	kernel accordingly.
-
-config GCOV_FORMAT_3_4
-	bool "GCC 3.4 format"
-	depends on GCC_VERSION < 40700
-	---help---
-	Select this option to use the format defined by GCC 3.4.
-
-config GCOV_FORMAT_4_7
-	bool "GCC 4.7 format"
-	---help---
-	Select this option to use the format defined by GCC 4.7.
-
-endchoice
-
 endmenu
diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index d66a74b0f100..16f8ecc7d882 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -2,6 +2,5 @@
 ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
 
 obj-y := base.o fs.o
-obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
-obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
+obj-$(CONFIG_CC_IS_GCC) += gcc_base.o gcc_4_7.o
 obj-$(CONFIG_CC_IS_CLANG) += clang.o
diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
deleted file mode 100644
index 801ee4b0b969..000000000000
--- a/kernel/gcov/gcc_3_4.c
+++ /dev/null
@@ -1,573 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  This code provides functions to handle gcc's profiling data format
- *  introduced with gcc 3.4. Future versions of gcc may change the gcov
- *  format (as happened before), so all format-specific information needs
- *  to be kept modular and easily exchangeable.
- *
- *  This file is based on gcc-internal definitions. Functions and data
- *  structures are defined to be compatible with gcc counterparts.
- *  For a better understanding, refer to gcc source: gcc/gcov-io.h.
- *
- *    Copyright IBM Corp. 2009
- *    Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
- *
- *    Uses gcc-internal data definitions.
- */
-
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/string.h>
-#include <linux/seq_file.h>
-#include <linux/vmalloc.h>
-#include "gcov.h"
-
-#define GCOV_COUNTERS		5
-
-static struct gcov_info *gcov_info_head;
-
-/**
- * struct gcov_fn_info - profiling meta data per function
- * @ident: object file-unique function identifier
- * @checksum: function checksum
- * @n_ctrs: number of values per counter type belonging to this function
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time.
- */
-struct gcov_fn_info {
-	unsigned int ident;
-	unsigned int checksum;
-	unsigned int n_ctrs[0];
-};
-
-/**
- * struct gcov_ctr_info - profiling data per counter type
- * @num: number of counter values for this type
- * @values: array of counter values for this type
- * @merge: merge function for counter values of this type (unused)
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the values array.
- */
-struct gcov_ctr_info {
-	unsigned int	num;
-	gcov_type	*values;
-	void		(*merge)(gcov_type *, unsigned int);
-};
-
-/**
- * struct gcov_info - profiling data per object file
- * @version: gcov version magic indicating the gcc version used for compilation
- * @next: list head for a singly-linked list
- * @stamp: time stamp
- * @filename: name of the associated gcov data file
- * @n_functions: number of instrumented functions
- * @functions: function data
- * @ctr_mask: mask specifying which counter types are active
- * @counts: counter data per counter type
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the next pointer.
- */
-struct gcov_info {
-	unsigned int			version;
-	struct gcov_info		*next;
-	unsigned int			stamp;
-	const char			*filename;
-	unsigned int			n_functions;
-	const struct gcov_fn_info	*functions;
-	unsigned int			ctr_mask;
-	struct gcov_ctr_info		counts[0];
-};
-
-/**
- * gcov_info_filename - return info filename
- * @info: profiling data set
- */
-const char *gcov_info_filename(struct gcov_info *info)
-{
-	return info->filename;
-}
-
-/**
- * gcov_info_version - return info version
- * @info: profiling data set
- */
-unsigned int gcov_info_version(struct gcov_info *info)
-{
-	return info->version;
-}
-
-/**
- * gcov_info_next - return next profiling data set
- * @info: profiling data set
- *
- * Returns next gcov_info following @info or first gcov_info in the chain if
- * @info is %NULL.
- */
-struct gcov_info *gcov_info_next(struct gcov_info *info)
-{
-	if (!info)
-		return gcov_info_head;
-
-	return info->next;
-}
-
-/**
- * gcov_info_link - link/add profiling data set to the list
- * @info: profiling data set
- */
-void gcov_info_link(struct gcov_info *info)
-{
-	info->next = gcov_info_head;
-	gcov_info_head = info;
-}
-
-/**
- * gcov_info_unlink - unlink/remove profiling data set from the list
- * @prev: previous profiling data set
- * @info: profiling data set
- */
-void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
-{
-	if (prev)
-		prev->next = info->next;
-	else
-		gcov_info_head = info->next;
-}
-
-/**
- * gcov_info_within_module - check if a profiling data set belongs to a module
- * @info: profiling data set
- * @mod: module
- *
- * Returns true if profiling data belongs module, false otherwise.
- */
-bool gcov_info_within_module(struct gcov_info *info, struct module *mod)
-{
-	return within_module((unsigned long)info, mod);
-}
-
-/* Symbolic links to be created for each profiling data file. */
-const struct gcov_link gcov_link[] = {
-	{ OBJ_TREE, "gcno" },	/* Link to .gcno file in $(objtree). */
-	{ 0, NULL},
-};
-
-/*
- * Determine whether a counter is active. Based on gcc magic. Doesn't change
- * at run-time.
- */
-static int counter_active(struct gcov_info *info, unsigned int type)
-{
-	return (1 << type) & info->ctr_mask;
-}
-
-/* Determine number of active counters. Based on gcc magic. */
-static unsigned int num_counter_active(struct gcov_info *info)
-{
-	unsigned int i;
-	unsigned int result = 0;
-
-	for (i = 0; i < GCOV_COUNTERS; i++) {
-		if (counter_active(info, i))
-			result++;
-	}
-	return result;
-}
-
-/**
- * gcov_info_reset - reset profiling data to zero
- * @info: profiling data set
- */
-void gcov_info_reset(struct gcov_info *info)
-{
-	unsigned int active = num_counter_active(info);
-	unsigned int i;
-
-	for (i = 0; i < active; i++) {
-		memset(info->counts[i].values, 0,
-		       info->counts[i].num * sizeof(gcov_type));
-	}
-}
-
-/**
- * gcov_info_is_compatible - check if profiling data can be added
- * @info1: first profiling data set
- * @info2: second profiling data set
- *
- * Returns non-zero if profiling data can be added, zero otherwise.
- */
-int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
-{
-	return (info1->stamp == info2->stamp);
-}
-
-/**
- * gcov_info_add - add up profiling data
- * @dest: profiling data set to which data is added
- * @source: profiling data set which is added
- *
- * Adds profiling counts of @source to @dest.
- */
-void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
-{
-	unsigned int i;
-	unsigned int j;
-
-	for (i = 0; i < num_counter_active(dest); i++) {
-		for (j = 0; j < dest->counts[i].num; j++) {
-			dest->counts[i].values[j] +=
-				source->counts[i].values[j];
-		}
-	}
-}
-
-/* Get size of function info entry. Based on gcc magic. */
-static size_t get_fn_size(struct gcov_info *info)
-{
-	size_t size;
-
-	size = sizeof(struct gcov_fn_info) + num_counter_active(info) *
-	       sizeof(unsigned int);
-	if (__alignof__(struct gcov_fn_info) > sizeof(unsigned int))
-		size = ALIGN(size, __alignof__(struct gcov_fn_info));
-	return size;
-}
-
-/* Get address of function info entry. Based on gcc magic. */
-static struct gcov_fn_info *get_fn_info(struct gcov_info *info, unsigned int fn)
-{
-	return (struct gcov_fn_info *)
-		((char *) info->functions + fn * get_fn_size(info));
-}
-
-/**
- * gcov_info_dup - duplicate profiling data set
- * @info: profiling data set to duplicate
- *
- * Return newly allocated duplicate on success, %NULL on error.
- */
-struct gcov_info *gcov_info_dup(struct gcov_info *info)
-{
-	struct gcov_info *dup;
-	unsigned int i;
-	unsigned int active;
-
-	/* Duplicate gcov_info. */
-	active = num_counter_active(info);
-	dup = kzalloc(struct_size(dup, counts, active), GFP_KERNEL);
-	if (!dup)
-		return NULL;
-	dup->version		= info->version;
-	dup->stamp		= info->stamp;
-	dup->n_functions	= info->n_functions;
-	dup->ctr_mask		= info->ctr_mask;
-	/* Duplicate filename. */
-	dup->filename		= kstrdup(info->filename, GFP_KERNEL);
-	if (!dup->filename)
-		goto err_free;
-	/* Duplicate table of functions. */
-	dup->functions = kmemdup(info->functions, info->n_functions *
-				 get_fn_size(info), GFP_KERNEL);
-	if (!dup->functions)
-		goto err_free;
-	/* Duplicate counter arrays. */
-	for (i = 0; i < active ; i++) {
-		struct gcov_ctr_info *ctr = &info->counts[i];
-		size_t size = ctr->num * sizeof(gcov_type);
-
-		dup->counts[i].num = ctr->num;
-		dup->counts[i].merge = ctr->merge;
-		dup->counts[i].values = vmalloc(size);
-		if (!dup->counts[i].values)
-			goto err_free;
-		memcpy(dup->counts[i].values, ctr->values, size);
-	}
-	return dup;
-
-err_free:
-	gcov_info_free(dup);
-	return NULL;
-}
-
-/**
- * gcov_info_free - release memory for profiling data set duplicate
- * @info: profiling data set duplicate to free
- */
-void gcov_info_free(struct gcov_info *info)
-{
-	unsigned int active = num_counter_active(info);
-	unsigned int i;
-
-	for (i = 0; i < active ; i++)
-		vfree(info->counts[i].values);
-	kfree(info->functions);
-	kfree(info->filename);
-	kfree(info);
-}
-
-/**
- * struct type_info - iterator helper array
- * @ctr_type: counter type
- * @offset: index of the first value of the current function for this type
- *
- * This array is needed to convert the in-memory data format into the in-file
- * data format:
- *
- * In-memory:
- *   for each counter type
- *     for each function
- *       values
- *
- * In-file:
- *   for each function
- *     for each counter type
- *       values
- *
- * See gcc source gcc/gcov-io.h for more information on data organization.
- */
-struct type_info {
-	int ctr_type;
-	unsigned int offset;
-};
-
-/**
- * struct gcov_iterator - specifies current file position in logical records
- * @info: associated profiling data
- * @record: record type
- * @function: function number
- * @type: counter type
- * @count: index into values array
- * @num_types: number of counter types
- * @type_info: helper array to get values-array offset for current function
- */
-struct gcov_iterator {
-	struct gcov_info *info;
-
-	int record;
-	unsigned int function;
-	unsigned int type;
-	unsigned int count;
-
-	int num_types;
-	struct type_info type_info[0];
-};
-
-static struct gcov_fn_info *get_func(struct gcov_iterator *iter)
-{
-	return get_fn_info(iter->info, iter->function);
-}
-
-static struct type_info *get_type(struct gcov_iterator *iter)
-{
-	return &iter->type_info[iter->type];
-}
-
-/**
- * gcov_iter_new - allocate and initialize profiling data iterator
- * @info: profiling data set to be iterated
- *
- * Return file iterator on success, %NULL otherwise.
- */
-struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
-{
-	struct gcov_iterator *iter;
-
-	iter = kzalloc(struct_size(iter, type_info, num_counter_active(info)),
-		       GFP_KERNEL);
-	if (iter)
-		iter->info = info;
-
-	return iter;
-}
-
-/**
- * gcov_iter_free - release memory for iterator
- * @iter: file iterator to free
- */
-void gcov_iter_free(struct gcov_iterator *iter)
-{
-	kfree(iter);
-}
-
-/**
- * gcov_iter_get_info - return profiling data set for given file iterator
- * @iter: file iterator
- */
-struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
-{
-	return iter->info;
-}
-
-/**
- * gcov_iter_start - reset file iterator to starting position
- * @iter: file iterator
- */
-void gcov_iter_start(struct gcov_iterator *iter)
-{
-	int i;
-
-	iter->record = 0;
-	iter->function = 0;
-	iter->type = 0;
-	iter->count = 0;
-	iter->num_types = 0;
-	for (i = 0; i < GCOV_COUNTERS; i++) {
-		if (counter_active(iter->info, i)) {
-			iter->type_info[iter->num_types].ctr_type = i;
-			iter->type_info[iter->num_types++].offset = 0;
-		}
-	}
-}
-
-/* Mapping of logical record number to actual file content. */
-#define RECORD_FILE_MAGIC	0
-#define RECORD_GCOV_VERSION	1
-#define RECORD_TIME_STAMP	2
-#define RECORD_FUNCTION_TAG	3
-#define RECORD_FUNCTON_TAG_LEN	4
-#define RECORD_FUNCTION_IDENT	5
-#define RECORD_FUNCTION_CHECK	6
-#define RECORD_COUNT_TAG	7
-#define RECORD_COUNT_LEN	8
-#define RECORD_COUNT		9
-
-/**
- * gcov_iter_next - advance file iterator to next logical record
- * @iter: file iterator
- *
- * Return zero if new position is valid, non-zero if iterator has reached end.
- */
-int gcov_iter_next(struct gcov_iterator *iter)
-{
-	switch (iter->record) {
-	case RECORD_FILE_MAGIC:
-	case RECORD_GCOV_VERSION:
-	case RECORD_FUNCTION_TAG:
-	case RECORD_FUNCTON_TAG_LEN:
-	case RECORD_FUNCTION_IDENT:
-	case RECORD_COUNT_TAG:
-		/* Advance to next record */
-		iter->record++;
-		break;
-	case RECORD_COUNT:
-		/* Advance to next count */
-		iter->count++;
-		/* fall through */
-	case RECORD_COUNT_LEN:
-		if (iter->count < get_func(iter)->n_ctrs[iter->type]) {
-			iter->record = 9;
-			break;
-		}
-		/* Advance to next counter type */
-		get_type(iter)->offset += iter->count;
-		iter->count = 0;
-		iter->type++;
-		/* fall through */
-	case RECORD_FUNCTION_CHECK:
-		if (iter->type < iter->num_types) {
-			iter->record = 7;
-			break;
-		}
-		/* Advance to next function */
-		iter->type = 0;
-		iter->function++;
-		/* fall through */
-	case RECORD_TIME_STAMP:
-		if (iter->function < iter->info->n_functions)
-			iter->record = 3;
-		else
-			iter->record = -1;
-		break;
-	}
-	/* Check for EOF. */
-	if (iter->record == -1)
-		return -EINVAL;
-	else
-		return 0;
-}
-
-/**
- * seq_write_gcov_u32 - write 32 bit number in gcov format to seq_file
- * @seq: seq_file handle
- * @v: value to be stored
- *
- * Number format defined by gcc: numbers are recorded in the 32 bit
- * unsigned binary form of the endianness of the machine generating the
- * file.
- */
-static int seq_write_gcov_u32(struct seq_file *seq, u32 v)
-{
-	return seq_write(seq, &v, sizeof(v));
-}
-
-/**
- * seq_write_gcov_u64 - write 64 bit number in gcov format to seq_file
- * @seq: seq_file handle
- * @v: value to be stored
- *
- * Number format defined by gcc: numbers are recorded in the 32 bit
- * unsigned binary form of the endianness of the machine generating the
- * file. 64 bit numbers are stored as two 32 bit numbers, the low part
- * first.
- */
-static int seq_write_gcov_u64(struct seq_file *seq, u64 v)
-{
-	u32 data[2];
-
-	data[0] = (v & 0xffffffffUL);
-	data[1] = (v >> 32);
-	return seq_write(seq, data, sizeof(data));
-}
-
-/**
- * gcov_iter_write - write data for current pos to seq_file
- * @iter: file iterator
- * @seq: seq_file handle
- *
- * Return zero on success, non-zero otherwise.
- */
-int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
-{
-	int rc = -EINVAL;
-
-	switch (iter->record) {
-	case RECORD_FILE_MAGIC:
-		rc = seq_write_gcov_u32(seq, GCOV_DATA_MAGIC);
-		break;
-	case RECORD_GCOV_VERSION:
-		rc = seq_write_gcov_u32(seq, iter->info->version);
-		break;
-	case RECORD_TIME_STAMP:
-		rc = seq_write_gcov_u32(seq, iter->info->stamp);
-		break;
-	case RECORD_FUNCTION_TAG:
-		rc = seq_write_gcov_u32(seq, GCOV_TAG_FUNCTION);
-		break;
-	case RECORD_FUNCTON_TAG_LEN:
-		rc = seq_write_gcov_u32(seq, 2);
-		break;
-	case RECORD_FUNCTION_IDENT:
-		rc = seq_write_gcov_u32(seq, get_func(iter)->ident);
-		break;
-	case RECORD_FUNCTION_CHECK:
-		rc = seq_write_gcov_u32(seq, get_func(iter)->checksum);
-		break;
-	case RECORD_COUNT_TAG:
-		rc = seq_write_gcov_u32(seq,
-			GCOV_TAG_FOR_COUNTER(get_type(iter)->ctr_type));
-		break;
-	case RECORD_COUNT_LEN:
-		rc = seq_write_gcov_u32(seq,
-				get_func(iter)->n_ctrs[iter->type] * 2);
-		break;
-	case RECORD_COUNT:
-		rc = seq_write_gcov_u64(seq,
-			iter->info->counts[iter->type].
-				values[iter->count + get_type(iter)->offset]);
-		break;
-	}
-	return rc;
-}
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v2 10/10] gcov: Remove old GCC 3.4 support

[Nick Desaulniers]

On Thu, Jan 23, 2020 at 7:34 AM Will Deacon <will@kernel.org> wrote:
>
> The kernel requires at least GCC 4.8 in order to build, and so there is
> no need to cater for the pre-4.7 gcov format.

Thanks for the cleanup! Deleting such a large volume of code is great!

>
> Remove the obsolete code.
>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  kernel/gcov/Kconfig   |  24 --
>  kernel/gcov/Makefile  |   3 +-
>  kernel/gcov/gcc_3_4.c | 573 ------------------------------------------
>  3 files changed, 1 insertion(+), 599 deletions(-)
>  delete mode 100644 kernel/gcov/gcc_3_4.c
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 060e8e726755..8df7fad82df8 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -51,28 +51,4 @@ config GCOV_PROFILE_ALL
>         larger and run slower. Also be sure to exclude files from profiling
>         which are not linked to the kernel image to prevent linker errors.
>
> -choice
> -       prompt "Specify GCOV format"
> -       depends on GCOV_KERNEL
> -       depends on CC_IS_GCC
> -       ---help---
> -       The gcov format is usually determined by the GCC version, and the
> -       default is chosen according to your GCC version. However, there are
> -       exceptions where format changes are integrated in lower-version GCCs.
> -       In such a case, change this option to adjust the format used in the
> -       kernel accordingly.
> -
> -config GCOV_FORMAT_3_4
> -       bool "GCC 3.4 format"
> -       depends on GCC_VERSION < 40700

I was thinking this patch could be independant of the series, until I
saw this; before the series GCC 4.6+ is still supported, after GCC
4.8+ is.  So NVM.

> -       ---help---
> -       Select this option to use the format defined by GCC 3.4.
> -
> -config GCOV_FORMAT_4_7
> -       bool "GCC 4.7 format"
> -       ---help---
> -       Select this option to use the format defined by GCC 4.7.
> -
> -endchoice
> -
>  endmenu
> diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
> index d66a74b0f100..16f8ecc7d882 100644
> --- a/kernel/gcov/Makefile
> +++ b/kernel/gcov/Makefile
> @@ -2,6 +2,5 @@
>  ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>
>  obj-y := base.o fs.o
> -obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
> -obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
> +obj-$(CONFIG_CC_IS_GCC) += gcc_base.o gcc_4_7.o

For further refactoring/cleanups, the split between gcc_base.c and
gcc_4_7.c can now be combined into one source file.  gcc_base.c just
shared code with either 3.4 or 4.7, but I'll leave that as work that
can be done another day.  Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>  obj-$(CONFIG_CC_IS_CLANG) += clang.o
> diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
> deleted file mode 100644
> index 801ee4b0b969..000000000000
> --- a/kernel/gcov/gcc_3_4.c
> +++ /dev/null
> @@ -1,573 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - *  This code provides functions to handle gcc's profiling data format
> - *  introduced with gcc 3.4. Future versions of gcc may change the gcov
> - *  format (as happened before), so all format-specific information needs
> - *  to be kept modular and easily exchangeable.
> - *
> - *  This file is based on gcc-internal definitions. Functions and data
> - *  structures are defined to be compatible with gcc counterparts.
> - *  For a better understanding, refer to gcc source: gcc/gcov-io.h.
> - *
> - *    Copyright IBM Corp. 2009
> - *    Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
> - *
> - *    Uses gcc-internal data definitions.
> - */
> -
> -#include <linux/errno.h>
> -#include <linux/slab.h>
> -#include <linux/string.h>
> -#include <linux/seq_file.h>
> -#include <linux/vmalloc.h>
> -#include "gcov.h"
> -
> -#define GCOV_COUNTERS          5
> -
> -static struct gcov_info *gcov_info_head;
> -
> -/**
> - * struct gcov_fn_info - profiling meta data per function
> - * @ident: object file-unique function identifier
> - * @checksum: function checksum
> - * @n_ctrs: number of values per counter type belonging to this function
> - *
> - * This data is generated by gcc during compilation and doesn't change
> - * at run-time.
> - */
> -struct gcov_fn_info {
> -       unsigned int ident;
> -       unsigned int checksum;
> -       unsigned int n_ctrs[0];
> -};
> -
> -/**
> - * struct gcov_ctr_info - profiling data per counter type
> - * @num: number of counter values for this type
> - * @values: array of counter values for this type
> - * @merge: merge function for counter values of this type (unused)
> - *
> - * This data is generated by gcc during compilation and doesn't change
> - * at run-time with the exception of the values array.
> - */
> -struct gcov_ctr_info {
> -       unsigned int    num;
> -       gcov_type       *values;
> -       void            (*merge)(gcov_type *, unsigned int);
> -};
> -
> -/**
> - * struct gcov_info - profiling data per object file
> - * @version: gcov version magic indicating the gcc version used for compilation
> - * @next: list head for a singly-linked list
> - * @stamp: time stamp
> - * @filename: name of the associated gcov data file
> - * @n_functions: number of instrumented functions
> - * @functions: function data
> - * @ctr_mask: mask specifying which counter types are active
> - * @counts: counter data per counter type
> - *
> - * This data is generated by gcc during compilation and doesn't change
> - * at run-time with the exception of the next pointer.
> - */
> -struct gcov_info {
> -       unsigned int                    version;
> -       struct gcov_info                *next;
> -       unsigned int                    stamp;
> -       const char                      *filename;
> -       unsigned int                    n_functions;
> -       const struct gcov_fn_info       *functions;
> -       unsigned int                    ctr_mask;
> -       struct gcov_ctr_info            counts[0];
> -};
> -
> -/**
> - * gcov_info_filename - return info filename
> - * @info: profiling data set
> - */
> -const char *gcov_info_filename(struct gcov_info *info)
> -{
> -       return info->filename;
> -}
> -
> -/**
> - * gcov_info_version - return info version
> - * @info: profiling data set
> - */
> -unsigned int gcov_info_version(struct gcov_info *info)
> -{
> -       return info->version;
> -}
> -
> -/**
> - * gcov_info_next - return next profiling data set
> - * @info: profiling data set
> - *
> - * Returns next gcov_info following @info or first gcov_info in the chain if
> - * @info is %NULL.
> - */
> -struct gcov_info *gcov_info_next(struct gcov_info *info)
> -{
> -       if (!info)
> -               return gcov_info_head;
> -
> -       return info->next;
> -}
> -
> -/**
> - * gcov_info_link - link/add profiling data set to the list
> - * @info: profiling data set
> - */
> -void gcov_info_link(struct gcov_info *info)
> -{
> -       info->next = gcov_info_head;
> -       gcov_info_head = info;
> -}
> -
> -/**
> - * gcov_info_unlink - unlink/remove profiling data set from the list
> - * @prev: previous profiling data set
> - * @info: profiling data set
> - */
> -void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
> -{
> -       if (prev)
> -               prev->next = info->next;
> -       else
> -               gcov_info_head = info->next;
> -}
> -
> -/**
> - * gcov_info_within_module - check if a profiling data set belongs to a module
> - * @info: profiling data set
> - * @mod: module
> - *
> - * Returns true if profiling data belongs module, false otherwise.
> - */
> -bool gcov_info_within_module(struct gcov_info *info, struct module *mod)
> -{
> -       return within_module((unsigned long)info, mod);
> -}
> -
> -/* Symbolic links to be created for each profiling data file. */
> -const struct gcov_link gcov_link[] = {
> -       { OBJ_TREE, "gcno" },   /* Link to .gcno file in $(objtree). */
> -       { 0, NULL},
> -};
> -
> -/*
> - * Determine whether a counter is active. Based on gcc magic. Doesn't change
> - * at run-time.
> - */
> -static int counter_active(struct gcov_info *info, unsigned int type)
> -{
> -       return (1 << type) & info->ctr_mask;
> -}
> -
> -/* Determine number of active counters. Based on gcc magic. */
> -static unsigned int num_counter_active(struct gcov_info *info)
> -{
> -       unsigned int i;
> -       unsigned int result = 0;
> -
> -       for (i = 0; i < GCOV_COUNTERS; i++) {
> -               if (counter_active(info, i))
> -                       result++;
> -       }
> -       return result;
> -}
> -
> -/**
> - * gcov_info_reset - reset profiling data to zero
> - * @info: profiling data set
> - */
> -void gcov_info_reset(struct gcov_info *info)
> -{
> -       unsigned int active = num_counter_active(info);
> -       unsigned int i;
> -
> -       for (i = 0; i < active; i++) {
> -               memset(info->counts[i].values, 0,
> -                      info->counts[i].num * sizeof(gcov_type));
> -       }
> -}
> -
> -/**
> - * gcov_info_is_compatible - check if profiling data can be added
> - * @info1: first profiling data set
> - * @info2: second profiling data set
> - *
> - * Returns non-zero if profiling data can be added, zero otherwise.
> - */
> -int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
> -{
> -       return (info1->stamp == info2->stamp);
> -}
> -
> -/**
> - * gcov_info_add - add up profiling data
> - * @dest: profiling data set to which data is added
> - * @source: profiling data set which is added
> - *
> - * Adds profiling counts of @source to @dest.
> - */
> -void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
> -{
> -       unsigned int i;
> -       unsigned int j;
> -
> -       for (i = 0; i < num_counter_active(dest); i++) {
> -               for (j = 0; j < dest->counts[i].num; j++) {
> -                       dest->counts[i].values[j] +=
> -                               source->counts[i].values[j];
> -               }
> -       }
> -}
> -
> -/* Get size of function info entry. Based on gcc magic. */
> -static size_t get_fn_size(struct gcov_info *info)
> -{
> -       size_t size;
> -
> -       size = sizeof(struct gcov_fn_info) + num_counter_active(info) *
> -              sizeof(unsigned int);
> -       if (__alignof__(struct gcov_fn_info) > sizeof(unsigned int))
> -               size = ALIGN(size, __alignof__(struct gcov_fn_info));
> -       return size;
> -}
> -
> -/* Get address of function info entry. Based on gcc magic. */
> -static struct gcov_fn_info *get_fn_info(struct gcov_info *info, unsigned int fn)
> -{
> -       return (struct gcov_fn_info *)
> -               ((char *) info->functions + fn * get_fn_size(info));
> -}
> -
> -/**
> - * gcov_info_dup - duplicate profiling data set
> - * @info: profiling data set to duplicate
> - *
> - * Return newly allocated duplicate on success, %NULL on error.
> - */
> -struct gcov_info *gcov_info_dup(struct gcov_info *info)
> -{
> -       struct gcov_info *dup;
> -       unsigned int i;
> -       unsigned int active;
> -
> -       /* Duplicate gcov_info. */
> -       active = num_counter_active(info);
> -       dup = kzalloc(struct_size(dup, counts, active), GFP_KERNEL);
> -       if (!dup)
> -               return NULL;
> -       dup->version            = info->version;
> -       dup->stamp              = info->stamp;
> -       dup->n_functions        = info->n_functions;
> -       dup->ctr_mask           = info->ctr_mask;
> -       /* Duplicate filename. */
> -       dup->filename           = kstrdup(info->filename, GFP_KERNEL);
> -       if (!dup->filename)
> -               goto err_free;
> -       /* Duplicate table of functions. */
> -       dup->functions = kmemdup(info->functions, info->n_functions *
> -                                get_fn_size(info), GFP_KERNEL);
> -       if (!dup->functions)
> -               goto err_free;
> -       /* Duplicate counter arrays. */
> -       for (i = 0; i < active ; i++) {
> -               struct gcov_ctr_info *ctr = &info->counts[i];
> -               size_t size = ctr->num * sizeof(gcov_type);
> -
> -               dup->counts[i].num = ctr->num;
> -               dup->counts[i].merge = ctr->merge;
> -               dup->counts[i].values = vmalloc(size);
> -               if (!dup->counts[i].values)
> -                       goto err_free;
> -               memcpy(dup->counts[i].values, ctr->values, size);
> -       }
> -       return dup;
> -
> -err_free:
> -       gcov_info_free(dup);
> -       return NULL;
> -}
> -
> -/**
> - * gcov_info_free - release memory for profiling data set duplicate
> - * @info: profiling data set duplicate to free
> - */
> -void gcov_info_free(struct gcov_info *info)
> -{
> -       unsigned int active = num_counter_active(info);
> -       unsigned int i;
> -
> -       for (i = 0; i < active ; i++)
> -               vfree(info->counts[i].values);
> -       kfree(info->functions);
> -       kfree(info->filename);
> -       kfree(info);
> -}
> -
> -/**
> - * struct type_info - iterator helper array
> - * @ctr_type: counter type
> - * @offset: index of the first value of the current function for this type
> - *
> - * This array is needed to convert the in-memory data format into the in-file
> - * data format:
> - *
> - * In-memory:
> - *   for each counter type
> - *     for each function
> - *       values
> - *
> - * In-file:
> - *   for each function
> - *     for each counter type
> - *       values
> - *
> - * See gcc source gcc/gcov-io.h for more information on data organization.
> - */
> -struct type_info {
> -       int ctr_type;
> -       unsigned int offset;
> -};
> -
> -/**
> - * struct gcov_iterator - specifies current file position in logical records
> - * @info: associated profiling data
> - * @record: record type
> - * @function: function number
> - * @type: counter type
> - * @count: index into values array
> - * @num_types: number of counter types
> - * @type_info: helper array to get values-array offset for current function
> - */
> -struct gcov_iterator {
> -       struct gcov_info *info;
> -
> -       int record;
> -       unsigned int function;
> -       unsigned int type;
> -       unsigned int count;
> -
> -       int num_types;
> -       struct type_info type_info[0];
> -};
> -
> -static struct gcov_fn_info *get_func(struct gcov_iterator *iter)
> -{
> -       return get_fn_info(iter->info, iter->function);
> -}
> -
> -static struct type_info *get_type(struct gcov_iterator *iter)
> -{
> -       return &iter->type_info[iter->type];
> -}
> -
> -/**
> - * gcov_iter_new - allocate and initialize profiling data iterator
> - * @info: profiling data set to be iterated
> - *
> - * Return file iterator on success, %NULL otherwise.
> - */
> -struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
> -{
> -       struct gcov_iterator *iter;
> -
> -       iter = kzalloc(struct_size(iter, type_info, num_counter_active(info)),
> -                      GFP_KERNEL);
> -       if (iter)
> -               iter->info = info;
> -
> -       return iter;
> -}
> -
> -/**
> - * gcov_iter_free - release memory for iterator
> - * @iter: file iterator to free
> - */
> -void gcov_iter_free(struct gcov_iterator *iter)
> -{
> -       kfree(iter);
> -}
> -
> -/**
> - * gcov_iter_get_info - return profiling data set for given file iterator
> - * @iter: file iterator
> - */
> -struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
> -{
> -       return iter->info;
> -}
> -
> -/**
> - * gcov_iter_start - reset file iterator to starting position
> - * @iter: file iterator
> - */
> -void gcov_iter_start(struct gcov_iterator *iter)
> -{
> -       int i;
> -
> -       iter->record = 0;
> -       iter->function = 0;
> -       iter->type = 0;
> -       iter->count = 0;
> -       iter->num_types = 0;
> -       for (i = 0; i < GCOV_COUNTERS; i++) {
> -               if (counter_active(iter->info, i)) {
> -                       iter->type_info[iter->num_types].ctr_type = i;
> -                       iter->type_info[iter->num_types++].offset = 0;
> -               }
> -       }
> -}
> -
> -/* Mapping of logical record number to actual file content. */
> -#define RECORD_FILE_MAGIC      0
> -#define RECORD_GCOV_VERSION    1
> -#define RECORD_TIME_STAMP      2
> -#define RECORD_FUNCTION_TAG    3
> -#define RECORD_FUNCTON_TAG_LEN 4
> -#define RECORD_FUNCTION_IDENT  5
> -#define RECORD_FUNCTION_CHECK  6
> -#define RECORD_COUNT_TAG       7
> -#define RECORD_COUNT_LEN       8
> -#define RECORD_COUNT           9
> -
> -/**
> - * gcov_iter_next - advance file iterator to next logical record
> - * @iter: file iterator
> - *
> - * Return zero if new position is valid, non-zero if iterator has reached end.
> - */
> -int gcov_iter_next(struct gcov_iterator *iter)
> -{
> -       switch (iter->record) {
> -       case RECORD_FILE_MAGIC:
> -       case RECORD_GCOV_VERSION:
> -       case RECORD_FUNCTION_TAG:
> -       case RECORD_FUNCTON_TAG_LEN:
> -       case RECORD_FUNCTION_IDENT:
> -       case RECORD_COUNT_TAG:
> -               /* Advance to next record */
> -               iter->record++;
> -               break;
> -       case RECORD_COUNT:
> -               /* Advance to next count */
> -               iter->count++;
> -               /* fall through */
> -       case RECORD_COUNT_LEN:
> -               if (iter->count < get_func(iter)->n_ctrs[iter->type]) {
> -                       iter->record = 9;
> -                       break;
> -               }
> -               /* Advance to next counter type */
> -               get_type(iter)->offset += iter->count;
> -               iter->count = 0;
> -               iter->type++;
> -               /* fall through */
> -       case RECORD_FUNCTION_CHECK:
> -               if (iter->type < iter->num_types) {
> -                       iter->record = 7;
> -                       break;
> -               }
> -               /* Advance to next function */
> -               iter->type = 0;
> -               iter->function++;
> -               /* fall through */
> -       case RECORD_TIME_STAMP:
> -               if (iter->function < iter->info->n_functions)
> -                       iter->record = 3;
> -               else
> -                       iter->record = -1;
> -               break;
> -       }
> -       /* Check for EOF. */
> -       if (iter->record == -1)
> -               return -EINVAL;
> -       else
> -               return 0;
> -}
> -
> -/**
> - * seq_write_gcov_u32 - write 32 bit number in gcov format to seq_file
> - * @seq: seq_file handle
> - * @v: value to be stored
> - *
> - * Number format defined by gcc: numbers are recorded in the 32 bit
> - * unsigned binary form of the endianness of the machine generating the
> - * file.
> - */
> -static int seq_write_gcov_u32(struct seq_file *seq, u32 v)
> -{
> -       return seq_write(seq, &v, sizeof(v));
> -}
> -
> -/**
> - * seq_write_gcov_u64 - write 64 bit number in gcov format to seq_file
> - * @seq: seq_file handle
> - * @v: value to be stored
> - *
> - * Number format defined by gcc: numbers are recorded in the 32 bit
> - * unsigned binary form of the endianness of the machine generating the
> - * file. 64 bit numbers are stored as two 32 bit numbers, the low part
> - * first.
> - */
> -static int seq_write_gcov_u64(struct seq_file *seq, u64 v)
> -{
> -       u32 data[2];
> -
> -       data[0] = (v & 0xffffffffUL);
> -       data[1] = (v >> 32);
> -       return seq_write(seq, data, sizeof(data));
> -}
> -
> -/**
> - * gcov_iter_write - write data for current pos to seq_file
> - * @iter: file iterator
> - * @seq: seq_file handle
> - *
> - * Return zero on success, non-zero otherwise.
> - */
> -int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
> -{
> -       int rc = -EINVAL;
> -
> -       switch (iter->record) {
> -       case RECORD_FILE_MAGIC:
> -               rc = seq_write_gcov_u32(seq, GCOV_DATA_MAGIC);
> -               break;
> -       case RECORD_GCOV_VERSION:
> -               rc = seq_write_gcov_u32(seq, iter->info->version);
> -               break;
> -       case RECORD_TIME_STAMP:
> -               rc = seq_write_gcov_u32(seq, iter->info->stamp);
> -               break;
> -       case RECORD_FUNCTION_TAG:
> -               rc = seq_write_gcov_u32(seq, GCOV_TAG_FUNCTION);
> -               break;
> -       case RECORD_FUNCTON_TAG_LEN:
> -               rc = seq_write_gcov_u32(seq, 2);
> -               break;
> -       case RECORD_FUNCTION_IDENT:
> -               rc = seq_write_gcov_u32(seq, get_func(iter)->ident);
> -               break;
> -       case RECORD_FUNCTION_CHECK:
> -               rc = seq_write_gcov_u32(seq, get_func(iter)->checksum);
> -               break;
> -       case RECORD_COUNT_TAG:
> -               rc = seq_write_gcov_u32(seq,
> -                       GCOV_TAG_FOR_COUNTER(get_type(iter)->ctr_type));
> -               break;
> -       case RECORD_COUNT_LEN:
> -               rc = seq_write_gcov_u32(seq,
> -                               get_func(iter)->n_ctrs[iter->type] * 2);
> -               break;
> -       case RECORD_COUNT:
> -               rc = seq_write_gcov_u64(seq,
> -                       iter->info->counts[iter->type].
> -                               values[iter->count + get_type(iter)->offset]);
> -               break;
> -       }
> -       return rc;
> -}
> --
> 2.25.0.341.g760bfbb309-goog
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 10/10] gcov: Remove old GCC 3.4 support

[Peter Oberparleiter]

On 23.01.2020 16:33, Will Deacon wrote:
> The kernel requires at least GCC 4.8 in order to build, and so there is
> no need to cater for the pre-4.7 gcov format.
> 
> Remove the obsolete code.
> 
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  kernel/gcov/Kconfig   |  24 --
>  kernel/gcov/Makefile  |   3 +-
>  kernel/gcov/gcc_3_4.c | 573 ------------------------------------------
>  3 files changed, 1 insertion(+), 599 deletions(-)
>  delete mode 100644 kernel/gcov/gcc_3_4.c

Acked-by: Peter Oberparleiter <oberpar@linux.ibm.com>

Compiles, boots, produces coverage output on a s390 system with a
recent-ish GCC version, so that change is fine by me.

There's a final reference to the gcc_3_4.c file in the gcc_4_7.c header
comment, but its probably not worth the extra effort to adjust that as well.

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany

RE: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[David Laight]

From: Will Deacon
> Sent: 23 January 2020 15:34
...
>   * Only warn once at build-time if GCC prior to 4.8 is detected...
> 
>   * ... and then raise the minimum GCC version to 4.8, with an error for
>     older versions of the compiler

If the kernel compiled with gcc 4.7 is likely to be buggy, don't these
need to be in the other order?

Otherwise you need to keep the old versions for use with the old
compilers.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Will Deacon]

On Thu, Jan 23, 2020 at 05:07:40PM +0000, David Laight wrote:
> From: Will Deacon
> > Sent: 23 January 2020 15:34
> ...
> >   * Only warn once at build-time if GCC prior to 4.8 is detected...
> > 
> >   * ... and then raise the minimum GCC version to 4.8, with an error for
> >     older versions of the compiler
> 
> If the kernel compiled with gcc 4.7 is likely to be buggy, don't these
> need to be in the other order?
> 
> Otherwise you need to keep the old versions for use with the old
> compilers.

I think it depends how much we care about those older compilers. My series
first moves it to "Good luck mate, you're on your own" and then follows up
with a "Let me take that off you it's sharp".

Will

RE: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[David Laight]

From: Will Deacon
> Sent: 23 January 2020 17:17
> 
> On Thu, Jan 23, 2020 at 05:07:40PM +0000, David Laight wrote:
> > From: Will Deacon
> > > Sent: 23 January 2020 15:34
> > ...
> > >   * Only warn once at build-time if GCC prior to 4.8 is detected...
> > >
> > >   * ... and then raise the minimum GCC version to 4.8, with an error for
> > >     older versions of the compiler
> >
> > If the kernel compiled with gcc 4.7 is likely to be buggy, don't these
> > need to be in the other order?
> >
> > Otherwise you need to keep the old versions for use with the old
> > compilers.
> 
> I think it depends how much we care about those older compilers. My series
> first moves it to "Good luck mate, you're on your own" and then follows up
> with a "Let me take that off you it's sharp".

Depends on how 'sharp' it is.

If the kernel suffers from the code example in the gcc bug itself
(where 'volatile' is lost and some code is moved out of a loop)
then things will really break somewhere odd.

OTOH if it might generate code that reads something twice
you'd have to be unlucky as well.

Oh - and I need to find a newer compiler :-(

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Nick Desaulniers]

On Thu, Jan 23, 2020 at 9:32 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Will Deacon
> > Sent: 23 January 2020 17:17
> >
> > I think it depends how much we care about those older compilers. My series
> > first moves it to "Good luck mate, you're on your own" and then follows up

I wish the actual warning was worded that way. :P

> > with a "Let me take that off you it's sharp".

> Oh - and I need to find a newer compiler :-(

What distro are you using? Does it have a package for a newer
compiler?  I'm honestly curious about what policies if any the kernel
has for supporting developer's toolchains from their distributions.
(ie. Arnd usually has pretty good stats what distro's use which
version of GCC and are still supported; Do we strive to not break
them? Is asking kernel devs to compile their own toolchain too much to
ask?  Is it still if they're using really old distro's/toolchains that
we don't want to support?  Do we survey kernel devs about what they're
using?).  Apologies if this is already documented somewhere, but if
not I'd eventually like to brainstorm and write it down somewhere in
the tree.  Documentation/process/changes.rst doesn't really answer the
above questions, I think.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Arvind Sankar]

On Thu, Jan 23, 2020 at 10:45:08AM -0800, Nick Desaulniers wrote:
> On Thu, Jan 23, 2020 at 9:32 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Will Deacon
> > > Sent: 23 January 2020 17:17
> > >
> > > I think it depends how much we care about those older compilers. My series
> > > first moves it to "Good luck mate, you're on your own" and then follows up
> 
> I wish the actual warning was worded that way. :P
> 
> > > with a "Let me take that off you it's sharp".
> 
> > Oh - and I need to find a newer compiler :-(
> 
> What distro are you using? Does it have a package for a newer
> compiler?  I'm honestly curious about what policies if any the kernel
> has for supporting developer's toolchains from their distributions.
> (ie. Arnd usually has pretty good stats what distro's use which
> version of GCC and are still supported; Do we strive to not break
> them? Is asking kernel devs to compile their own toolchain too much to
> ask?  Is it still if they're using really old distro's/toolchains that
> we don't want to support?  Do we survey kernel devs about what they're
> using?).  Apologies if this is already documented somewhere, but if
> not I'd eventually like to brainstorm and write it down somewhere in
> the tree.  Documentation/process/changes.rst doesn't really answer the
> above questions, I think.
> 
> -- 
> Thanks,
> ~Nick Desaulniers

Reposting Arnd's link
https://www.spinics.net/lists/linux-kbuild/msg23648.html

Seems like there is nothing still on versions between 4.6 and 4.8. It
might be useful to put a list of distro's that are running the current
minimum supported toolchain versions in process/changes.rst?

I think the issue is not just kernel devs. Users may need to compile a
custom kernel or at least build a module.

RE: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[David Laight]

From: Arvind Sankar
> Sent: 23 January 2020 19:01
...
> > > Oh - and I need to find a newer compiler :-(
> >
> > What distro are you using? Does it have a package for a newer
> > compiler?  I'm honestly curious about what policies if any the kernel
> > has for supporting developer's toolchains from their distributions.

Like many developers I get lazy with upgrades of some systems.
So my main Linux 'desktop' system is running Ubuntu 13.04 userspace
with a very recent kernel.
So it has gcc 4.7.3 as default and a gcc 4.8.1 installed (probably not mature
enough to be safe!).
(I've 2 other i7 systems running Ubuntu 18.04.0x with the distro kernels.)
...
> I think the issue is not just kernel devs. Users may need to compile a
> custom kernel or at least build a module.

If they are just building a module they'll be using the kernel headers
that go with the distro kernel - and the module sources will need to
match. So the distro's gcc should match.

Much more interesting is the gcc compiler we use to generate the
'binary blob' for our 'out of tree' drivers.
These get compiled (without any of the kernel headers) on the
same system as we build the userspace programs on.
To get libc and (especially) libc++ compatibility this has to be
a very old system - as old as the oldest system any of our
customers are likely to use.
In practise this (now) means something running a 2.6.32 era
kernel (or slightly earlier).
So is a debian gcc 4.3.2-1.1 compiler.
I think we've finally persuaded all our customers to upgrade from 2.6.18.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Qais Yousef]

On 01/23/20 14:01, Arvind Sankar wrote:
> On Thu, Jan 23, 2020 at 10:45:08AM -0800, Nick Desaulniers wrote:
> > On Thu, Jan 23, 2020 at 9:32 AM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Will Deacon
> > > > Sent: 23 January 2020 17:17
> > > >
> > > > I think it depends how much we care about those older compilers. My series
> > > > first moves it to "Good luck mate, you're on your own" and then follows up
> > 
> > I wish the actual warning was worded that way. :P
> > 
> > > > with a "Let me take that off you it's sharp".
> > 
> > > Oh - and I need to find a newer compiler :-(
> > 
> > What distro are you using? Does it have a package for a newer
> > compiler?  I'm honestly curious about what policies if any the kernel
> > has for supporting developer's toolchains from their distributions.
> > (ie. Arnd usually has pretty good stats what distro's use which
> > version of GCC and are still supported; Do we strive to not break
> > them? Is asking kernel devs to compile their own toolchain too much to
> > ask?  Is it still if they're using really old distro's/toolchains that
> > we don't want to support?  Do we survey kernel devs about what they're
> > using?).  Apologies if this is already documented somewhere, but if
> > not I'd eventually like to brainstorm and write it down somewhere in
> > the tree.  Documentation/process/changes.rst doesn't really answer the
> > above questions, I think.
> > 
> > -- 
> > Thanks,
> > ~Nick Desaulniers
> 
> Reposting Arnd's link
> https://www.spinics.net/lists/linux-kbuild/msg23648.html

This list seems to be x86 centric? I remember when the switch to GCC 4.6
happened a couple or more archs had to be dropped because they lacked a newer
compiler.

So popular archs would probably have moved quickly, but 'niche' ones might
still be catching up at a slower pace.

--
Qais Yousef

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Arnd Bergmann]

On Sun, Jan 26, 2020 at 2:10 AM Qais Yousef <qais.yousef@arm.com> wrote:
> On 01/23/20 14:01, Arvind Sankar wrote:
> > On Thu, Jan 23, 2020 at 10:45:08AM -0800, Nick Desaulniers wrote:
> > > On Thu, Jan 23, 2020 at 9:32 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > Reposting Arnd's link
> > https://www.spinics.net/lists/linux-kbuild/msg23648.html
>
> This list seems to be x86 centric? I remember when the switch to GCC 4.6
> happened a couple or more archs had to be dropped because they lacked a newer
> compiler.

There are two architectures that already had problems last time:

- unicore32 never had any compiler that shipped with sources, only an ancient
  set of gcc binaries that already had problems building the kernel during the
  move to gcc-4.6. The maintainer said he'd work on providing support for
  modern gcc or clang, but I don't think anything came out of that.

- hexagon had an unmaintained gcc-4.5 port, but internally Qualcomm were
  already using clang to build their kernels, which should now work with the
  upstream version. I don't think there are any plans to have a more modern
  gcc.

Everything else works with mainline gcc now, openrisc and csky were the
last to get added in gcc-9.

Some of the older sub-targets (armv3, s390-g6, powerpcspe) are removed
in gcc-9, but these have a few more years before we need to worry about
them.

     Arnd

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Linus Torvalds]

On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
>
> This is version two of the patches I previously posted as an RFC here:

Looks fine to me, as far as I can tell,

              Linus

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Peter Zijlstra]

On Thu, Jan 23, 2020 at 09:59:03AM -0800, Linus Torvalds wrote:
> On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
> >
> > This is version two of the patches I previously posted as an RFC here:
> 
> Looks fine to me, as far as I can tell,

Awesome, I've picked them up with a target for tip/locking/core.

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Peter Zijlstra]

On Fri, Jan 24, 2020 at 09:33:07AM +0100, Peter Zijlstra wrote:
> On Thu, Jan 23, 2020 at 09:59:03AM -0800, Linus Torvalds wrote:
> > On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
> > >
> > > This is version two of the patches I previously posted as an RFC here:
> > 
> > Looks fine to me, as far as I can tell,
> 
> Awesome, I've picked them up with a target for tip/locking/core.

FWIW, I have the following merge resolution against locking/kcsan.

---

diff --cc include/linux/compiler.h
index 8c0beb10c1dd,994c35638584..000000000000
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@@ -177,28 -177,69 +177,74 @@@ void ftrace_likely_update(struct ftrace
  # define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__)
  #endif
  
- #include <uapi/linux/types.h>
- #include <linux/kcsan-checks.h>
+ /*
+  * Prevent the compiler from merging or refetching reads or writes. The
+  * compiler is also forbidden from reordering successive instances of
+  * READ_ONCE and WRITE_ONCE, but only when the compiler is aware of some
+  * particular ordering. One way to make the compiler aware of ordering is to
+  * put the two invocations of READ_ONCE or WRITE_ONCE in different C
+  * statements.
+  *
+  * These two macros will also work on aggregate data types like structs or
+  * unions.
+  *
+  * Their two major use cases are: (1) Mediating communication between
+  * process-level code and irq/NMI handlers, all running on the same CPU,
+  * and (2) Ensuring that the compiler does not fold, spindle, or otherwise
+  * mutilate accesses that either do not require ordering or that interact
+  * with an explicit memory barrier or atomic instruction that provides the
+  * required ordering.
+  */
+ #include <asm/barrier.h>
+ #include <linux/kasan-checks.h>
+ 
+ /*
+  * Use __READ_ONCE() instead of READ_ONCE() if you do not require any
+  * atomicity or dependency ordering guarantees. Note that this may result
+  * in tears!
+  */
+ #define __READ_ONCE(x)	(*(const volatile __unqual_scalar_typeof(x) *)&(x))
+ 
+ #define __READ_ONCE_SCALAR(x)						\
+ ({									\
+ 	__unqual_scalar_typeof(x) __x = __READ_ONCE(x);			\
+ 	smp_read_barrier_depends();					\
+ 	(typeof(x))__x;							\
+ })
  
- #define __READ_ONCE_SIZE						\
+ /*
+  * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
+  * to hide memory access from KASAN.
+  */
+ #define READ_ONCE_NOCHECK(x)						\
  ({									\
- 	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();						\
- 	}								\
+ 	compiletime_assert_rwonce_type(x);				\
+ 	__READ_ONCE_SCALAR(x);						\
+ })
+ 
 -#define READ_ONCE(x)	READ_ONCE_NOCHECK(x)
++#define READ_ONCE(x)					\
++({							\
++	kcsan_check_atomic_read(&(x), sizeof(x));	\
++	READ_ONCE_NOCHECK(x);				\
 +})
  
+ #define __WRITE_ONCE(x, val)				\
+ do {							\
+ 	*(volatile typeof(x) *)&(x) = (val);		\
+ } while (0)
+ 
+ #define WRITE_ONCE(x, val)				\
+ do {							\
+ 	compiletime_assert_rwonce_type(x);		\
++	kcsan_check_atomic_write(&(x), sizeof(x));	\
+ 	__WRITE_ONCE(x, val);				\
+ } while (0)
+ 
  #ifdef CONFIG_KASAN
  /*
 - * We can't declare function 'inline' because __no_sanitize_address conflicts
 + * 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
 - *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
++ *	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
@@@ -207,97 -247,6 +253,24 @@@
  # define __no_kasan_or_inline __always_inline
  #endif
  
 +#define __no_kcsan __no_sanitize_thread
 +#ifdef __SANITIZE_THREAD__
 +/*
 + * Rely on __SANITIZE_THREAD__ instead of CONFIG_KCSAN, to avoid not inlining in
 + * compilation units where instrumentation is disabled. The attribute 'noinline'
 + * is required for older compilers, where implicit inlining of very small
 + * functions renders __no_sanitize_thread ineffective.
 + */
 +# define __no_kcsan_or_inline __no_kcsan noinline notrace __maybe_unused
 +# define __no_sanitize_or_inline __no_kcsan_or_inline
 +#else
 +# define __no_kcsan_or_inline __always_inline
 +#endif
 +
 +#ifndef __no_sanitize_or_inline
 +#define __no_sanitize_or_inline __always_inline
 +#endif
 +
- static __no_kcsan_or_inline
- void __read_once_size(const volatile void *p, void *res, int size)
- {
- 	kcsan_check_atomic_read(p, size);
- 	__READ_ONCE_SIZE;
- }
- 
- static __no_sanitize_or_inline
- void __read_once_size_nocheck(const volatile void *p, void *res, int size)
- {
- 	__READ_ONCE_SIZE;
- }
- 
- static __no_kcsan_or_inline
- void __write_once_size(volatile void *p, void *res, int size)
- {
- 	kcsan_check_atomic_write(p, 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
-  * READ_ONCE and WRITE_ONCE, but only when the compiler is aware of some
-  * particular ordering. One way to make the compiler aware of ordering is to
-  * put the two invocations of READ_ONCE or WRITE_ONCE in different C
-  * 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.
-  *
-  * Their two major use cases are: (1) Mediating communication between
-  * process-level code and irq/NMI handlers, all running on the same CPU,
-  * and (2) Ensuring that the compiler does not fold, spindle, or otherwise
-  * mutilate accesses that either do not require ordering or that interact
-  * with an explicit memory barrier or atomic instruction that provides the
-  * required ordering.
-  */
- #include <asm/barrier.h>
- #include <linux/kasan-checks.h>
- 
- #define __READ_ONCE(x, check)						\
- ({									\
- 	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;							\
- })
- #define READ_ONCE(x) __READ_ONCE(x, 1)
- 
- /*
-  * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
-  * to hide memory access from KASAN.
-  */
- #define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
- 
  static __no_kasan_or_inline
  unsigned long read_word_at_a_time(const void *addr)
  {
@@@ -305,34 -254,6 +278,26 @@@
  	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;					\
- })
- 
 +#include <linux/kcsan.h>
 +
 +/*
 + * data_race(): macro to document that accesses in an expression may conflict with
 + * other concurrent accesses resulting in data races, but the resulting
 + * behaviour is deemed safe regardless.
 + *
 + * This macro *does not* affect normal code generation, but is a hint to tooling
 + * that data races here should be ignored.
 + */
 +#define data_race(expr)                                                        \
 +	({                                                                     \
 +		typeof(({ expr; })) __val;                                     \
 +		kcsan_nestable_atomic_begin();                                 \
 +		__val = ({ expr; });                                           \
 +		kcsan_nestable_atomic_end();                                   \
 +		__val;                                                         \
 +	})
 +#else
 +
  #endif /* __KERNEL__ */
  
  /*

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Masahiro Yamada]

On Fri, Jan 24, 2020 at 5:33 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jan 23, 2020 at 09:59:03AM -0800, Linus Torvalds wrote:
> > On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
> > >
> > > This is version two of the patches I previously posted as an RFC here:
> >
> > Looks fine to me, as far as I can tell,
>
> Awesome, I've picked them up with a target for tip/locking/core.

Were they really picked up?

The MW is closed now, but I do not them in Linus' tree.
I do not see them even in linux-next.




--
Best Regards

Masahiro Yamada

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[Will Deacon]

On Mon, Feb 10, 2020 at 06:50:53PM +0900, Masahiro Yamada wrote:
> On Fri, Jan 24, 2020 at 5:33 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Jan 23, 2020 at 09:59:03AM -0800, Linus Torvalds wrote:
> > > On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@kernel.org> wrote:
> > > >
> > > > This is version two of the patches I previously posted as an RFC here:
> > >
> > > Looks fine to me, as far as I can tell,
> >
> > Awesome, I've picked them up with a target for tip/locking/core.
> 
> Were they really picked up?
> 
> The MW is closed now, but I do not them in Linus' tree.
> I do not see them even in linux-next.

We ended up running into build issues with m68k which took quite a bit of
effort to fix, so that meant we missed the merge window. It also seems that
we've now run into similar looking issues for sparc32 :(

Will

Re: [PATCH v2 00/10] Rework READ_ONCE() to improve codegen

[David Howells]

Of course, if you're feeling adventurous and willing to at least entertain the
mere speculation of switching the kernel source to C++, we could then use
inline template functions:

	template <typename T>
	static inline T __READ_ONCE(T &var)
	{
		T val = *(const volatile T *)&var;
		return val;
	}

	template <typename T>
	static inline T READ_ONCE(T &var)
	{
		T val;
		compiletime_assert_rwonce_type(var);
		val = __READ_ONCE(var);
		smp_read_barrier_depends();
		return val;
	}

Of course, that would mean using C++...

David