Re: [PATCH v4 2/2] kbuild: Support directory targets for building DTBs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kbuild: parallel build race in dtbs target and extra boot/dts traversal
--

commit 3f00c18c0d96a2ac1d59c7335959fec4fa041ff7
Author: Rob Herring (Arm) <robh@kernel.org>

kbuild: Support directory targets for building DTBs

This commit enables building DTBs using directory targets so that all DTBs
for a vendor directory can be built, including base+overlay DTB targets,
and updates the dtbs target to use the directory target rule.

> diff --git a/Makefile b/Makefile
> index c307ee9293bcd..6d1d1c803ee86 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1664,8 +1664,8 @@ dtbindingtree := Documentation/devicetree/bindings
>  ifneq ($(dtstree),)
>  
>  PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
> -dtbs: dtbs_prepare
> -	$(Q)$(MAKE) $(build)=$(dtstree) need-dtbslist=1
> +dtbs:
> +	$(Q)$(MAKE) $(dtstree)/

[Severity: High]
Does dropping dtbs_prepare from the dtbs target prerequisites and spawning a
separate top-level make for $(dtstree)/ introduce a parallel build race when
vmlinux and dtbs are built together (for example, during make -j all when
CONFIG_OF_EARLY_FLATTREE=y, make -j vmlinux when CONFIG_GENERIC_BUILTIN_DTB=y,
or make -j image.fit on arm64)?

Previously, vmlinux (via . -> prepare -> prepare0 -> archprepare) and dtbs
(via dtbs_prepare) shared the same top-level make dependency graph, which
serialized include/config/kernel.release, scripts_basic, and scripts_dtc:

Makefile:
    scripts: scripts_basic scripts_dtc
    	$(Q)$(MAKE) $(build)=$(@)
    ...
    archprepare: outputmakefile archheaders archscripts scripts include/config/kernel.release \
    ...
    dtbs_prepare: include/config/kernel.release scripts_dtc

With dtbs running $(MAKE) $(dtstree)/, the child make enables single-build
and evaluates $(build-dir): $(single-build-prep) where single-build-prep is
dtbs_prepare. In a parallel build, the parent make (building archprepare) and
the child make (building dtbs_prepare) have independent dependency graphs and
can run concurrently:

1. Both make instances can run filechk for include/config/kernel.release at
   the same time, racing on the shared include/config/.tmp_kernel.release
   temporary file in scripts/Kbuild.include:

   define filechk
   	$(check-FORCE)
   	$(Q)set -e;						\
   	mkdir -p $(dir $@);					\
   	trap "rm -f $(tmp-target)" EXIT;			\
   	{ $(filechk_$(1)); } > $(tmp-target);			\
   	if [ ! -r $@ ] || ! cmp -s $@ $(tmp-target); then	\
   		$(kecho) '  UPD     $@';			\
   		mv -f $(tmp-target) $@;				\
   	fi
   endef

2. Both make instances can concurrently invoke $(MAKE) $(build)=scripts/basic
   and $(MAKE) $(build)=scripts/dtc in the same output directory, racing on
   building the host binaries (fixdep, dtc, fdtoverlay) and their .*.cmd
   files.

[ ... ]

> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 4349108e75e1f..d22b3b2d43b32 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -38,6 +38,13 @@ include $(srctree)/scripts/Makefile.compiler
>  include $(kbuild-file)
>  include $(srctree)/scripts/Makefile.lib
>  
> +# Make DT directory targets reachable for architectures with a DTS tree.
> +ifeq ($(obj),arch/$(SRCARCH))
> +ifneq ($(wildcard $(src)/boot/dts/),)
> +subdir-y += boot/dts
> +endif
> +endif
> +

[Severity: High]
Because subdir-y += boot/dts is added unconditionally whenever $(obj) is
arch/$(SRCARCH), won't the main "." build pass (as well as non-DT builds like
make vmlinux or make modules) now recurse into arch/$(SRCARCH)/boot/dts and
all vendor subdirectories even when need-dtbslist is not set?

During make -j, the parent make's "." traversal will descend into
arch/$(SRCARCH)/boot/dts in parallel with the child $(MAKE) $(dtstree)/
spawned by the dtbs target. In every vendor directory where dtb-y or dtb- is
non-empty, scripts/Makefile.build includes scripts/Makefile.dtbs (which adds
$(real-dtb-y) to targets) and then includes the .cmd files for existing
targets:

scripts/Makefile.build:
    ifneq ($(need-dtbslist)$(dtb-y)$(dtb-)$(filter %.dtb %.dtb.o %.dtbo.o,$(targets)),)
    include $(srctree)/scripts/Makefile.dtbs
    endif
    ...
    existing-targets := $(wildcard $(sort $(targets)))

    -include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)

If the parallel dtbs sub-make has just created a .dtb file and cmd_and_fixdep
is still writing the corresponding .*.dtb.cmd file while the "." build walk
is evaluating that directory, can the "." sub-make read a partially written
.*.dtb.cmd file and fail with a syntax error?

[ ... ]

$>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-dtb-dirs-v3-v4-0-0ec42fd8ef9b@kernel.org?part=2