lkml.org 
[lkml]   [2024]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 2/2] perf pmu: Fix num_events calculation
Date
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)
\
 
 \ /
  Last update: 2024-05-10 08:54    [W:0.153 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site