Received-SPF: pass (google.com: domain of linux-kernel+bounces-175310-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240510024729.1075732-1-justin.he@arm.com> <20240510024729.1075732-3-justin.he@arm.com>
In-Reply-To: <20240510024729.1075732-3-justin.he@arm.com>
From: Ian Rogers <irogers@google.com>
Date: Thu, 9 May 2024 23:16:40 -0700
Message-ID: <CAP-5=fVGD-pK1igABj0wiq6-KVM+Z4i7rnRhM=Vy7bFHW4pLQA@mail.gmail.com>
Subject: Re: [PATCH 2/2] perf pmu: Fix num_events calculation
To: Jia 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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 9, 2024 at 7:47=E2=80=AFPM 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 +=3D pmu->loaded_json_aliases;
>         else if (pmu->events_table)
>                 nr +=3D pmu_events_table__num_events(pmu->events_table, p=
mu) - pmu->loaded_json_aliases;
> +       else
> +               nr +=3D 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
>