Re: [PATCH v3 2/3] dt-bindings: wire style checker into dt_binding_check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The wrapper script `scripts/jobserver-exec` drops the exit status of the underlying `dt-check-style` command, causing `make dt_binding_check` to falsely succeed and mark the style check as complete even when violations or fatal errors occur.
- [High] The `chk_style` recipe fails to integrate with the GNU Make jobserver due to a missing `+` prefix, causing the checker to spawn `os.cpu_count()` workers regardless of the `make -j` limit.
- [Low] The command `cmd_chk_style` creates a temporary file via `$(tmp-target)` but never removes it, leaving a stale `.tmp_.dt-style.checked` file in the build tree.
--

commit b2a3666e05aa3ee60565bab78c721d9f2be7e0af
Author: Daniel Golle <daniel@makrotopia.org>

dt-bindings: wire style checker into dt_binding_check

This commit integrates the new dt-check-style tool into the dt_binding_check
Makefile target. It uses scripts/jobserver-exec to attempt to respect GNU
Make's jobserver limits and passes the file list via an argfile.

> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 7b668f7fd4007..df4c4e0da1dca 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
[ ... ]
> +DT_CHK_STYLE = $(srctree)/scripts/dtc/dt-check-style
> +
> +# Feed the file list to the checker via @argfile in a single Python
> +# process so the ruamel.yaml import is paid once. scripts/jobserver-exec
> +# claims slots from the GNU make jobserver and exposes the count via
> +# $PARALLELISM, which dt-check-style picks up to size its worker pool.
> +quiet_cmd_chk_style = STYLE   $(src)
> +      cmd_chk_style = $(find_cmd) > $(tmp-target) && \
> +		      $(PYTHON3) $(srctree)/scripts/jobserver-exec \
> +		      $(PYTHON3) $(DT_CHK_STYLE) @$(tmp-target) \
> +		      && touch $@ || true

Does scripts/jobserver-exec properly propagate the exit status from the
underlying command?

If the main() function in scripts/jobserver-exec calls jobserver.run() but
ignores its return value, it would unconditionally exit with code 0. This
would cause make dt_binding_check to falsely succeed and mark the style
check as complete even when violations or fatal errors occur.

Also, is it possible to clean up $(tmp-target) after execution?

Unlike cmd_mk_schema which uses rm -f $$f, this command leaves a stale
.tmp_.dt-style.checked file in the build tree since Kbuild does not
automatically delete $(tmp-target) after an if_changed rule completes.

[ ... ]
> +$(obj)/.dt-style.checked: $(DT_DOCS) FORCE
> +	$(call if_changed,chk_style)

Does this recipe successfully integrate with the GNU Make jobserver?

GNU Make only passes jobserver file descriptors and the --jobserver-auth flag
to recipes it recognizes as recursive makes, which typically require the
string $(MAKE) or a + prefix on the unexpanded command line.

Since Kbuild's if_changed macro does not include $(MAKE) and this recipe
lacks a + prefix, GNU Make strips the jobserver arguments from MAKEFLAGS
and closes the file descriptors before executing the command.

Could this cause scripts/jobserver-exec to silently fail to connect to the
jobserver, meaning the PARALLELISM environment variable is not set? This
would force dt-check-style to fall back to spawning os.cpu_count() workers
regardless of the make -j limit, contradicting the commit message claim
that it constrains the checker to N workers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778454442.git.daniel@makrotopia.org?part=2