On Sat, May 7, 2022 at 10:13 PM Vincent Mailhol
<[email protected]> 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 <[email protected]>
I do not like this approach.
I wrote a different patch.
https://lore.kernel.org/lkml/[email protected]/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
On Sat. 14 May 2022 at 04:01, Masahiro Yamada <[email protected]> wrote:
> On Sat, May 7, 2022 at 10:13 PM Vincent Mailhol
> <[email protected]> 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 <[email protected]>
>
>
> I do not like this approach.
>
> I wrote a different patch.
> https://lore.kernel.org/lkml/[email protected]/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.
On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> I wrote a different patch.
> https://lore.kernel.org/lkml/[email protected]/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
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/[email protected]/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.
On Sat, May 14, 2022 at 10:15 PM Peter Zijlstra <[email protected]> wrote:
>
> On Sat, May 14, 2022 at 04:01:18AM +0900, Masahiro Yamada wrote:
> > I wrote a different patch.
> > https://lore.kernel.org/lkml/[email protected]/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
On Sat, May 14, 2022 at 11:56 PM Peter Zijlstra <[email protected]> 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/[email protected]/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
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.
On Sun, May 15, 2022 at 01:07:27AM +0900, Masahiro Yamada wrote:
> On Sat, May 14, 2022 at 10:15 PM Peter Zijlstra <[email protected]> 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.