Re: [PATCH v2 14/18] 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 2 potential issue(s) to consider:
- [High] The use of the `&:` grouped target syntax in the Makefile breaks compatibility with GNU Make < 4.3 and causes race conditions during parallel builds.
- [Low] The newly generated `pmu-events-string.c` is not added to `tools/perf/.gitignore`, resulting in an untracked file after an in-tree build.
--

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

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

Currently, jevents.py emits both the massive 2.8 MB big_c_string literal
and tens of thousands of compact_pmu_event struct arrays into a single
pmu-events.c compilation unit. This commit refactors jevents.py to emit
big_c_string into a dedicated pmu-events-string.c compilation unit. This
allows Kbuild to compile them simultaneously in parallel, cutting C
compilation latency.

> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> index dc1df2d57ddc1..e957aa2fcb480 100644
> --- a/tools/perf/pmu-events/Build
> +++ b/tools/perf/pmu-events/Build
> @@ -1,7 +1,12 @@
>  EMPTY_PMU_EVENTS_C = pmu-events/empty-pmu-events.c
>  # pmu-events.c will be generated by jevents.py or copied from EMPTY_PMU_EVENTS_C
>  PMU_EVENTS_C	=  $(OUTPUT)pmu-events/pmu-events.c
> +PMU_EVENTS_STRING_C = $(OUTPUT)pmu-events/pmu-events-string.c
> +

Does this new generated file need to be added to tools/perf/.gitignore?

Looking at tools/perf/.gitignore, pmu-events/pmu-events.c is explicitly
ignored, but the new string file is not:

tools/perf/.gitignore:
    ...
    pmu-events/pmu-events.c
    pmu-events/jevents
    pmu-events/metric_test.log
    ...

Compiling perf in-tree might leave the working directory dirty with an
untracked pmu-events-string.c file.

>  pmu-events-y	+= pmu-events.o
> +ifneq ($(NO_JEVENTS),1)
> +pmu-events-y	+= pmu-events-string.o
> +endif
>  
>  # pmu-events.c file is generated in the OUTPUT directory so it needs a
>  # separate rule to depend on it properly

[ ... ]

> @@ -222,8 +232,9 @@ endif
>  
>  # Finally, the rule to build pmu-events.c using jevents.py. All test
>  # and inputs are dependencies.
> -$(PMU_EVENTS_C): $(JEVENTS_DEPS)
> +$(PMU_EVENTS_STRING_C) $(PMU_EVENTS_C) &: $(JEVENTS_DEPS)
>  	$(call rule_mkdir)

Will this rule work correctly on systems using GNU Make older than 4.3?

The top-level Makefile enforces a minimum requirement of GNU Make 4.0, but
the &: grouped target feature was introduced in GNU Make 4.3.

On older, supported versions of Make (like 4.2.1), the &: syntax is not
recognized. Make parses & as a literal target and interprets the line as
having three independent targets: $(PMU_EVENTS_STRING_C), $(PMU_EVENTS_C),
and &.

During a parallel build, could this result in multiple jevents.py
processes spawning simultaneously? It seems they might write to pmu-events.c
and pmu-events-string.c concurrently, potentially interleaving output and
corrupting the generated C source files.

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