| Message ID | 20240427153253.2809911-2-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] kbuild: remove redundant $(wildcard ) for rm-files | expand |
On Sun, Apr 28, 2024 at 12:32:53AM +0900, Masahiro Yamada wrote: > Currently, Kbuild produces inconsistent results in some cases. > > You can do an interesting experiment using the --shuffle option, which > is supported by GNU Make 4.4 or later. > > Set CONFIG_KVM_INTEL=y and CONFIG_KVM_AMD=m (or vice versa), and repeat > incremental builds w/wo --shuffle=reverse. > > $ make > [ snip ] > CC arch/x86/kvm/kvm-asm-offsets.s > > $ make --shuffle=reverse > [ snip ] > CC [M] arch/x86/kvm/kvm-asm-offsets.s > > $ make > [ snip ] > CC arch/x86/kvm/kvm-asm-offsets.s > > arch/x86/kvm/kvm-asm-offsets.s is rebuilt every time w/wo the [M] marker. > > arch/x86/kvm/kvm-asm-offsets.s is built as built-in when it is built as > a prerequisite of arch/x86/kvm/kvm-intel.o, which is built-in. > > arch/x86/kvm/kvm-asm-offsets.s is built as modular when it is built as > a prerequisite of arch/x86/kvm/kvm-amd.o, which is a module. > > Another odd example is single target builds. > > When CONFIG_LKDTM=m, drivers/misc/lkdtm/rodata.o can be built as > built-in or modular, depending on how it is built. > > $ make drivers/misc/lkdtm/lkdtm.o > [ snip ] > CC [M] drivers/misc/lkdtm/rodata.o > > $ make drivers/misc/lkdtm/rodata.o > [ snip ] > CC drivers/misc/lkdtm/rodata.o > > drivers/misc/lkdtm/rodata.o is built as modular when it is built as a > prerequisite of another, but built as built-in when it is a final > target. > > The same thing happens to drivers/memory/emif-asm-offsets.s when > CONFIG_TI_EMIF_SRAM=m. > > $ make drivers/memory/ti-emif-sram.o > [ snip ] > CC [M] drivers/memory/emif-asm-offsets.s > > $ make drivers/memory/emif-asm-offsets.s > [ snip ] > CC drivers/memory/emif-asm-offsets.s > > This is because the part-of-module=y flag defined for the modules is > inherited by its prerequisites. > > Target-specific variables are likely intended only for local use. > This commit adds 'private' to them. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- uh, thanks for fixing this! (And for the bug documentation, as always!) I have just one question below. > > Makefile | 8 ++++---- > scripts/Makefile.build | 6 +++--- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index 62557fabfee5..25dcc7ead330 100644 > --- a/Makefile > +++ b/Makefile [...] > @@ -1500,7 +1500,7 @@ MRPROPER_FILES += include/config include/generated \ > > # clean - Delete most, but leave enough to build external modules > # > -clean: rm-files := $(CLEAN_FILES) > +clean: private rm-files := $(CLEAN_FILES) Did you leave 'clean: rm-files := $(KBUILD_EXTMOD)/...' for oot kmods the way it is (w/o 'private') by intention? Even though I cannot think of a possible problem without the 'private', I think it makes sense to change the line as well. W/ or w/o the 'clean'-update for oot kmods: Reviewed-by: Nicolas Schier <n.schier@avm.de> Kind regards, Nicolas
On Tue, May 7, 2024 at 8:16 PM Nicolas Schier <nicolas@fjasle.eu> wrote: > > On Sun, Apr 28, 2024 at 12:32:53AM +0900, Masahiro Yamada wrote: > > Currently, Kbuild produces inconsistent results in some cases. > > > > You can do an interesting experiment using the --shuffle option, which > > is supported by GNU Make 4.4 or later. > > > > Set CONFIG_KVM_INTEL=y and CONFIG_KVM_AMD=m (or vice versa), and repeat > > incremental builds w/wo --shuffle=reverse. > > > > $ make > > [ snip ] > > CC arch/x86/kvm/kvm-asm-offsets.s > > > > $ make --shuffle=reverse > > [ snip ] > > CC [M] arch/x86/kvm/kvm-asm-offsets.s > > > > $ make > > [ snip ] > > CC arch/x86/kvm/kvm-asm-offsets.s > > > > arch/x86/kvm/kvm-asm-offsets.s is rebuilt every time w/wo the [M] marker. > > > > arch/x86/kvm/kvm-asm-offsets.s is built as built-in when it is built as > > a prerequisite of arch/x86/kvm/kvm-intel.o, which is built-in. > > > > arch/x86/kvm/kvm-asm-offsets.s is built as modular when it is built as > > a prerequisite of arch/x86/kvm/kvm-amd.o, which is a module. > > > > Another odd example is single target builds. > > > > When CONFIG_LKDTM=m, drivers/misc/lkdtm/rodata.o can be built as > > built-in or modular, depending on how it is built. > > > > $ make drivers/misc/lkdtm/lkdtm.o > > [ snip ] > > CC [M] drivers/misc/lkdtm/rodata.o > > > > $ make drivers/misc/lkdtm/rodata.o > > [ snip ] > > CC drivers/misc/lkdtm/rodata.o > > > > drivers/misc/lkdtm/rodata.o is built as modular when it is built as a > > prerequisite of another, but built as built-in when it is a final > > target. > > > > The same thing happens to drivers/memory/emif-asm-offsets.s when > > CONFIG_TI_EMIF_SRAM=m. > > > > $ make drivers/memory/ti-emif-sram.o > > [ snip ] > > CC [M] drivers/memory/emif-asm-offsets.s > > > > $ make drivers/memory/emif-asm-offsets.s > > [ snip ] > > CC drivers/memory/emif-asm-offsets.s > > > > This is because the part-of-module=y flag defined for the modules is > > inherited by its prerequisites. > > > > Target-specific variables are likely intended only for local use. > > This commit adds 'private' to them. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > uh, thanks for fixing this! (And for the bug documentation, as always!) > > I have just one question below. > > > > > Makefile | 8 ++++---- > > scripts/Makefile.build | 6 +++--- > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 62557fabfee5..25dcc7ead330 100644 > > --- a/Makefile > > +++ b/Makefile > [...] > > @@ -1500,7 +1500,7 @@ MRPROPER_FILES += include/config include/generated \ > > > > # clean - Delete most, but leave enough to build external modules > > # > > -clean: rm-files := $(CLEAN_FILES) > > +clean: private rm-files := $(CLEAN_FILES) > > Did you leave 'clean: rm-files := $(KBUILD_EXTMOD)/...' for oot kmods > the way it is (w/o 'private') by intention? No. I missed to update this line. I will fix it up for consistency. Thanks. > > Even though I cannot think of a possible problem without the 'private', > I think it makes sense to change the line as well. > > W/ or w/o the 'clean'-update for oot kmods: > > Reviewed-by: Nicolas Schier <n.schier@avm.de> > > Kind regards, > Nicolas
diff --git a/Makefile b/Makefile index 62557fabfee5..25dcc7ead330 100644 --- a/Makefile +++ b/Makefile @@ -1255,8 +1255,8 @@ define filechk_version.h echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL) endef -$(version_h): PATCHLEVEL := $(or $(PATCHLEVEL), 0) -$(version_h): SUBLEVEL := $(or $(SUBLEVEL), 0) +$(version_h): private PATCHLEVEL := $(or $(PATCHLEVEL), 0) +$(version_h): private SUBLEVEL := $(or $(SUBLEVEL), 0) $(version_h): FORCE $(call filechk,version.h) @@ -1500,7 +1500,7 @@ MRPROPER_FILES += include/config include/generated \ # clean - Delete most, but leave enough to build external modules # -clean: rm-files := $(CLEAN_FILES) +clean: private rm-files := $(CLEAN_FILES) PHONY += archclean vmlinuxclean @@ -1512,7 +1512,7 @@ clean: archclean vmlinuxclean resolve_btfids_clean # mrproper - Delete all generated files, including .config # -mrproper: rm-files := $(MRPROPER_FILES) +mrproper: private rm-files := $(MRPROPER_FILES) mrproper-dirs := $(addprefix _mrproper_,scripts) PHONY += $(mrproper-dirs) mrproper diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 43b71c3d0ff6..c9c07a6144eb 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -216,7 +216,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),y) -$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y)) +$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y)) ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),) cmd_warn_shared_object = $(if $(word 2, $(modname-multi)),$(warning $(kbuild-file): $*.o is added to multiple modules: $(modname-multi))) @@ -437,8 +437,8 @@ define rule_ld_multi_m $(call cmd,gen_objtooldep) endef -$(multi-obj-m): objtool-enabled := $(delay-objtool) -$(multi-obj-m): part-of-module := y +$(multi-obj-m): private objtool-enabled := $(delay-objtool) +$(multi-obj-m): private part-of-module := y $(multi-obj-m): %.o: %.mod FORCE $(call if_changed_rule,ld_multi_m) $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
Currently, Kbuild produces inconsistent results in some cases. You can do an interesting experiment using the --shuffle option, which is supported by GNU Make 4.4 or later. Set CONFIG_KVM_INTEL=y and CONFIG_KVM_AMD=m (or vice versa), and repeat incremental builds w/wo --shuffle=reverse. $ make [ snip ] CC arch/x86/kvm/kvm-asm-offsets.s $ make --shuffle=reverse [ snip ] CC [M] arch/x86/kvm/kvm-asm-offsets.s $ make [ snip ] CC arch/x86/kvm/kvm-asm-offsets.s arch/x86/kvm/kvm-asm-offsets.s is rebuilt every time w/wo the [M] marker. arch/x86/kvm/kvm-asm-offsets.s is built as built-in when it is built as a prerequisite of arch/x86/kvm/kvm-intel.o, which is built-in. arch/x86/kvm/kvm-asm-offsets.s is built as modular when it is built as a prerequisite of arch/x86/kvm/kvm-amd.o, which is a module. Another odd example is single target builds. When CONFIG_LKDTM=m, drivers/misc/lkdtm/rodata.o can be built as built-in or modular, depending on how it is built. $ make drivers/misc/lkdtm/lkdtm.o [ snip ] CC [M] drivers/misc/lkdtm/rodata.o $ make drivers/misc/lkdtm/rodata.o [ snip ] CC drivers/misc/lkdtm/rodata.o drivers/misc/lkdtm/rodata.o is built as modular when it is built as a prerequisite of another, but built as built-in when it is a final target. The same thing happens to drivers/memory/emif-asm-offsets.s when CONFIG_TI_EMIF_SRAM=m. $ make drivers/memory/ti-emif-sram.o [ snip ] CC [M] drivers/memory/emif-asm-offsets.s $ make drivers/memory/emif-asm-offsets.s [ snip ] CC drivers/memory/emif-asm-offsets.s This is because the part-of-module=y flag defined for the modules is inherited by its prerequisites. Target-specific variables are likely intended only for local use. This commit adds 'private' to them. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Makefile | 8 ++++---- scripts/Makefile.build | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)