Re: [PATCH v3 2/2] perf jevents: Autogenerate empty-pmu-events.c

[Arnaldo Carvalho de Melo]

On Wed, Jul 31, 2024 at 01:20:06PM -0700, Ian Rogers wrote:
> On Wed, Jul 31, 2024 at 11:53 AM Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx> wrote:
> >
> > On Wed, Jul 31, 2024 at 08:58:43AM -0700, Ian Rogers wrote:
> > > On Wed, Jul 31, 2024 at 8:46 AM Arnaldo Carvalho de Melo
> > > <acme@xxxxxxxxxx> wrote:
> > > >
> > > > On Wed, Jul 31, 2024 at 12:33:50PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > On Wed, Jul 31, 2024 at 07:08:18AM -0700, Ian Rogers wrote:
> > > > > > On Wed, Jul 31, 2024 at 6:18 AM Arnaldo Carvalho de Melo
> > > > > > <acme@xxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 30, 2024 at 12:17:44PM -0700, Ian Rogers wrote:
> > > > > > > > empty-pmu-events.c exists so that builds may occur without python
> > > > > > > > being installed on a system. Manually updating empty-pmu-events.c to
> > > > > > > > be in sync with jevents.py is a pain, let's use jevents.py to generate
> > > > > > > > empty-pmu-events.c.
> > > > > > >
> > > > > > > What am I missing here?
> > > > > > >
> > > > > > > If it exists so that we can build on a system without python how can we
> > > > > > > use python to generate it?
> > > > > > >
> > > > > > > Now having python in the system is a requirement and thus we don't need
> > > > > > > empty-pmu-events.c anymore?
> > > > > > >
> > > > > > > Can you guys please clarify that?
> > > > > >
> > > > > > The requirement for python hasn't changed.
> > > > > >
> > > > > > Case 1: no python or NO_JEVENTS=1
> > > > > > Build happens using empty-pmu-events.c that is checked in, no python
> > > > > > is required.
> > > > > >
> > > > > > Case 2: python
> > > > > > pmu-events.c is created by jevents.py (requiring python) and then built.
> > > > > > This change adds a step where the empty-pmu-events.c is created using
> > > > > > jevents.py and that file is diffed against the checked in version.
> > > > > > This stops the checked in empty-pmu-events.c diverging if changes are
> > > > > > made to jevents.py. If the diff causes the build to fail then you just
> > > > > > copy the diff empty-pmu-events.c over the checked in one.
> > > > >
> > > > > I'll try and add your explanation to the log message, thanks for
> > > > > clarifying it!
> > > >
> > > > So, with it in place I'm now noticing:
> > > >
> > > > ⬢[acme@toolbox perf-tools-next]$ rm -rf /tmp/build/$(basename $PWD)/ ; mkdir -p /tmp/build/$(basename $PWD)/
> > > > ⬢[acme@toolbox perf-tools-next]$ alias m='rm -rf ~/libexec/perf-core/ ; make -k CORESIGHT=1 O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin && perf test python'
> > > > ⬢[acme@toolbox perf-tools-next]$ m
> > > > <SNIP>
> > > >   GEN     /tmp/build/perf-tools-next/pmu-events/test-empty-pmu-events.c
> > > >   MKDIR   /tmp/build/perf-tools-next/arch/x86/util/
> > > >   CC      /tmp/build/perf-tools-next/util/annotate.o
> > > >   CC      /tmp/build/perf-tools-next/arch/x86/util/tsc.o
> > > >   CC      /tmp/build/perf-tools-next/arch/x86/tests/hybrid.o
> > > >   CC      /tmp/build/perf-tools-next/util/block-info.o
> > > >   CC      /tmp/build/perf-tools-next/arch/x86/tests/intel-pt-test.o
> > > >   CC      /tmp/build/perf-tools-next/arch/x86/util/pmu.o
> > > >   MKDIR   /tmp/build/perf-tools-next/ui/browsers/
> > > >   CC      /tmp/build/perf-tools-next/ui/browsers/annotate.o
> > > >   CC      /tmp/build/perf-tools-next/builtin-kallsyms.o
> > > >   CC      /tmp/build/perf-tools-next/util/block-range.o
> > > >   TEST    /tmp/build/perf-tools-next/pmu-events/empty-pmu-events.log
> > > > --- pmu-events/empty-pmu-events.c       2024-07-31 12:44:14.355042296 -0300
> > > > +++ /tmp/build/perf-tools-next/pmu-events/test-empty-pmu-events.c       2024-07-31 12:45:35.048682785 -0300
> > > > @@ -380,7 +380,7 @@
> > > >                          continue;
> > > >
> > > >                  ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
> > > > -                if (pmu || ret)
> > > > +                if (ret)
> > >
> > > Right, you need to copy:
> > >  /tmp/build/perf-tools-next/pmu-events/test-empty-pmu-events.c
> > > to
> > > tools/perf/pmu-events/empty-pmu-events.c
> > > to fix this.
> > >
> > > This change has happened as you are testing with:
> > > https://lore.kernel.org/lkml/20240716132951.1748662-1-kan.liang@xxxxxxxxxxxxxxx/
> > > which isn't in the git repo yet (therefore, I can't make a patch set
> > > on it). The change is WAI as it is telling you empty-pmu-events.c has
> > > become stale and needs Kan's fix applying to it.
> >
> > ok, I'll remove Kan's patch, publish perf-tools-next and wait for the
> > now normal flow of patches.
> 
> I can resend Kan's patch with the empty-pmu-events.c fix applied. I
> don't see the changes in tmp.perf-tools-next so I can do it with
> cherry picks.

Just force pushed one more time. After a while should be there, there
are still some issues here and there, notably:


root@x1:~# perf test 105 106 118
105: perf all metricgroups test                                      : FAILED!
106: perf all metrics test                                           : FAILED!
118: Miscellaneous Intel PT testing                                  : FAILED!
root@x1:~# perf test 110
110: perf stat --bpf-counters --for-each-cgroup test                 : FAILED!
root@x1:~#

I'm running out of time today, so I'll probably just push what I have to
perf-tools-next so that we can start getting testing from linux-next and
we can then go on fixing up stuff from there.

- Arnaldo