Re: [PATCH v4 10/14] perf pmu-events: Split big_c_string storage into standalone compilation unit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Race condition in parallel builds causing non-deterministic 'No such file or directory' errors.
- [Medium] Meaningful code and commit message mismatch regarding dependency chaining.
- [Low] NO_JEVENTS=1 fallback and default jevents.py mode fail to compile due to big_c_string linkage conflict.
--

commit b03f8034531e9c582fd4777daf59f93349bcf916
Author: Ian Rogers <irogers@google.com>

perf pmu-events: Split big_c_string storage into standalone compilation unit

This commit splits the generation of big_c_string into a dedicated
compilation unit to speed up Kbuild. It modifies jevents.py to
optionally generate this file, adds it to the build system, and
updates the pmu-events.h header.

>     To guarantee 100% backward compatibility with GNU Make 4.0+ (avoiding the
>     Make 4.3+ grouped target &: syntax which causes older Make versions like
>     4.2.1 to spawn multiple concurrent jevents.py processes during parallel
>     builds), implement a robust dependency chaining pattern:
>       $(PMU_EVENTS_C): $(JEVENTS_DEPS)
>       $(PMU_EVENTS_STRING_C): $(PMU_EVENTS_C)
>               @:

The commit message states that a no-op recipe (@:) is used for dependency
chaining, but the actual code implements a file-recovery shell script
instead. Could the commit message be updated to match the code?

> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> index dc1df2d57ddc1..00ae173cdb5c8 100644
> --- a/tools/perf/pmu-events/Build
> +++ b/tools/perf/pmu-events/Build

[ ... ]

> @@ -224,6 +234,10 @@ endif
>  # and inputs are dependencies.
>  $(PMU_EVENTS_C): $(JEVENTS_DEPS)
>  	$(call rule_mkdir)
> -	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) $(OUT_DIR) $@
> +	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) \
> +            $(OUT_DIR) $(PMU_EVENTS_C) $(PMU_EVENTS_STRING_C)
> +
> +$(PMU_EVENTS_STRING_C): $(PMU_EVENTS_C)
> +	$(Q)test -f $@ || { rm -f $<; $(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events obj=pmu-events; }

Does this introduce a race condition during parallel builds?

If pmu-events-string.c is missing but pmu-events.c is up-to-date, a
parallel build could execute this recipe concurrently with the compilation
of pmu-events.o. The rm -f $< would remove pmu-events.c while the C compiler
is trying to read it, leading to a "No such file or directory" error.

> diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
> index d3b24014c6ff1..fb4eb5b7ec163 100644
> --- a/tools/perf/pmu-events/pmu-events.h
> +++ b/tools/perf/pmu-events/pmu-events.h
> @@ -141,4 +141,6 @@ int pmu_for_each_sys_metric(pmu_metric_iter_fn fn, void *data);
>  
>  const char *describe_metricgroup(const char *group);
>  
> +extern const char big_c_string[];

Will this cause a compilation error when building with NO_JEVENTS=1?

The build uses empty-pmu-events.c which still defines big_c_string as:

static const char *const big_c_string = ...;

Including this header will result in a conflicting type and linkage error
between const char[] and const char *const, and between extern and static.
The same conflict appears to happen when invoking jevents.py without the
output_string_file argument.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515173852.1378571-1-irogers@google.com?part=10