Messages in this thread | | | From | Justin He <> | Subject | RE: [PATCH 2/2] perf pmu: Fix num_events calculation | Date | Fri, 10 May 2024 06:52:08 +0000 |
| |
Hi, Ian
> -----Original Message----- > From: Ian Rogers <irogers@google.com> > Sent: Friday, May 10, 2024 2:17 PM > To: Justin He <Justin.He@arm.com> > Cc: Peter Zijlstra <peterz@infradead.org>; Ingo Molnar <mingo@redhat.com>; > Arnaldo Carvalho de Melo <acme@kernel.org>; Namhyung Kim > <namhyung@kernel.org>; Mark Rutland <Mark.Rutland@arm.com>; Alexander > Shishkin <alexander.shishkin@linux.intel.com>; Jiri Olsa <jolsa@kernel.org>; > Adrian Hunter <adrian.hunter@intel.com>; Kan Liang > <kan.liang@linux.intel.com>; James Clark <James.Clark@arm.com>; > linux-perf-users@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/2] perf pmu: Fix num_events calculation > > On Thu, May 9, 2024 at 7:47 PM Jia He <justin.he@arm.com> wrote: > > > > When pe is NULL in the function perf_pmu__new_alias(), the total > > number of events is added to loaded_json_aliases. However, if > > pmu->events_table is NULL and cpu_aliases_added is false, the > > calculation for the events number in perf_pmu__num_events() is incorrect. > > > > Then cause the error report after "perf list": > > Unexpected event > smmuv3_pmcg_3f062/smmuv3_pmcg_3f062/transaction// > > > > Fix it by adding loaded_json_aliases in the calculation under the > > mentioned conditions. > > > > Test it also with "perf bench internals pmu-scan" and there is no > > regression. > > > > Fixes: e6ff1eed3584 ("perf pmu: Lazily add JSON events") > > Signed-off-by: Jia He <justin.he@arm.com> > > --- > > tools/perf/util/pmu.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index > > a1eef7b2e389..a53224e2ce7e 100644 > > --- a/tools/perf/util/pmu.c > > +++ b/tools/perf/util/pmu.c > > @@ -1639,6 +1639,8 @@ size_t perf_pmu__num_events(struct perf_pmu > *pmu) > > nr += pmu->loaded_json_aliases; > > else if (pmu->events_table) > > nr += > pmu_events_table__num_events(pmu->events_table, > > pmu) - pmu->loaded_json_aliases; > > + else > > + nr += pmu->loaded_json_aliases; > > Thanks for working on this! The "struct pmu_event *pe" in new_alias is an entry > from the json data, and "pmu->events_table" should NULL if there is no json > data. I believe the code is assuming that these lines aren't necessary as it > shouldn't be possible to load json data if the json events table doesn't exist for > the PMU - if there is no json data then loaded_json_aliases should be 0 and no > addition is necessary. I'm wondering why this case isn't true for you. On my Armv8a N2 server, "pmu->events_table" is NULL because perf_pmu__find_events_table() return NULL.
I also noticed that pmu->loaded_json_aliases is *not* 0. The missing adding calculation will cause perf_pmu__num_events() less than normal case and will trigger latter check failure in perf_pmus__print_pmu_events__callback(). At last, perf list will report many lines similar as: Unexpected event smmuv3_pmcg_3f062/smmuv3_pmcg_3f062/transaction//
-- Cheers, Justin (Jia He)
| |