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

[Namhyung Kim <namhyung@kernel.org>]

On Fri, May 15, 2026 at 04:25:29PM -0700, Ian Rogers wrote:
> On Fri, May 15, 2026 at 12:58 PM <sashiko-bot@kernel.org> wrote:
> >
> > 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?
> 
> So previously we were worried about the opposite scenario. Using &:
> for a group of generated files solves the problem but Linux builds
> only require GNU make 4.0 not 4.3, which added this group construct.
> We may need to drop this change as I don't see a way to satisfy
> Sashiko.
 
Isn't the message at the end a proposed solution by Sashiko to satisfy
the problem?  See below ...


> > > 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?

... here.  Or, maybe you can call os.utime().

Thanks,
Namhyung