[PATCHv2] locking/atomics: build atomic headers as required

[PATCHv2] locking/atomics: build atomic headers as required

[Mark Rutland]

Andrew and Ingo report that the check-atomics.sh script is simply too
slow to run for every kernel build, and it's impractical to make it
faster without rewriting it in something other than shell.

Rather than committing the generated headers, let's regenerate these
as-required, if we change the data or scripts used to generate the
atomic headers, or when building from a pristine tree.

That ensures they're always up-to-date, allows them to be built in
parallel, and avoid redundant rebuilds, which is a 2-8s saving per
incremental build. Since the results are not committed, it's very
obvious that they should not be modified directly. If we need to
generate more headers in future, it's easy to extend Makefile.genheader
to permit this.

I've verified that this works in the cases we previously had issues with
(out-of-tree builds and where scripts have no execute permissions), and
have tested these cases for both x86_64 and arm64.

The diffstat looks nice, at least...

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-kernel@vger.kernel.org
---
 Kbuild                                    |   18 +-
 Makefile                                  |    8 +-
 arch/arm64/include/asm/atomic.h           |    2 +-
 arch/x86/include/asm/atomic.h             |    2 +-
 include/asm-generic/atomic-instrumented.h | 1787 ----------------------
 include/asm-generic/atomic-long.h         | 1012 -------------
 include/linux/atomic-fallback.h           | 2294 -----------------------------
 include/linux/atomic.h                    |    4 +-
 scripts/Makefile.genheader                |   26 +
 scripts/atomic/check-atomics.sh           |   19 -
 10 files changed, 38 insertions(+), 5134 deletions(-)
 delete mode 100644 include/asm-generic/atomic-instrumented.h
 delete mode 100644 include/asm-generic/atomic-long.h
 delete mode 100644 include/linux/atomic-fallback.h
 create mode 100644 scripts/Makefile.genheader
 delete mode 100755 scripts/atomic/check-atomics.sh

Hi Ingo,

Would you be happy to pick this up? Andrew and Peter seemed happy with
v1 [1].

To save on ~5100 lines of preimage for the deleted files, I generated
this patch with `git format-patch -D`, which git am will complain about.

I've pushed this to my atomics/regenerate branch [2], based on the tip
locking/core branch, in the hope you're happy to pick it from there. If
you can let me know your preference I can resend this however you
prefer.

Thanks,
Mark.

Since v1 [1]:
* Clarify commit message w.r.t. when headers get regenerated
* Add missing dependencies to Makefile.genheader
* Minor cleanups for Makefile.genheader

[1] https://lkml.kernel.org/r/20181123153321.8561-1-mark.rutland@arm.com
[2] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git atomics/regenerate

diff --git a/Kbuild b/Kbuild
index 780048056ac5..005304205482 100644
--- a/Kbuild
+++ b/Kbuild
@@ -6,8 +6,7 @@
 # 2) Generate timeconst.h
 # 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
 # 4) Check for missing system calls
-# 5) check atomics headers are up-to-date
-# 6) Generate constants.py (may need bounds.h)
+# 5) Generate constants.py (may need bounds.h)
 
 #####
 # 1) Generate bounds.h
@@ -73,20 +72,7 @@ missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
 	$(call cmd,syscalls)
 
 #####
-# 5) Check atomic headers are up-to-date
-#
-
-always += old-atomics
-targets += old-atomics
-
-quiet_cmd_atomics = CALL    $<
-      cmd_atomics = $(CONFIG_SHELL) $<
-
-old-atomics: scripts/atomic/check-atomics.sh FORCE
-	$(call cmd,atomics)
-
-#####
-# 6) Generate constants for Python GDB integration
+# 5) Generate constants for Python GDB integration
 #
 
 extra-$(CONFIG_GDB_SCRIPTS) += build_constants_py
diff --git a/Makefile b/Makefile
index 6c40d547513c..fd871d2c2f54 100644
--- a/Makefile
+++ b/Makefile
@@ -224,7 +224,7 @@ clean-targets := %clean mrproper cleandocs
 no-dot-config-targets := $(clean-targets) \
 			 cscope gtags TAGS tags help% %docs check% coccicheck \
 			 $(version_h) headers_% archheaders archscripts \
-			 %asm-generic kernelversion %src-pkg
+			 %asm-generic genheader kernelversion %src-pkg
 no-sync-config-targets := $(no-dot-config-targets) install %install \
 			   kernelrelease
 
@@ -1089,7 +1089,7 @@ endif
 # prepare2 creates a makefile if using a separate output directory.
 # From this point forward, .config has been reprocessed, so any rules
 # that need to depend on updated CONFIG_* values can be checked here.
-prepare2: prepare3 outputmakefile asm-generic
+prepare2: prepare3 outputmakefile asm-generic genheader
 
 prepare1: prepare2 $(version_h) $(autoksyms_h) include/generated/utsrelease.h
 	$(cmd_crmodverdir)
@@ -1113,6 +1113,10 @@ uapi-asm-generic:
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
 	            src=uapi/asm obj=arch/$(SRCARCH)/include/generated/uapi/asm
 
+PHONY += genheader
+genheader:
+	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.genheader obj=include/generated
+
 PHONY += prepare-objtool
 prepare-objtool: $(objtool_target)
 
diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index 1f4e9ee641c9..57c73cde3a51 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -166,7 +166,7 @@
 
 #define arch_atomic64_dec_if_positive		arch_atomic64_dec_if_positive
 
-#include <asm-generic/atomic-instrumented.h>
+#include <generated/atomic-instrumented.h>
 
 #endif
 #endif
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index ea3d95275b43..d1e3aca236e2 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -262,6 +262,6 @@ static inline int arch_atomic_fetch_xor(int i, atomic_t *v)
 # include <asm/atomic64_64.h>
 #endif
 
-#include <asm-generic/atomic-instrumented.h>
+#include <generated/atomic-instrumented.h>
 
 #endif /* _ASM_X86_ATOMIC_H */
diff --git a/include/asm-generic/atomic-instrumented.h b/include/asm-generic/atomic-instrumented.h
deleted file mode 100644
index b8f5b35216e1..000000000000
diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
deleted file mode 100644
index a833d385a70b..000000000000
diff --git a/include/linux/atomic-fallback.h b/include/linux/atomic-fallback.h
deleted file mode 100644
index 1c02c0112fbb..000000000000
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 4c0d009a46f0..bea7ceec44af 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -71,8 +71,8 @@
 	__ret;								\
 })
 
-#include <linux/atomic-fallback.h>
+#include <generated/atomic-fallback.h>
 
-#include <asm-generic/atomic-long.h>
+#include <generated/atomic-long.h>
 
 #endif /* _LINUX_ATOMIC_H */
diff --git a/scripts/Makefile.genheader b/scripts/Makefile.genheader
new file mode 100644
index 000000000000..03aac5628f69
--- /dev/null
+++ b/scripts/Makefile.genheader
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Generate common headers under include/generated/
+
+include scripts/Kbuild.include
+
+.PHONY: all
+
+targets += atomic-fallback.h
+targets += atomic-instrumented.h
+targets += atomic-long.h
+
+all: $(addprefix $(obj)/,$(targets))
+
+atomic-dir := $(srctree)/scripts/atomic
+atomic-tbl := $(atomic-dir)/atomics.tbl
+
+atomic-deps := $(atomic-tbl) $(atomic-dir)/atomic-tbl.sh \
+	       $(atomic-dir)/fallbacks/*
+
+quiet_cmd_genatomic = GENHDR  $@
+cmd_genatomic = $(CONFIG_SHELL) $< $(atomic-tbl) > $@
+
+$(obj)/atomic-%.h: $(atomic-dir)/gen-atomic-%.sh $(atomic-deps)
+	$(call if_changed,genatomic)
+
diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh
deleted file mode 100755
index c30101cddf2d..000000000000
-- 
2.11.0

[GIT PULL] atomic regeneration (was "[PATCHv2] locking/atomics: build atomic headers as required")

[Mark Rutland]

Hi Ingo,

Peter suggested you'd prefer a pull request for the atomic scripting rework, so
hopefully this is sufficient. :)

The rework means that the atomics are now rebuilt as-required. The headers are
built in parallel, along with other early build steps, so this isn't as
noticeable on a reasonably powerful machine. Once the headers are built,
they'll only be regenerated when their dependencies change, and can be cleaned
with a mrproper.

Thanks,
Mark.

The following changes since commit bdf37b4dd35d2517cadc10735cd33022da7df133:

  locking/atomics: Fix out-of-tree build (2018-11-09 09:06:01 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git atomics/regenerate

for you to fetch changes up to 12eea59cbedcd46fe8b526a16a7f429c8548187d:

  locking/atomics: build atomic headers as required (2018-11-27 13:25:09 +0000)

----------------------------------------------------------------
Mark Rutland (1):
      locking/atomics: build atomic headers as required

 Kbuild                                    |   18 +-
 Makefile                                  |    8 +-
 arch/arm64/include/asm/atomic.h           |    2 +-
 arch/x86/include/asm/atomic.h             |    2 +-
 include/asm-generic/atomic-instrumented.h | 1787 ----------------------
 include/asm-generic/atomic-long.h         | 1012 -------------
 include/linux/atomic-fallback.h           | 2294 -----------------------------
 include/linux/atomic.h                    |    4 +-
 scripts/Makefile.genheader                |   26 +
 scripts/atomic/check-atomics.sh           |   19 -
 10 files changed, 38 insertions(+), 5134 deletions(-)
 delete mode 100644 include/asm-generic/atomic-instrumented.h
 delete mode 100644 include/asm-generic/atomic-long.h
 delete mode 100644 include/linux/atomic-fallback.h
 create mode 100644 scripts/Makefile.genheader
 delete mode 100755 scripts/atomic/check-atomics.sh

Re: [PATCHv2] locking/atomics: build atomic headers as required

[Ingo Molnar]

* Mark Rutland <mark.rutland@arm.com> wrote:

> Andrew and Ingo report that the check-atomics.sh script is simply too
> slow to run for every kernel build, and it's impractical to make it
> faster without rewriting it in something other than shell.
> 
> Rather than committing the generated headers, let's regenerate these
> as-required, if we change the data or scripts used to generate the
> atomic headers, or when building from a pristine tree.
> 
> That ensures they're always up-to-date, allows them to be built in
> parallel, and avoid redundant rebuilds, which is a 2-8s saving per
> incremental build. Since the results are not committed, it's very
> obvious that they should not be modified directly. If we need to
> generate more headers in future, it's easy to extend Makefile.genheader
> to permit this.
> 
> I've verified that this works in the cases we previously had issues with
> (out-of-tree builds and where scripts have no execute permissions), and
> have tested these cases for both x86_64 and arm64.
> 
> The diffstat looks nice, at least...
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  Kbuild                                    |   18 +-
>  Makefile                                  |    8 +-
>  arch/arm64/include/asm/atomic.h           |    2 +-
>  arch/x86/include/asm/atomic.h             |    2 +-
>  include/asm-generic/atomic-instrumented.h | 1787 ----------------------
>  include/asm-generic/atomic-long.h         | 1012 -------------
>  include/linux/atomic-fallback.h           | 2294 -----------------------------
>  include/linux/atomic.h                    |    4 +-
>  scripts/Makefile.genheader                |   26 +
>  scripts/atomic/check-atomics.sh           |   19 -
>  10 files changed, 38 insertions(+), 5134 deletions(-)
>  delete mode 100644 include/asm-generic/atomic-instrumented.h
>  delete mode 100644 include/asm-generic/atomic-long.h
>  delete mode 100644 include/linux/atomic-fallback.h
>  create mode 100644 scripts/Makefile.genheader
>  delete mode 100755 scripts/atomic/check-atomics.sh

So these 'automatically generated' headers are actual and important code, 
and I think it's bad practice to remove these from the git grep search 
space and history as well.

Really, this whole notion of auto-generating the headers should be 
implemented correctly, instead of working around deficiencies in a 
short-term fashion that introduces other deficiencies.

I never got any replies to my previous comments about this:

  <20181128083057.GA7879@gmail.com>

Did I miss some mails of yours perhaps?

Thanks,

	Ingo

Re: [PATCHv2] locking/atomics: build atomic headers as required

[Mark Rutland]

On Thu, Dec 06, 2018 at 05:13:44PM +0100, Ingo Molnar wrote:
> 
> * Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > Andrew and Ingo report that the check-atomics.sh script is simply too
> > slow to run for every kernel build, and it's impractical to make it
> > faster without rewriting it in something other than shell.
> > 
> > Rather than committing the generated headers, let's regenerate these
> > as-required, if we change the data or scripts used to generate the
> > atomic headers, or when building from a pristine tree.
> > 
> > That ensures they're always up-to-date, allows them to be built in
> > parallel, and avoid redundant rebuilds, which is a 2-8s saving per
> > incremental build. Since the results are not committed, it's very
> > obvious that they should not be modified directly. If we need to
> > generate more headers in future, it's easy to extend Makefile.genheader
> > to permit this.
> > 
> > I've verified that this works in the cases we previously had issues with
> > (out-of-tree builds and where scripts have no execute permissions), and
> > have tested these cases for both x86_64 and arm64.
> > 
> > The diffstat looks nice, at least...
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  Kbuild                                    |   18 +-
> >  Makefile                                  |    8 +-
> >  arch/arm64/include/asm/atomic.h           |    2 +-
> >  arch/x86/include/asm/atomic.h             |    2 +-
> >  include/asm-generic/atomic-instrumented.h | 1787 ----------------------
> >  include/asm-generic/atomic-long.h         | 1012 -------------
> >  include/linux/atomic-fallback.h           | 2294 -----------------------------
> >  include/linux/atomic.h                    |    4 +-
> >  scripts/Makefile.genheader                |   26 +
> >  scripts/atomic/check-atomics.sh           |   19 -
> >  10 files changed, 38 insertions(+), 5134 deletions(-)
> >  delete mode 100644 include/asm-generic/atomic-instrumented.h
> >  delete mode 100644 include/asm-generic/atomic-long.h
> >  delete mode 100644 include/linux/atomic-fallback.h
> >  create mode 100644 scripts/Makefile.genheader
> >  delete mode 100755 scripts/atomic/check-atomics.sh
> 
> So these 'automatically generated' headers are actual and important code, 
> and I think it's bad practice to remove these from the git grep search 
> space and history as well.

Ok. Let's aim for that, then. :)

> Really, this whole notion of auto-generating the headers should be 
> implemented correctly, instead of working around deficiencies in a 
> short-term fashion that introduces other deficiencies.

I certainly agree.

> I never got any replies to my previous comments about this:
> 
>   <20181128083057.GA7879@gmail.com>
> 
> Did I miss some mails of yours perhaps?

It appears my mailserver dropped that on the floor, as I can't find it
in any of my mailboxes. :(

Sorry about that; I'll try to figure out what happened there.

This patch covers your suggestion of parallelizing things. On my box
with -j64, that saves ~1.5s for a pristine allnoconfig build which
otherwise takes ~15s.

Practically, optimizing the scripts requires moving away from shell.
The cost is all down to sub-shells resulting in fork+exec, but the
scripts would be illegible without this.

One option is to generate a checksum when the headers are generated and
checked in, and check that at build time. I'll look into this tomorrow.

Thanks,
Mark.

Re: [PATCHv2] locking/atomics: build atomic headers as required

[Mark Rutland]

Hi Ingo,

On Thu, Dec 06, 2018 at 05:13:44PM +0100, Ingo Molnar wrote:
> 
> * Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > Andrew and Ingo report that the check-atomics.sh script is simply too
> > slow to run for every kernel build, and it's impractical to make it
> > faster without rewriting it in something other than shell.
> > 
> > Rather than committing the generated headers, let's regenerate these
> > as-required, if we change the data or scripts used to generate the
> > atomic headers, or when building from a pristine tree.
> > 
> > That ensures they're always up-to-date, allows them to be built in
> > parallel, and avoid redundant rebuilds, which is a 2-8s saving per
> > incremental build. Since the results are not committed, it's very
> > obvious that they should not be modified directly. If we need to
> > generate more headers in future, it's easy to extend Makefile.genheader
> > to permit this.

> So these 'automatically generated' headers are actual and important code, 
> and I think it's bad practice to remove these from the git grep search 
> space and history as well.
> 
> Really, this whole notion of auto-generating the headers should be 
> implemented correctly, instead of working around deficiencies in a 
> short-term fashion that introduces other deficiencies.
> 
> I never got any replies to my previous comments about this:
> 
>   <20181128083057.GA7879@gmail.com>

As I've just mentioned in another thread, I've sent out a couple of patches
attempting to address this while leaving the headers checked-in:

https://lkml.kernel.org/r/20181210175035.45096-1-mark.rutland@arm.com

Thanks,
Mark.