[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZvsIgszeRrlWEUj8@google.com>
Date: Mon, 30 Sep 2024 13:22:26 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, kan.liang@...ux.intel.com,
adrian.hunter@...el.com, peterz@...radead.org, mingo@...nel.org,
linux-kernel@...r.kernel.org,
kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH] perf jevents: Don't stop at the first matched pmu when
searching a events table
On Thu, Sep 26, 2024 at 02:34:43PM -0700, Ian Rogers wrote:
> On Wed, Jul 31, 2024 at 6:31 AM Arnaldo Carvalho de Melo
> <acme@...nel.org> wrote:
> >
> > On Tue, Jul 16, 2024 at 09:30:01AM -0700, Ian Rogers wrote:
> > > On Tue, Jul 16, 2024 at 6:29 AM <kan.liang@...ux.intel.com> wrote:
> > > >
> > > > From: Kan Liang <kan.liang@...ux.intel.com>
> > > >
> > > > The "perf all PMU test" fails on a Coffee Lake machine.
> > > >
> > > > The failure is caused by the below change in the commit e2641db83f18
> > > > ("perf vendor events: Add/update skylake events/metrics").
> > > >
> > > > + {
> > > > + "BriefDescription": "This 48-bit fixed counter counts the UCLK cycles",
> > > > + "Counter": "FIXED",
> > > > + "EventCode": "0xff",
> > > > + "EventName": "UNC_CLOCK.SOCKET",
> > > > + "PerPkg": "1",
> > > > + "PublicDescription": "This 48-bit fixed counter counts the UCLK cycles.",
> > > > + "Unit": "cbox_0"
> > > > }
> > > >
> > > > The other cbox events have the unit name "CBOX", while the fixed counter
> > > > has a unit name "cbox_0". So the events_table will maintain separate
> > > > entries for cbox and cbox_0.
> > > >
> > > > The perf_pmus__print_pmu_events() calculates the total number of events,
> > > > allocate an aliases buffer, store all the events into the buffer, sort,
> > > > and print all the aliases one by one.
> > > >
> > > > The problem is that the calculated total number of events doesn't match
> > > > the stored events in the aliases buffer.
> > > >
> > > > The perf_pmu__num_events() is used to calculate the number of events. It
> > > > invokes the pmu_events_table__num_events() to go through the entire
> > > > events_table to find all events. Because of the
> > > > pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So
> > > > the events for cbox and cbox_0 are all counted.
> > > >
> > > > When storing events into the aliases buffer, the
> > > > perf_pmu__for_each_event() only process the events for cbox.
> > > >
> > > > Since a bigger buffer was allocated, the last entry are all 0.
> > > > When printing all the aliases, null will be outputted, and trigger the
> > > > failure.
> > > >
> > > > The mismatch was introduced from the commit e3edd6cf6399 ("perf
> > > > pmu-events: Reduce processed events by passing PMU"). The
> > > > pmu_events_table__for_each_event() stops immediately once a pmu is set.
> > > > But for uncore, especially this case, the method is wrong and mismatch
> > > > what perf does in the perf_pmu__num_events().
> > > >
> > > > With the patch,
> > > > $ perf list pmu | grep -A 1 clock.socket
> > > > unc_clock.socket
> > > > [This 48-bit fixed counter counts the UCLK cycles. Unit: uncore_cbox_0
> > > > $ perf test "perf all PMU test"
> > > > 107: perf all PMU test : Ok
> > > >
> > > > Reported-by: kernel test robot <oliver.sang@...el.com>
> > > > Closes: https://lore.kernel.org/all/202407101021.2c8baddb-oliver.sang@intel.com/
> > > > Fixes: e3edd6cf6399 ("perf pmu-events: Reduce processed events by passing PMU")
> > > > Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> > >
> > > Awesome sauce, thanks!
> > >
> > > Reviewed-by: Ian Rogers <irogers@...gle.com>
> >
> > Thanks, applied to tmp.perf-tools-next,
>
> Did this get applied? I'm not seeing it in perf-tools-next:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/perf/pmu-events/jevents.py?h=perf-tools-next
I'm seeing this build failure after applying this.
TEST pmu-events/empty-pmu-events.log
--- pmu-events/empty-pmu-events.c 2024-09-26 10:27:15.600055460 -0700
+++ pmu-events/test-empty-pmu-events.c 2024-09-30 13:20:34.631357001 -0700
@@ -380,7 +380,7 @@
continue;
ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
- if (pmu || ret)
+ if (ret)
return ret;
}
return 0;
make[3]: *** [pmu-events/Build:42: pmu-events/empty-pmu-events.log] Error 1
make[3]: *** Deleting file 'pmu-events/empty-pmu-events.log'
make[2]: *** [Makefile.perf:765: pmu-events/pmu-events-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....
Thanks,
Namhyung
Powered by blists - more mailing lists