* [PATCH 0/2] Fix num_events calculation in lazy loading
@ 2024-05-10 2:47 Jia He
2024-05-10 2:47 ` [PATCH 1/2] perf pmu: Allow finishing loading json events when !events_table Jia He
2024-05-10 2:47 ` [PATCH 2/2] perf pmu: Fix num_events calculation Jia He
0 siblings, 2 replies; 8+ messages in thread
From: Jia He @ 2024-05-10 2:47 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Kan Liang, James Clark, linux-perf-users,
linux-kernel, Jia He
I noticed "perf list" reported the error as follows on an Armv8 Neoverse
N2 server:
Unexpected event smmuv3_pmcg_3f062/smmuv3_pmcg_3f062/transaction//
The root cause is due to the incorrect calculation in
perf_pmu__num_events().
Jia He (2):
perf pmu: Allow finishing loading json events when !events_table
perf pmu: Fix num_events calculation
tools/perf/util/pmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] perf pmu: Allow finishing loading json events when !events_table
2024-05-10 2:47 [PATCH 0/2] Fix num_events calculation in lazy loading Jia He
@ 2024-05-10 2:47 ` Jia He
2024-05-10 2:47 ` [PATCH 2/2] perf pmu: Fix num_events calculation Jia He
1 sibling, 0 replies; 8+ messages in thread
From: Jia He @ 2024-05-10 2:47 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Kan Liang, James Clark, linux-perf-users,
linux-kernel, Jia He
Otherwise, cpu_aliases_added is never set to true on an Arm v8a
Neoverse N2 server.
Signed-off-by: Jia He <justin.he@arm.com>
---
tools/perf/util/pmu.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f39cbbc1a7ec..a1eef7b2e389 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -915,13 +915,11 @@ void pmu_add_cpu_aliases_table(struct perf_pmu *pmu, const struct pmu_events_tab
static void pmu_add_cpu_aliases(struct perf_pmu *pmu)
{
- if (!pmu->events_table)
- return;
-
if (pmu->cpu_aliases_added)
return;
- pmu_add_cpu_aliases_table(pmu, pmu->events_table);
+ if (pmu->events_table)
+ pmu_add_cpu_aliases_table(pmu, pmu->events_table);
pmu->cpu_aliases_added = true;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-10 2:47 [PATCH 0/2] Fix num_events calculation in lazy loading Jia He
2024-05-10 2:47 ` [PATCH 1/2] perf pmu: Allow finishing loading json events when !events_table Jia He
@ 2024-05-10 2:47 ` Jia He
2024-05-10 6:16 ` Ian Rogers
1 sibling, 1 reply; 8+ messages in thread
From: Jia He @ 2024-05-10 2:47 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Kan Liang, James Clark, linux-perf-users,
linux-kernel, Jia He
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;
return pmu->selectable ? nr + 1 : nr;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-10 2:47 ` [PATCH 2/2] perf pmu: Fix num_events calculation Jia He
@ 2024-05-10 6:16 ` Ian Rogers
2024-05-10 6:52 ` Justin He
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2024-05-10 6:16 UTC (permalink / raw)
To: Jia He
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Kan Liang, James Clark, linux-perf-users,
linux-kernel
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.
Thanks,
Ian
>
> return pmu->selectable ? nr + 1 : nr;
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-10 6:16 ` Ian Rogers
@ 2024-05-10 6:52 ` Justin He
2024-05-10 20:41 ` Ian Rogers
0 siblings, 1 reply; 8+ messages in thread
From: Justin He @ 2024-05-10 6:52 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Kan Liang, James Clark,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
nd
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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-10 6:52 ` Justin He
@ 2024-05-10 20:41 ` Ian Rogers
2024-05-11 0:50 ` Ian Rogers
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2024-05-10 20:41 UTC (permalink / raw)
To: Justin He, John Garry
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Kan Liang, James Clark,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
nd
On Thu, May 9, 2024 at 11:52 PM Justin He <Justin.He@arm.com> wrote:
>
> 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//
The issue is the sys events which currently are ARM only. Here is an
lldb stack trace:
```
frame #6: 0x0000aaaaabc9b49c
perf`__assert_fail(assertion="pmu->events_table",
file="tools/perf/util/pmu.c", line=522, function="int
perf_pmu__new_alias(struct perf_pmu *, const char *, const char *,
const char *, FILE *, const struct pmu_event *)")
frame #7: 0x0000aaaaab41e018
perf`perf_pmu__new_alias(pmu=0x00007377e7e09b40,
name="hnf_brd_snoops_sent", desc="Counts number of multicast snoops
sent (not including SF back invalidation)", val="eventid=9,type=5",
val_fd=0x0000000000000000, pe=0x0000ffffffffde88) at pmu.c:522:3
frame #8: 0x0000aaaaab4175cc
perf`pmu_add_sys_aliases_iter_fn(pe=0x0000ffffffffde88,
table=0x0000aaaaac299bb0, vdata=0x00007377e7e09b40) at pmu.c:939:3
frame #9: 0x0000aaaaaaf6d000
perf`pmu_events_table__for_each_event_pmu(table=0x0000aaaaac299bb0,
pmu=0x0000aaaaac299238, fn=(perf`pmu_add_sys_aliases_iter_fn at
pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:5994:23
frame #10: 0x0000aaaaaaf6cd78
perf`pmu_events_table__for_each_event(table=0x0000aaaaac299bb0,
pmu=0x0000000000000000, fn=(perf`pmu_add_sys_aliases_iter_fn at
pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:6057:23
frame #11: 0x0000aaaaaaf6ec44
perf`pmu_for_each_sys_event(fn=(perf`pmu_add_sys_aliases_iter_fn at
pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:6295:27
frame #12: 0x0000aaaaab41738c
perf`pmu_add_sys_aliases(pmu=0x00007377e7e09b40) at pmu.c:955:2
frame #13: 0x0000aaaaab4179fc
perf`perf_pmu__lookup(pmus=0x0000aaaaac500970, dirfd=34,
name="arm_cmn_0") at pmu.c:1037:2
frame #14: 0x0000aaaaab4242d0 perf`perf_pmu__find2(dirfd=34,
name="arm_cmn_0") at pmus.c:161:9
frame #15: 0x0000aaaaab421614 perf`pmu_read_sysfs(core_only=false)
at pmus.c:209:3
frame #16: 0x0000aaaaab42278c
perf`perf_pmus__scan_skip_duplicates(pmu=0x0000000000000000) at
pmus.c:297:3
frame #17: 0x0000aaaaab422078
perf`perf_pmus__print_pmu_events(print_cb=0x0000ffffffffec68,
print_state=0x0000ffffffffecd0) at pmus.c:462:16
frame #18: 0x0000aaaaab425d9c
perf`print_events(print_cb=0x0000ffffffffec68,
print_state=0x0000ffffffffecd0) at print-events.c:409:2
frame #19: 0x0000aaaaab093ef0 perf`cmd_list(argc=0,
argv=0x0000fffffffff340) at builtin-list.c:592:3
frame #20: 0x0000aaaaaaf5b490
perf`run_builtin(p=0x0000aaaaac4e8220, argc=1,
argv=0x0000fffffffff340) at perf.c:349:11
frame #21: 0x0000aaaaaaf5a3e0 perf`handle_internal_command(argc=1,
argv=0x0000fffffffff340) at perf.c:402:8
frame #22: 0x0000aaaaaaf5b1a0
perf`run_argv(argcp=0x0000fffffffff1c8, argv=0x0000fffffffff1c0) at
perf.c:446:2
frame #23: 0x0000aaaaaaf59f44 perf`main(argc=1,
argv=0x0000fffffffff340) at perf.c:562:3
```
As such the fix here is incomplete. There may be both sys json events
(detected by PMU/id name) and CPU json events (detected by CPUID). I'm
looking into a fix.
Thanks,
Ian
> --
> Cheers,
> Justin (Jia He)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-10 20:41 ` Ian Rogers
@ 2024-05-11 0:50 ` Ian Rogers
2024-05-11 15:52 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2024-05-11 0:50 UTC (permalink / raw)
To: Justin He, John Garry
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Kan Liang, James Clark,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
nd
On Fri, May 10, 2024 at 1:41 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, May 9, 2024 at 11:52 PM Justin He <Justin.He@arm.com> wrote:
> >
> > 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//
>
> The issue is the sys events which currently are ARM only. Here is an
> lldb stack trace:
> ```
> frame #6: 0x0000aaaaabc9b49c
> perf`__assert_fail(assertion="pmu->events_table",
> file="tools/perf/util/pmu.c", line=522, function="int
> perf_pmu__new_alias(struct perf_pmu *, const char *, const char *,
> const char *, FILE *, const struct pmu_event *)")
> frame #7: 0x0000aaaaab41e018
> perf`perf_pmu__new_alias(pmu=0x00007377e7e09b40,
> name="hnf_brd_snoops_sent", desc="Counts number of multicast snoops
> sent (not including SF back invalidation)", val="eventid=9,type=5",
> val_fd=0x0000000000000000, pe=0x0000ffffffffde88) at pmu.c:522:3
> frame #8: 0x0000aaaaab4175cc
> perf`pmu_add_sys_aliases_iter_fn(pe=0x0000ffffffffde88,
> table=0x0000aaaaac299bb0, vdata=0x00007377e7e09b40) at pmu.c:939:3
> frame #9: 0x0000aaaaaaf6d000
> perf`pmu_events_table__for_each_event_pmu(table=0x0000aaaaac299bb0,
> pmu=0x0000aaaaac299238, fn=(perf`pmu_add_sys_aliases_iter_fn at
> pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:5994:23
> frame #10: 0x0000aaaaaaf6cd78
> perf`pmu_events_table__for_each_event(table=0x0000aaaaac299bb0,
> pmu=0x0000000000000000, fn=(perf`pmu_add_sys_aliases_iter_fn at
> pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:6057:23
> frame #11: 0x0000aaaaaaf6ec44
> perf`pmu_for_each_sys_event(fn=(perf`pmu_add_sys_aliases_iter_fn at
> pmu.c:931), data=0x00007377e7e09b40) at pmu-events.c:6295:27
> frame #12: 0x0000aaaaab41738c
> perf`pmu_add_sys_aliases(pmu=0x00007377e7e09b40) at pmu.c:955:2
> frame #13: 0x0000aaaaab4179fc
> perf`perf_pmu__lookup(pmus=0x0000aaaaac500970, dirfd=34,
> name="arm_cmn_0") at pmu.c:1037:2
> frame #14: 0x0000aaaaab4242d0 perf`perf_pmu__find2(dirfd=34,
> name="arm_cmn_0") at pmus.c:161:9
> frame #15: 0x0000aaaaab421614 perf`pmu_read_sysfs(core_only=false)
> at pmus.c:209:3
> frame #16: 0x0000aaaaab42278c
> perf`perf_pmus__scan_skip_duplicates(pmu=0x0000000000000000) at
> pmus.c:297:3
> frame #17: 0x0000aaaaab422078
> perf`perf_pmus__print_pmu_events(print_cb=0x0000ffffffffec68,
> print_state=0x0000ffffffffecd0) at pmus.c:462:16
> frame #18: 0x0000aaaaab425d9c
> perf`print_events(print_cb=0x0000ffffffffec68,
> print_state=0x0000ffffffffecd0) at print-events.c:409:2
> frame #19: 0x0000aaaaab093ef0 perf`cmd_list(argc=0,
> argv=0x0000fffffffff340) at builtin-list.c:592:3
> frame #20: 0x0000aaaaaaf5b490
> perf`run_builtin(p=0x0000aaaaac4e8220, argc=1,
> argv=0x0000fffffffff340) at perf.c:349:11
> frame #21: 0x0000aaaaaaf5a3e0 perf`handle_internal_command(argc=1,
> argv=0x0000fffffffff340) at perf.c:402:8
> frame #22: 0x0000aaaaaaf5b1a0
> perf`run_argv(argcp=0x0000fffffffff1c8, argv=0x0000fffffffff1c0) at
> perf.c:446:2
> frame #23: 0x0000aaaaaaf59f44 perf`main(argc=1,
> argv=0x0000fffffffff340) at perf.c:562:3
> ```
>
> As such the fix here is incomplete. There may be both sys json events
> (detected by PMU/id name) and CPU json events (detected by CPUID). I'm
> looking into a fix.
Patch sent:
https://lore.kernel.org/lkml/20240511003601.2666907-1-irogers@google.com/
Thanks,
Ian
> Thanks,
> Ian
>
> > --
> > Cheers,
> > Justin (Jia He)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] perf pmu: Fix num_events calculation
2024-05-11 0:50 ` Ian Rogers
@ 2024-05-11 15:52 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-11 15:52 UTC (permalink / raw)
To: Ian Rogers
Cc: Justin He, John Garry, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, linux-perf-users@vger.kernel.org,
linux-kernel@vger.kernel.org, nd
On Fri, May 10, 2024 at 05:50:29PM -0700, Ian Rogers wrote:
> On Fri, May 10, 2024 at 1:41 PM Ian Rogers <irogers@google.com> wrote:
> > As such the fix here is incomplete. There may be both sys json events
> > (detected by PMU/id name) and CPU json events (detected by CPUID). I'm
> > looking into a fix.
>
> Patch sent:
> https://lore.kernel.org/lkml/20240511003601.2666907-1-irogers@google.com/
I applied the patch to tmp.perf-tools-next, will go to perf-tools-next
soon, would be good for Justin to test and provide a Tested-by.
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-11 15:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-10 2:47 [PATCH 0/2] Fix num_events calculation in lazy loading Jia He
2024-05-10 2:47 ` [PATCH 1/2] perf pmu: Allow finishing loading json events when !events_table Jia He
2024-05-10 2:47 ` [PATCH 2/2] perf pmu: Fix num_events calculation Jia He
2024-05-10 6:16 ` Ian Rogers
2024-05-10 6:52 ` Justin He
2024-05-10 20:41 ` Ian Rogers
2024-05-11 0:50 ` Ian Rogers
2024-05-11 15:52 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.