Re: [PATCH v1 12/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 2 potential issue(s) to consider:
- [Low] Missing header declaration for global variable `big_c_string`.
- [Low] Flawed multiple-output Make rule causes build failures on configuration changes.
--

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

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

This commit modifies jevents.py to emit the large big_c_string literal
into a dedicated pmu-events-string.c file instead of pmu-events.c. This
allows the build system to compile both files in parallel to reduce
compilation latency.

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

[ ... ]

> @@ -224,6 +234,9 @@ 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_STRING_C)
> +
> +$(PMU_EVENTS_STRING_C): $(PMU_EVENTS_C)
> +	@:

Does this dummy recipe correctly handle configuration changes?

If switching from NO_JEVENTS=1 to NO_JEVENTS=0, pmu-events.c might already
exist from a copy of empty-pmu-events.c and have a timestamp newer than
JEVENTS_DEPS.

Make could consider pmu-events.c up to date, execute this dummy recipe
for the missing pmu-events-string.c, and then fail to compile
pmu-events-string.o with a missing file error.

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 3a1bcdcdc685f..70a45e62f5d16 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py

[ ... ]

> @@ -1463,10 +1465,20 @@ struct pmu_table_entry {
>      ftw(arch_path, [], preprocess_one_file)
>  
>    _bcs.compute()
> -  _args.output_file.write('static const char *const big_c_string =\n')
> -  for s in _bcs.big_string:
> -    _args.output_file.write(s)
> -  _args.output_file.write(';\n\n')
> +  if not _args.output_string_file:
> +    _args.output_file.write('static const char *const big_c_string =\n')
> +    for s in _bcs.big_string:
> +      _args.output_file.write(s)
> +    _args.output_file.write(';\n\n')
> +  else:
> +    _args.output_string_file.write('/* SPDX-License-Identifier: GPL-2.0 */\n')
> +    _args.output_string_file.write('/* Autogenerated by jevents.py */\n')
> +    _args.output_string_file.write('const char big_c_string[] =\n')

Since big_c_string is now emitted as a non-static global variable in a
separate compilation unit, does it need a shared header declaration?

Without a prior declaration included in the generated pmu-events-string.c,
compilers may emit -Wmissing-variable-declarations warnings during the
build.

Also, is big_c_string too generic of a name to place in the global namespace?

> +    for s in _bcs.big_string:
> +      _args.output_string_file.write(s)
> +    _args.output_string_file.write(';\n')
> +    _args.output_string_file.close()
> +    _args.output_file.write('extern const char big_c_string[];\n\n')

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