| Message ID | 20180904104830.2975-6-mark.rutland@arm.com |
|---|---|
| State | Accepted |
| Commit | 8d32588077bdc390420cfa6946f407033a20d7a8 |
| Headers | show |
| Series | atomics: generate atomic headers / instrument arm64 | expand |
* tip-bot for Mark Rutland <tipbot@zytor.com> wrote: > Commit-ID: 8d32588077bdc390420cfa6946f407033a20d7a8 > Gitweb: https://git.kernel.org/tip/8d32588077bdc390420cfa6946f407033a20d7a8 > Author: Mark Rutland <mark.rutland@arm.com> > AuthorDate: Tue, 4 Sep 2018 11:48:29 +0100 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Thu, 1 Nov 2018 11:01:10 +0100 > > locking/atomics: Check generated headers are up-to-date > > Now that all the generated atomic headers are in place, it would be good > to ensure that: > > a) the headers are up-to-date when scripting changes. > > b) developers don't directly modify the generated headers. > > To ensure both of these properties, let's add a Kbuild step to check > that the generated headers are up-to-date. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: linux-arm-kernel@lists.infradead.org > Cc: catalin.marinas@arm.com > Cc: Will Deacon <will.deacon@arm.com> > Cc: linuxdrivers@attotech.com > Cc: dvyukov@google.com > Cc: Boqun Feng <boqun.feng@gmail.com> > Cc: arnd@arndb.de > Cc: aryabinin@virtuozzo.com > Cc: glider@google.com > Link: http://lkml.kernel.org/r/20180904104830.2975-6-mark.rutland@arm.com > Signed-off-by: Ingo Molnar <mingo@kernel.org> > --- > Kbuild | 18 ++++++++++++++++-- > scripts/atomic/check-atomics.sh | 19 +++++++++++++++++++ > 2 files changed, 35 insertions(+), 2 deletions(-) These scripts are *awfully* slow to be run at every kernel build - even a reasonably fast machine: model name : Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz ... they are adding 3-4 seconds to the build time: [Before]: galatea:~/linux/linux> perf stat --null --repeat 3 make kernel/sched/core.o CALL scripts/checksyscalls.sh DESCEND objtool CALL scripts/checksyscalls.sh DESCEND objtool CALL scripts/checksyscalls.sh DESCEND objtool Performance counter stats for 'make kernel/sched/core.o' (3 runs): 1.201874 +- 0.000371 seconds time elapsed ( +- 0.03% ) [After]: galatea:~/linux/linux> perf stat --null --repeat 3 make kernel/sched/core.o CALL scripts/checksyscalls.sh CALL scripts/atomic/check-atomics.sh DESCEND objtool CALL scripts/checksyscalls.sh CALL scripts/atomic/check-atomics.sh DESCEND objtool CALL scripts/checksyscalls.sh CALL scripts/atomic/check-atomics.sh DESCEND objtool Performance counter stats for 'make kernel/sched/core.o' (3 runs): 4.5987 +- 0.0109 seconds time elapsed ( +- 0.24% ) Could we please get this fixed so that proper dependencies are checked and it's only regenerated when needed? This slowdown makes additive-build kernel development quite painful, as ~5 seconds is in the 'too long' category already, while 1.2 seconds is basically instantaneous. I cannot even imagine the slowdown on a truly slow box where kernel development *has* to be additive. Thanks, Ingo
Hi Ingo, On Wed, Nov 21, 2018 at 09:02:58AM +0100, Ingo Molnar wrote: > > * tip-bot for Mark Rutland <tipbot@zytor.com> wrote: > > > Commit-ID: 8d32588077bdc390420cfa6946f407033a20d7a8 > > Gitweb: https://git.kernel.org/tip/8d32588077bdc390420cfa6946f407033a20d7a8 > > Author: Mark Rutland <mark.rutland@arm.com> > > AuthorDate: Tue, 4 Sep 2018 11:48:29 +0100 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Thu, 1 Nov 2018 11:01:10 +0100 > > > > locking/atomics: Check generated headers are up-to-date > > > > Now that all the generated atomic headers are in place, it would be good > > to ensure that: > > > > a) the headers are up-to-date when scripting changes. > > > > b) developers don't directly modify the generated headers. > > > > To ensure both of these properties, let's add a Kbuild step to check > > that the generated headers are up-to-date. > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: catalin.marinas@arm.com > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: linuxdrivers@attotech.com > > Cc: dvyukov@google.com > > Cc: Boqun Feng <boqun.feng@gmail.com> > > Cc: arnd@arndb.de > > Cc: aryabinin@virtuozzo.com > > Cc: glider@google.com > > Link: http://lkml.kernel.org/r/20180904104830.2975-6-mark.rutland@arm.com > > Signed-off-by: Ingo Molnar <mingo@kernel.org> > > --- > > Kbuild | 18 ++++++++++++++++-- > > scripts/atomic/check-atomics.sh | 19 +++++++++++++++++++ > > 2 files changed, 35 insertions(+), 2 deletions(-) > > These scripts are *awfully* slow to be run at every kernel build - even a > reasonably fast machine: Andrew also reported this, and I'm trying to figure out what to do. I looked into optimizing the scripts, but that turned out to be a dead end. The majority of the cost is in the fork() etc for sub-shells, which the scripts make heavy use of to capture string return values. Trying to avoid sub-shells rendered the scripts unreadable. > Could we please get this fixed so that proper dependencies are checked > and it's only regenerated when needed? This slowdown makes additive-build > kernel development quite painful, as ~5 seconds is in the 'too long' > category already, while 1.2 seconds is basically instantaneous. Just to check, are we happy to eat the full cost for the first build of a pristine tree? One reason we do the check rather than (re-)generating the headers is that Linus requested [1] the generated header be committed so that they show up in git grep, but it looks like he was happy to be flexible on that. If we're happy to not commit in the generated headers, and if we're happy to pay the cost for a pristine tree, that's fairly straightforward to do. Otherwise, this has to be an optional check. Thoughts? Thanks, Mark. [1] https://lkml.kernel.org/r/CA+55aFxjU0op8QLLu0n-RjHBs7gQsLvD8jcyedgw6jeZFN7B+Q@mail.gmail.com
* Mark Rutland <mark.rutland@arm.com> wrote: > > Could we please get this fixed so that proper dependencies are checked > > and it's only regenerated when needed? This slowdown makes additive-build > > kernel development quite painful, as ~5 seconds is in the 'too long' > > category already, while 1.2 seconds is basically instantaneous. > > Just to check, are we happy to eat the full cost for the first build of a > pristine tree? No, not happy to add 3-4 seconds to a full build that usually takes less than 60 seconds. This stuff isn't parallelized nor particularly well optimized it appears. This *must* get faster. > One reason we do the check rather than (re-)generating the headers is > that Linus requested [1] the generated header be committed so that they > show up in git grep, but it looks like he was happy to be flexible on > that. I think the generated headers should be part of the commit space, the grepping is important. > If we're happy to not commit in the generated headers, and if we're happy to > pay the cost for a pristine tree, that's fairly straightforward to do. > Otherwise, this has to be an optional check. Or faster code, or a different concept! Thanks, Ingo
diff --git a/Kbuild b/Kbuild index 005304205482..47c9fe175bd9 100644 --- a/Kbuild +++ b/Kbuild @@ -6,7 +6,8 @@ # 2) Generate timeconst.h # 3) Generate asm-offsets.h (may need bounds.h and timeconst.h) # 4) Check for missing system calls -# 5) Generate constants.py (may need bounds.h) +# 5) check atomics headers are up-to-date +# 6) Generate constants.py (may need bounds.h) ##### # 1) Generate bounds.h @@ -72,7 +73,20 @@ missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE $(call cmd,syscalls) ##### -# 5) Generate constants for Python GDB integration +# 5) Check atomic headers are up-to-date +# + +always += old-atomics +targets += old-atomics + +quiet_cmd_atomics = CALL $< + cmd_atomics = $(CONFIG_SHELL) scripts/atomic/check-atomics.sh + +old-atomics: scripts/atomic/check-atomics.sh FORCE + $(call cmd,atomics) + +##### +# 6) Generate constants for Python GDB integration # extra-$(CONFIG_GDB_SCRIPTS) += build_constants_py diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh new file mode 100755 index 000000000000..c30101cddf2d --- /dev/null +++ b/scripts/atomic/check-atomics.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Check if atomic headers are up-to-date + +ATOMICDIR=$(dirname $0) +ATOMICTBL=${ATOMICDIR}/atomics.tbl +LINUXDIR=${ATOMICDIR}/../.. + +cat <<EOF | +gen-atomic-instrumented.sh asm-generic/atomic-instrumented.h +gen-atomic-long.sh asm-generic/atomic-long.h +gen-atomic-fallback.sh linux/atomic-fallback.h +EOF +while read script header; do + if ! (${ATOMICDIR}/${script} ${ATOMICTBL} | diff - ${LINUXDIR}/include/${header} > /dev/null); then + printf "warning: include/${header} is out-of-date.\n" + fi +done
Now that all the generated atomic headers are in place, it would be good to ensure that: a) the headers are up-to-date when scripting changes. b) developers don't directly modify the generated headers. To ensure both of these properties, let's add a Kbuild step to check that the generated headers are up-to-date. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Boqun Feng <boqun.feng@gmail.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Will Deacon <will.deacon@arm.com> --- Kbuild | 18 ++++++++++++++++-- scripts/atomic/check-atomics.sh | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100755 scripts/atomic/check-atomics.sh -- 2.11.0