[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fXvgicd9FjHobQWNmouYHv87FUbtAQqaQfCE_i4bjzFVQ@mail.gmail.com>
Date: Wed, 31 Jul 2024 18:35:38 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: John Garry <john.g.garry@...cle.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jing Zhang <renyu.zj@...ux.alibaba.com>, Xu Yang <xu.yang_2@....com>,
Sandipan Das <sandipan.das@....com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, philip.li@...el.com, oliver.sang@...el.com,
Weilin Wang <weilin.wang@...el.com>
Subject: Re: [PATCH v3 2/2] perf jevents: Autogenerate empty-pmu-events.c
On Wed, Jul 31, 2024 at 1:41 PM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> 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@...nel.org> 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@...nel.org> 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@...nel.org> 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@...lbox perf-tools-next]$ rm -rf /tmp/build/$(basename $PWD)/ ; mkdir -p /tmp/build/$(basename $PWD)/
> > > > > ⬢[acme@...lbox 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@...lbox 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@linux.intel.com/
> > > > 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!
These tests can be sensitive to the NMI watchdog being enabled.
> 118: Miscellaneous Intel PT testing : FAILED!
I bisected this failure to:
```
Author: Leo Yan <leo.yan@....com>
Date: Thu Jul 25 07:46:17 2024 +0100
perf auxtrace: Iterate all AUX events when finish reading
When finished to read AUX trace data from mmaped buffer, based on the
AUX buffer index the core layer needs to search the corresponding PMU
event and re-enable it to continue tracing.
However, current code only searches the first AUX event. It misses to
search other enabled AUX events, thus, it returns failure if the buffer
index does not belong to the first AUX event.
This patch extends the auxtrace_record__read_finish() function to
search for every enabled AUX events, so all the mmaped buffer indexes
can be covered.
Signed-off-by: Leo Yan <leo.yan@....com>
Cc: Mark Rutland <mark.rutland@....com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>
Cc: Ian Rogers <irogers@...gle.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: James Clark <james.clark@...aro.org>
Cc: Mike Leach <mike.leach@...aro.org>
Cc: <coresight@...ts.linaro.org>
Cc: John Garry <john.g.garry@...cle.com>
Cc: <linux-perf-users@...r.kernel.org>
Link: https://lore.kernel.org/lkml/
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
tools/perf/util/auxtrace.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
```
> root@x1:~# perf test 110
> 110: perf stat --bpf-counters --for-each-cgroup test : FAILED!
This one flakes if the test machine has load on it.
> 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.
Thanks,
Ian
Powered by blists - more mailing lists