Re: [PATCH 2/2] kbuild: add 'private' to target-specific variables

From: Nicolas Schier
Date: Tue May 07 2024 - 07:25:15 EST


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@xxxxxxxxxx>
> ---

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@xxxxxx>

Kind regards,
Nicolas