linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change
@ 2022-04-26 15:52 Vincent Mailhol
  2022-04-29 17:11 ` Masahiro Yamada
  2022-05-07 13:11 ` [RFC PATCH v2 0/2] call " Vincent Mailhol
  0 siblings, 2 replies; 14+ messages in thread
From: Vincent Mailhol @ 2022-04-26 15:52 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers
  Cc: linux-kernel, linux-kbuild, Arnd Bergmann, Vincent Mailhol

Currently, checksyscalls.sh and check-atomics.sh are executed
unconditionally. Most developers will not modify the files being
checked by those scripts and thus do not need to execute these again
for each iterative make. Change Kbuild target so that those two
scripts get executed only if the prerequisite are modified.

In order to implement this we:

  1. use the if_change macro instead of cmd. c.f. [1]

  2. create two dot files: scripts/.checksyscalls and
  scripts/atomic/.check-atomics to keep track of whether the script
  were already executed or not. Otherwise, the prerequisite would
  always be considered as newer than the target (c.f. output "due to
  target missing" of make V=2).

  3. modify the CLEAN_FILES target of the root Makefile to removed the
  two temporary dot files created in 2.

We also added an additional dependency to include/linux/atomic/* for
check-atomics.sh to make sure that the script gets executed again if
the header are modified. check-atomics.sh already has a dependency
toward include/generated/asm-offsets.h and so no additional
dependencies were added.

[1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Sending this as RFC because I am not an expert of Kbuild. The use of
the dot files was my best shot at tackling this issue. Maybe there is
a smarter way which I just missed?

If I receive no comments for the next two weeks, I will resend this
patch without the RFC tag.
---
 Kbuild   | 14 ++++++++------
 Makefile |  3 ++-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Kbuild b/Kbuild
index fa441b98c9f6..d579f4971aa3 100644
--- a/Kbuild
+++ b/Kbuild
@@ -39,21 +39,23 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
 #####
 # Check for missing system calls
 
-always-y += missing-syscalls
+always-y += scripts/.missing-syscalls
 
 quiet_cmd_syscalls = CALL    $<
       cmd_syscalls = $(CONFIG_SHELL) $< $(CC) $(c_flags) $(missing_syscalls_flags)
 
-missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
-	$(call cmd,syscalls)
+scripts/.missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
+	$(call if_changed,syscalls)
+	@touch $@
 
 #####
 # Check atomic headers are up-to-date
 
-always-y += old-atomics
+always-y += scripts/atomic/.old-atomics
 
 quiet_cmd_atomics = CALL    $<
       cmd_atomics = $(CONFIG_SHELL) $<
 
-old-atomics: scripts/atomic/check-atomics.sh FORCE
-	$(call cmd,atomics)
+scripts/atomic/.old-atomics: scripts/atomic/check-atomics.sh $(wildcard include/linux/atomic/*) FORCE
+	$(call if_changed,atomics)
+	@touch $@
diff --git a/Makefile b/Makefile
index fa5112a0ec1b..b18af9d4248a 100644
--- a/Makefile
+++ b/Makefile
@@ -1483,7 +1483,8 @@ endif # CONFIG_MODULES
 # Directories & files removed with 'make clean'
 CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
 	       modules.builtin modules.builtin.modinfo modules.nsdeps \
-	       compile_commands.json .thinlto-cache
+	       compile_commands.json .thinlto-cache \
+	       scripts/.missing-syscalls scripts/atomic/.old-atomics
 
 # Directories & files removed with 'make mrproper'
 MRPROPER_FILES += include/config include/generated          \
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change
  2022-04-26 15:52 [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change Vincent Mailhol
@ 2022-04-29 17:11 ` Masahiro Yamada
  2022-05-07 11:43   ` Vincent MAILHOL
  2022-05-07 13:11 ` [RFC PATCH v2 0/2] call " Vincent Mailhol
  1 sibling, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2022-04-29 17:11 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List,
	Linux Kbuild mailing list, Arnd Bergmann

On Wed, Apr 27, 2022 at 12:52 AM Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
>
> Currently, checksyscalls.sh and check-atomics.sh are executed
> unconditionally. Most developers will not modify the files being
> checked by those scripts and thus do not need to execute these again
> for each iterative make. Change Kbuild target so that those two
> scripts get executed only if the prerequisite are modified.
>
> In order to implement this we:
>
>   1. use the if_change macro instead of cmd. c.f. [1]
>
>   2. create two dot files: scripts/.checksyscalls and
>   scripts/atomic/.check-atomics to keep track of whether the script
>   were already executed or not. Otherwise, the prerequisite would
>   always be considered as newer than the target (c.f. output "due to
>   target missing" of make V=2).
>
>   3. modify the CLEAN_FILES target of the root Makefile to removed the
>   two temporary dot files created in 2.
>
> We also added an additional dependency to include/linux/atomic/* for
> check-atomics.sh to make sure that the script gets executed again if
> the header are modified. check-atomics.sh already has a dependency
> toward include/generated/asm-offsets.h and so no additional
> dependencies were added.
>
> [1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection
>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> Sending this as RFC because I am not an expert of Kbuild. The use of
> the dot files was my best shot at tackling this issue. Maybe there is
> a smarter way which I just missed?
>
> If I receive no comments for the next two weeks, I will resend this
> patch without the RFC tag.
> ---
>  Kbuild   | 14 ++++++++------
>  Makefile |  3 ++-
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/Kbuild b/Kbuild
> index fa441b98c9f6..d579f4971aa3 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -39,21 +39,23 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
>  #####
>  # Check for missing system calls
>
> -always-y += missing-syscalls
> +always-y += scripts/.missing-syscalls
>
>  quiet_cmd_syscalls = CALL    $<
>        cmd_syscalls = $(CONFIG_SHELL) $< $(CC) $(c_flags) $(missing_syscalls_flags)
>
> -missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> -       $(call cmd,syscalls)
> +scripts/.missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> +       $(call if_changed,syscalls)
> +       @touch $@

I am not sure about this hunk.

Avoiding the needless preprocess is good.
But, it is better to display a warning somewhere (maybe 'all' target)
until the required syscall is implemented.

Also, you need to check the timestamp of syscall_32.tbl.
When it is updated (i.e. when a new syscall is added),
this check must be re-run.


>  #####
>  # Check atomic headers are up-to-date
>
> -always-y += old-atomics
> +always-y += scripts/atomic/.old-atomics
>
>  quiet_cmd_atomics = CALL    $<
>        cmd_atomics = $(CONFIG_SHELL) $<
>
> -old-atomics: scripts/atomic/check-atomics.sh FORCE
> -       $(call cmd,atomics)
> +scripts/atomic/.old-atomics: scripts/atomic/check-atomics.sh $(wildcard include/linux/atomic/*) FORCE
> +       $(call if_changed,atomics)
> +       @touch $@


Presumably, this is wrong.
If the header is manually edited, Kbuild must stop the build.

This change just lets it keep going, and
what is worse, the warning is completely silenced
second time.







> diff --git a/Makefile b/Makefile
> index fa5112a0ec1b..b18af9d4248a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1483,7 +1483,8 @@ endif # CONFIG_MODULES
>  # Directories & files removed with 'make clean'
>  CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
>                modules.builtin modules.builtin.modinfo modules.nsdeps \
> -              compile_commands.json .thinlto-cache
> +              compile_commands.json .thinlto-cache \
> +              scripts/.missing-syscalls scripts/atomic/.old-atomics
>
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_FILES += include/config include/generated          \
> --
> 2.35.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change
  2022-04-29 17:11 ` Masahiro Yamada
@ 2022-05-07 11:43   ` Vincent MAILHOL
  0 siblings, 0 replies; 14+ messages in thread
From: Vincent MAILHOL @ 2022-05-07 11:43 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List,
	Linux Kbuild mailing list, Arnd Bergmann

On Sat. 30 Apr 2022 at 02:11, Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Wed, Apr 27, 2022 at 12:52 AM Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
> >
> > Currently, checksyscalls.sh and check-atomics.sh are executed
> > unconditionally. Most developers will not modify the files being
> > checked by those scripts and thus do not need to execute these again
> > for each iterative make. Change Kbuild target so that those two
> > scripts get executed only if the prerequisite are modified.
> >
> > In order to implement this we:
> >
> >   1. use the if_change macro instead of cmd. c.f. [1]
> >
> >   2. create two dot files: scripts/.checksyscalls and
> >   scripts/atomic/.check-atomics to keep track of whether the script
> >   were already executed or not. Otherwise, the prerequisite would
> >   always be considered as newer than the target (c.f. output "due to
> >   target missing" of make V=2).
> >
> >   3. modify the CLEAN_FILES target of the root Makefile to removed the
> >   two temporary dot files created in 2.
> >
> > We also added an additional dependency to include/linux/atomic/* for
> > check-atomics.sh to make sure that the script gets executed again if
> > the header are modified. check-atomics.sh already has a dependency
> > toward include/generated/asm-offsets.h and so no additional
> > dependencies were added.
> >
> > [1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection
> >
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > ---
> > Sending this as RFC because I am not an expert of Kbuild. The use of
> > the dot files was my best shot at tackling this issue. Maybe there is
> > a smarter way which I just missed?
> >
> > If I receive no comments for the next two weeks, I will resend this
> > patch without the RFC tag.
> > ---
> >  Kbuild   | 14 ++++++++------
> >  Makefile |  3 ++-
> >  2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/Kbuild b/Kbuild
> > index fa441b98c9f6..d579f4971aa3 100644
> > --- a/Kbuild
> > +++ b/Kbuild
> > @@ -39,21 +39,23 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
> >  #####
> >  # Check for missing system calls
> >
> > -always-y += missing-syscalls
> > +always-y += scripts/.missing-syscalls
> >
> >  quiet_cmd_syscalls = CALL    $<
> >        cmd_syscalls = $(CONFIG_SHELL) $< $(CC) $(c_flags) $(missing_syscalls_flags)
> >
> > -missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> > -       $(call cmd,syscalls)
> > +scripts/.missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> > +       $(call if_changed,syscalls)
> > +       @touch $@
>
> I am not sure about this hunk.
>
> Avoiding the needless preprocess is good.
> But, it is better to display a warning somewhere (maybe 'all' target)
> until the required syscall is implemented.

The classic way to achieve is to raise an error, not a warning.

This should have been achievable through CONFIG_WERROR, however,
the below commit deactivated it with -Wno-error (rationale of why
this was an issue is missing).

commit 20fbb11fe4ea ("don't make the syscall checking produce errors
from warnings")

If we just warn, I am not sure how to emit a warning each time
until it gets fixed. The normal behaviour everywhere else is to
only warn during the first build and be quiet (unless
dependencies change).

> Also, you need to check the timestamp of syscall_32.tbl.
> When it is updated (i.e. when a new syscall is added),
> this check must be re-run.

ACK

Regardless of the above discussion, I think I will give up on
checksyscalls.sh, at least for the moment. The patch assumed that
checksyscalls.sh would only be called from ./Kbuild. It appears
that there is another user: arch/mips/Makefile c.f. kernel test
robot's report:
https://lore.kernel.org/llvm/202205030015.JCmg5yPS-lkp@intel.com/

Because of that, only creating a single empty file to check the
timestamp is insufficient and I could not find a smart way to
manage all this.

> >  #####
> >  # Check atomic headers are up-to-date
> >
> > -always-y += old-atomics
> > +always-y += scripts/atomic/.old-atomics
> >
> >  quiet_cmd_atomics = CALL    $<
> >        cmd_atomics = $(CONFIG_SHELL) $<
> >
> > -old-atomics: scripts/atomic/check-atomics.sh FORCE
> > -       $(call cmd,atomics)
> > +scripts/atomic/.old-atomics: scripts/atomic/check-atomics.sh $(wildcard include/linux/atomic/*) FORCE
> > +       $(call if_changed,atomics)
> > +       @touch $@
>
>
> Presumably, this is wrong.
> If the header is manually edited, Kbuild must stop the build.

Currently (i.e. before my patch), Kbuild does not stop the build either.
c.f. check-atomics.sh which returns 0 inconditionally:
https://elixir.bootlin.com/linux/v5.17/source/scripts/atomic/check-atomics.sh#L33

This can be easily remediated by making check-atomics.sh return
an error code if the check does not succeed.

> This change just lets it keep going, and
> what is worse, the warning is completely silenced
> second time.

Same comment as before, I expect warnings to only be raised once,
making this an error would solve the issue.

I will send a v2 but only for check-atomics.sh


Yours sincerely,
Vincent Mailhol

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH v2 0/2] call check-atomics.sh only if prerequisites change
  2022-04-26 15:52 [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change Vincent Mailhol
  2022-04-29 17:11 ` Masahiro Yamada
@ 2022-05-07 13:11 ` Vincent Mailhol
  2022-05-07 13:11   ` [RFC PATCH v2 1/2] check-atomiscs: stop build if CONFIG_WERROR=y Vincent Mailhol
  2022-05-07 13:11   ` [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change Vincent Mailhol
  1 sibling, 2 replies; 14+ messages in thread
From: Vincent Mailhol @ 2022-05-07 13:11 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers, Tom Rix
  Cc: linux-kernel, Arnd Bergmann, Andy Shevchenko, Rikard Falkeborn,
	Andrew Morton, Kees Cook, Will Deacon, Peter Zijlstra,
	Vincent Mailhol

Currently, check-atomics.sh is executed unconditionally despite the
fact that most developers do not need this to execute this check on
each iterative make instance.

This series first make check-atomics.sh raise an error instead of a
warning when CONFIG_WERROR is set. The second and last patch modifies
Kbuild so that the script is only called when dependencies are
changed.


* Changelog *

v1 -> v2

  * drop the changes on checksyscalls.sh. v1 assumed that
    checksyscalls.sh would only be called from ./Kbuild. It appears
    that there is another user: arch/mips/Makefile c.f. kernel test
    robot's report:
    https://lore.kernel.org/llvm/202205030015.JCmg5yPS-lkp@intel.com/

  * add a patch to the to make Kbuild stop if check-atomics fails and
    if CONFIG_WERROR is set.

Vincent Mailhol (2):
  check-atomiscs: stop build if CONFIG_WERROR=y
  kbuild: call check-atomics.sh only if prerequisites change

 Kbuild                          | 7 ++++---
 Makefile                        | 3 ++-
 scripts/atomic/check-atomics.sh | 9 +++++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH v2 1/2] check-atomiscs: stop build if CONFIG_WERROR=y
  2022-05-07 13:11 ` [RFC PATCH v2 0/2] call " Vincent Mailhol
@ 2022-05-07 13:11   ` Vincent Mailhol
  2022-05-07 13:11   ` [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change Vincent Mailhol
  1 sibling, 0 replies; 14+ messages in thread
From: Vincent Mailhol @ 2022-05-07 13:11 UTC (permalink / raw)
  To: Masahiro Yamada, Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland
  Cc: linux-kernel, Arnd Bergmann, Andy Shevchenko, Rikard Falkeborn,
	Andrew Morton, Kees Cook, Vincent Mailhol

If CONFIG_WERROR=y, stop the build if one of the generated atomics
header in include/linux/atomic/* is modified by returning an error
instead of a warning.

If CONFIG_WERROR is not set, behavior is unchanged (let the build
continue).

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 scripts/atomic/check-atomics.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh
index 0e7bab3eb0d1..56f9e753bc74 100755
--- a/scripts/atomic/check-atomics.sh
+++ b/scripts/atomic/check-atomics.sh
@@ -26,8 +26,13 @@ while read header; do
 	NEWSUM="${NEWSUM%% *}"
 
 	if [ "${OLDSUM}" != "${NEWSUM}" ]; then
-		printf "warning: generated include/${header} has been modified.\n"
+		if grep -q CONFIG_WERROR=y .config; then
+			printf "error: generated include/${header} has been modified.\n"
+			exit 1
+		else
+			printf "warning: generated include/${header} has been modified.\n"
+		fi
 	fi
 done
 
-exit 0
+exit $?
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-07 13:11 ` [RFC PATCH v2 0/2] call " Vincent Mailhol
  2022-05-07 13:11   ` [RFC PATCH v2 1/2] check-atomiscs: stop build if CONFIG_WERROR=y Vincent Mailhol
@ 2022-05-07 13:11   ` Vincent Mailhol
  2022-05-13 19:01     ` Masahiro Yamada
  1 sibling, 1 reply; 14+ messages in thread
From: Vincent Mailhol @ 2022-05-07 13:11 UTC (permalink / raw)
  To: Masahiro Yamada, Masahiro Yamada, Michal Marek, Nick Desaulniers
  Cc: linux-kernel, Arnd Bergmann, Andy Shevchenko, Rikard Falkeborn,
	Andrew Morton, Kees Cook, Will Deacon, Peter Zijlstra,
	Vincent Mailhol

check-atomics.sh is executed unconditionally. Most developers will not
modify the files being checked by this script and thus do not need to
execute it again for each iterative make.

We first add an additional dependency to include/linux/atomic/* to
make sure that the script gets executed again if the headers are
modified. We then use the if_change macro instead of cmd. c.f. [1] and
create the dot file scripts/atomic/.check-atomics which is used for
the target timestamp. Finally, the dot file is added to the
CLEAN_FILES target.

[1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 Kbuild   | 7 ++++---
 Makefile | 3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Kbuild b/Kbuild
index fa441b98c9f6..c3cb76ebcbaf 100644
--- a/Kbuild
+++ b/Kbuild
@@ -50,10 +50,11 @@ missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
 #####
 # Check atomic headers are up-to-date
 
-always-y += old-atomics
+always-y += scripts/atomic/.check-atomics
 
 quiet_cmd_atomics = CALL    $<
       cmd_atomics = $(CONFIG_SHELL) $<
 
-old-atomics: scripts/atomic/check-atomics.sh FORCE
-	$(call cmd,atomics)
+scripts/atomic/.check-atomics: scripts/atomic/check-atomics.sh $(wildcard include/linux/atomic/*) FORCE
+	$(call if_changed,atomics)
+	@touch $@
diff --git a/Makefile b/Makefile
index 9a820c525b86..9e815c8bb0b6 100644
--- a/Makefile
+++ b/Makefile
@@ -1483,7 +1483,8 @@ endif # CONFIG_MODULES
 # Directories & files removed with 'make clean'
 CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
 	       modules.builtin modules.builtin.modinfo modules.nsdeps \
-	       compile_commands.json .thinlto-cache
+	       compile_commands.json .thinlto-cache \
+	       scripts/atomic/.check-atomics
 
 # Directories & files removed with 'make mrproper'
 MRPROPER_FILES += include/config include/generated          \
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-07 13:11   ` [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change Vincent Mailhol
@ 2022-05-13 19:01     ` Masahiro Yamada
  2022-05-14  3:21       ` Vincent MAILHOL
  2022-05-14 13:14       ` Peter Zijlstra
  0 siblings, 2 replies; 14+ messages in thread
From: Masahiro Yamada @ 2022-05-13 19:01 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List,
	Arnd Bergmann, Andy Shevchenko, Rikard Falkeborn, Andrew Morton,
	Kees Cook, Will Deacon, Peter Zijlstra

On Sat, May 7, 2022 at 10:13 PM Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
>
> check-atomics.sh is executed unconditionally. Most developers will not
> modify the files being checked by this script and thus do not need to
> execute it again for each iterative make.
>
> We first add an additional dependency to include/linux/atomic/* to
> make sure that the script gets executed again if the headers are
> modified. We then use the if_change macro instead of cmd. c.f. [1] and
> create the dot file scripts/atomic/.check-atomics which is used for
> the target timestamp. Finally, the dot file is added to the
> CLEAN_FILES target.
>
> [1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection
>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>


I do not like this approach.

I wrote a different patch.
https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u

This naturally works by comparing the timestamp
between *_shipped and include/generated/*.







> ---
>  Kbuild   | 7 ++++---
>  Makefile | 3 ++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Kbuild b/Kbuild
> index fa441b98c9f6..c3cb76ebcbaf 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -50,10 +50,11 @@ missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
>  #####
>  # Check atomic headers are up-to-date
>
> -always-y += old-atomics
> +always-y += scripts/atomic/.check-atomics
>
>  quiet_cmd_atomics = CALL    $<
>        cmd_atomics = $(CONFIG_SHELL) $<
>
> -old-atomics: scripts/atomic/check-atomics.sh FORCE
> -       $(call cmd,atomics)
> +scripts/atomic/.check-atomics: scripts/atomic/check-atomics.sh $(wildcard include/linux/atomic/*) FORCE
> +       $(call if_changed,atomics)
> +       @touch $@
> diff --git a/Makefile b/Makefile
> index 9a820c525b86..9e815c8bb0b6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1483,7 +1483,8 @@ endif # CONFIG_MODULES
>  # Directories & files removed with 'make clean'
>  CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
>                modules.builtin modules.builtin.modinfo modules.nsdeps \
> -              compile_commands.json .thinlto-cache
> +              compile_commands.json .thinlto-cache \
> +              scripts/atomic/.check-atomics
>
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_FILES += include/config include/generated          \
> --
> 2.35.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-13 19:01     ` Masahiro Yamada
@ 2022-05-14  3:21       ` Vincent MAILHOL
  2022-05-14 13:14       ` Peter Zijlstra
  1 sibling, 0 replies; 14+ messages in thread
From: Vincent MAILHOL @ 2022-05-14  3:21 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List,
	Arnd Bergmann, Andy Shevchenko, Rikard Falkeborn, Andrew Morton,
	Kees Cook, Will Deacon, Peter Zijlstra

On Sat. 14 May 2022 at 04:01, Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Sat, May 7, 2022 at 10:13 PM Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
> >
> > check-atomics.sh is executed unconditionally. Most developers will not
> > modify the files being checked by this script and thus do not need to
> > execute it again for each iterative make.
> >
> > We first add an additional dependency to include/linux/atomic/* to
> > make sure that the script gets executed again if the headers are
> > modified. We then use the if_change macro instead of cmd. c.f. [1] and
> > create the dot file scripts/atomic/.check-atomics which is used for
> > the target timestamp. Finally, the dot file is added to the
> > CLEAN_FILES target.
> >
> > [1] https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#command-change-detection
> >
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
>
> I do not like this approach.
>
> I wrote a different patch.
> https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u
>
> This naturally works by comparing the timestamp
> between *_shipped and include/generated/*.

Thank you very much for taking the time to fully rewrite it in order
to simply discard the check-atomics.sh. I like the idea.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-13 19:01     ` Masahiro Yamada
  2022-05-14  3:21       ` Vincent MAILHOL
@ 2022-05-14 13:14       ` Peter Zijlstra
  2022-05-14 14:55         ` Peter Zijlstra
  2022-05-14 16:07         ` Masahiro Yamada
  1 sibling, 2 replies; 14+ messages in thread
From: Peter Zijlstra @ 2022-05-14 13:14 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> I wrote a different patch.
> https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u

I'm not seeing that in my inbox :-(

AFAICT this way 'make tags' will not find and index the files, which is
a total no-go.

NAK

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-14 13:14       ` Peter Zijlstra
@ 2022-05-14 14:55         ` Peter Zijlstra
  2022-05-14 16:26           ` Masahiro Yamada
  2022-05-14 16:07         ` Masahiro Yamada
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2022-05-14 14:55 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sat, May 14, 2022 at 03:14:48PM +0200, Peter Zijlstra wrote:
> On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> > I wrote a different patch.
> > https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u
> 
> I'm not seeing that in my inbox :-(
> 
> AFAICT this way 'make tags' will not find and index the files, which is
> a total no-go.
> 
> NAK

Additionally, if you spuriously regenerate these files, you'll risk
rebuilding huge parts of the kernel through the dependencies.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-14 13:14       ` Peter Zijlstra
  2022-05-14 14:55         ` Peter Zijlstra
@ 2022-05-14 16:07         ` Masahiro Yamada
  2022-05-16  7:59           ` Peter Zijlstra
  1 sibling, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2022-05-14 16:07 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sat, May 14, 2022 at 10:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> > I wrote a different patch.
> > https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u
>
> I'm not seeing that in my inbox :-(
>
> AFAICT this way 'make tags' will not find and index the files, which is
> a total no-go.

Not only these, we have more generated files.
Why are these bad?

Also, "make tags" picks up headers in include/generated/


>
> NAK

I consider NAK as "I do not like it".



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-14 14:55         ` Peter Zijlstra
@ 2022-05-14 16:26           ` Masahiro Yamada
  2022-05-16  8:02             ` Peter Zijlstra
  0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2022-05-14 16:26 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sat, May 14, 2022 at 11:56 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Sat, May 14, 2022 at 03:14:48PM +0200, Peter Zijlstra wrote:
> > On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> > > I wrote a different patch.
> > > https://lore.kernel.org/lkml/20220513185340.239753-1-masahiroy@kernel.org/T/#u
> >
> > I'm not seeing that in my inbox :-(
> >
> > AFAICT this way 'make tags' will not find and index the files, which is
> > a total no-go.
> >
> > NAK
>
> Additionally, if you spuriously regenerate these files, you'll risk
> rebuilding huge parts of the kernel through the dependencies.

This is just one-time copying.
Rebuild only happens just once.


BTW, gen-atomics.sh takes more than 1 sec.
Do you think gen-atomics.sh can be much faster
if it is rewritten by Python or Perl?
Then, we can drop 5000 lines from the git repository.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-14 16:07         ` Masahiro Yamada
@ 2022-05-16  7:59           ` Peter Zijlstra
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2022-05-16  7:59 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sun, May 15, 2022 at 01:07:27AM +0900, Masahiro Yamada wrote:
> On Sat, May 14, 2022 at 10:15 PM Peter Zijlstra <peterz@infradead.org> wrote:

> > NAK
> 
> I consider NAK as "I do not like it".

Well, I *am* the maintainer of that stuff, you can consider all you
like, but I *will* revert anything I don't like.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change
  2022-05-14 16:26           ` Masahiro Yamada
@ 2022-05-16  8:02             ` Peter Zijlstra
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2022-05-16  8:02 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Vincent Mailhol, Michal Marek, Nick Desaulniers,
	Linux Kernel Mailing List, Arnd Bergmann, Andy Shevchenko,
	Rikard Falkeborn, Andrew Morton, Kees Cook, Will Deacon

On Sun, May 15, 2022 at 01:26:23AM +0900, Masahiro Yamada wrote:

> BTW, gen-atomics.sh takes more than 1 sec.
> Do you think gen-atomics.sh can be much faster
> if it is rewritten by Python or Perl?
> Then, we can drop 5000 lines from the git repository.

I don't speak either; which would make it unsuitable for purpose.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-05-16  8:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 15:52 [RFC PATCH] kbuild: call checksyscalls.sh and check-atomics.sh only if prerequisites change Vincent Mailhol
2022-04-29 17:11 ` Masahiro Yamada
2022-05-07 11:43   ` Vincent MAILHOL
2022-05-07 13:11 ` [RFC PATCH v2 0/2] call " Vincent Mailhol
2022-05-07 13:11   ` [RFC PATCH v2 1/2] check-atomiscs: stop build if CONFIG_WERROR=y Vincent Mailhol
2022-05-07 13:11   ` [RFC PATCH v2 2/2] kbuild: call check-atomics.sh only if prerequisites change Vincent Mailhol
2022-05-13 19:01     ` Masahiro Yamada
2022-05-14  3:21       ` Vincent MAILHOL
2022-05-14 13:14       ` Peter Zijlstra
2022-05-14 14:55         ` Peter Zijlstra
2022-05-14 16:26           ` Masahiro Yamada
2022-05-16  8:02             ` Peter Zijlstra
2022-05-14 16:07         ` Masahiro Yamada
2022-05-16  7:59           ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).