Re: [PATCH v5 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 1 potential issue(s) to consider:
- [Medium] Incremental build breakage and partial clean failure due to Makefile dependency chaining and Python file closing order.
--

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

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

This commit splits the large big_c_string literal into a dedicated
pmu-events-string.c file to allow parallel compilation during Kbuild.
It updates jevents.py to output both files and adjusts the Makefile
dependencies to ensure jevents.py is only executed once.

> In jevents.py, defer closing output_string_file until
> the absolute tail of main() to guarantee identical modification timestamps
> with output_file, preventing redundant rebuilds during incremental runs.

Is this guarantee accurate? Since output_file is never explicitly closed
in main(), it gets flushed and closed automatically during Python
interpreter shutdown, strictly after main() returns.

Will this result in pmu-events.c having a slightly newer modification
timestamp than pmu-events-string.c on filesystems with nanosecond
resolution?

> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> --- 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)
> +	@:

If pmu-events.c consistently ends up newer than pmu-events-string.c
due to the Python teardown order, won't Make permanently see
pmu-events-string.c as out-of-date?

Could this silently trigger redundant recompilations of pmu-events-string.o
and relinking of perf on every incremental build?

Additionally, if pmu-events-string.c is deleted while pmu-events.c
remains up-to-date, Make will execute this dummy recipe and then attempt
to compile a missing file. Does this pattern handle partial cleanups
gracefully without creating a build regression?

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py

[ ... ]

> @@ -1476,6 +1488,8 @@ struct pmu_table_entry {
>    print_mapping_table(archs)
>    print_system_mapping_table()
>    print_metricgroups()
> +  if _args.output_string_file:
> +    _args.output_string_file.close()
>  
>  if __name__ == '__main__':
>    main()

Should _args.output_file.close() be added here alongside
output_string_file.close() to ensure their final filesystem writes happen
simultaneously before interpreter shutdown?

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