Hi.
2015-03-11 19:01 GMT+09:00 Michal Marek <mmarek@xxxxxxx>:
> On Sun, Mar 08, 2015 at 04:08:06PM -0700, Linus Torvalds wrote:
>> On Tue, Mar 3, 2015 at 5:07 PM, Valdis Kletnieks
>> <Valdis.Kletnieks@xxxxxx> wrote:
>> >
>> > Kbuild regenerates bounds.h and asm-offsets.h, resetting the timestamps
>> > and forcing rebuilds even if the contents haven't changed. Add a bit of
>> > shell magic to only replace the file if the contents have in fact changed,
>> > which should speed up git bisects and similar.
>> > ...
>> > RFC because I can't wrap my head around why this wasn't done ages ago.
>> > If I'm missing something something obvious, please apply a cluestick. :)
>
> We do not regenerate the files *always*, but only if bounds.c /
> asm-offsets.c or one of the headers they include change. So the rule is
> not any worse than the rule for compiling a regular .c file into an .o
> file. You can use ccache to avoid repeated rebuilds.
Right.
> However, I can see that especially bounds.h ends up being included in
> many places, so cutting its dependencies helps in some cases.
Agreed.
>
>> > Lightly tested - if I rm one of those two files, it gets rebuilt. If I
>> > insert some whitespace, it gets replaced. If I don't touch it, the datestamp
>> > doesn't change.
>>
>> I don't think this is wrong, but I'd really prefer to do the whole
>> move-if-changed thing as a separate rule, and a separate command.
>>
>> IOW, could we please have something that generates the "build.h" file
>> by separately creating a new temporary file, and then having the
>> traditional kind of move-if-changed definition
>>
>> define move-if-changed
>> if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi
>> endef
>
> We already have it and it is called "filechk." Valdis, can you check if
> the below patch works equally well for you?
This looks almost nice, but a few comments below.
> diff --git a/Kbuild b/Kbuild
> index ab8ded9..47d0630 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -13,8 +13,9 @@ define sed-y
> s:->::; p;}"
> endef
>
> -quiet_cmd_offsets = GEN $@
> -define cmd_offsets
> +# Use filechk to avoid rebuilds when a header changes, but the resulting file
> +# does not
> +define filechk_offsets
> (set -e; \
> echo "#ifndef $2"; \
> echo "#define $2"; \
> @@ -24,9 +25,9 @@ define cmd_offsets
> echo " * This file was generated by Kbuild"; \
> echo " */"; \
> echo ""; \
> - sed -ne $(sed-y) $<; \
> + sed -ne $(sed-y); \
> echo ""; \
> - echo "#endif" ) > $@
> + echo "#endif" )
> endef
>
> #####
> @@ -44,7 +45,7 @@ kernel/bounds.s: kernel/bounds.c FORCE
>
> $(obj)/$(bounds-file): kernel/bounds.s Kbuild
You are checking the resulting file content,
so the dependency on "Kbuild" is not necessary.
Instead, you need to add "FORCE" so that this rule is always invoked.
> $(Q)mkdir -p $(dir $@)
You can drop this line because filechk automatically creates the
output directory.
> - $(call cmd,offsets,__LINUX_BOUNDS_H__)
> + $(call filechk,offsets,__LINUX_BOUNDS_H__)
To sum up, I want this part to look like this:
$(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
$(call cmd,filechk,__ASM_OFFSETS_H__)
> #####
> # 2) Generate asm-offsets.h
> @@ -63,7 +64,7 @@ arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c \
> $(call if_changed_dep,cc_s_c)
>
> $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
> - $(call cmd,offsets,__ASM_OFFSETS_H__)
> + $(call filechk,offsets,__ASM_OFFSETS_H__)
Ditto.
--
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
