linux-kernel - Re: [PATCH 2/3] perf pmu: Don't double count common sysfs and json events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fVKX6029y9W8JCw-kjMuL9O5z0Ut2LDUhz=Ske3Qg7vDQ@mail.gmail.com>
Date: Tue, 25 Feb 2025 09:22:22 -0800
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: linux-perf-users@...r.kernel.org, namhyung@...nel.org, cyy@...self.name, 
	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...ux.intel.com>, 
	Weilin Wang <weilin.wang@...el.com>, Yoshihiro Furudera <fj5100bi@...itsu.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Junhao He <hejunhao3@...wei.com>, 
	Jean-Philippe Romain <jean-philippe.romain@...s.st.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] perf pmu: Don't double count common sysfs and json events

On Tue, Feb 25, 2025 at 8:47 AM James Clark <james.clark@...aro.org> wrote:
>
> After pmu_add_cpu_aliases() is called, perf_pmu__num_events() returns an
> incorrect value that double counts common events and doesn't match the
> actual count of events in the alias list. This is because after
> 'cpu_aliases_added == true', the number of events returned is
> 'sysfs_aliases + cpu_json_aliases'. But when adding 'case
> EVENT_SRC_SYSFS' events, 'sysfs_aliases' and 'cpu_json_aliases' are both
> incremented together, failing to account that these ones overlap and
> only add a single item to the list. Fix it by adding another counter for
> overlapping events which doesn't influence 'cpu_json_aliases'.
>
> There doesn't seem to be a current issue because it's used in perf list
> before pmu_add_cpu_aliases() so the correct value is returned. Other
> uses in tests may also miss it for other reasons like only looking at
> uncore events. However it's marked as a fixes commit in case any new fix
> with new uses of perf_pmu__num_events() is backported.
>
> Fixes: d9c5f5f94c2d ("perf pmu: Count sys and cpuid JSON events separately")
> Signed-off-by: James Clark <james.clark@...aro.org>

Reviewed-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> ---
>  tools/perf/util/pmu.c | 7 ++++---
>  tools/perf/util/pmu.h | 5 +++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index ec3878c890a9..72aa6167c090 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -596,7 +596,7 @@ static int perf_pmu__new_alias(struct perf_pmu *pmu, const char *name,
>                         };
>                         if (pmu_events_table__find_event(pmu->events_table, pmu, name,
>                                                          update_alias, &data) == 0)
> -                               pmu->cpu_json_aliases++;
> +                               pmu->cpu_common_json_aliases++;
>                 }
>                 pmu->sysfs_aliases++;
>                 break;
> @@ -1884,9 +1884,10 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
>         if (pmu->cpu_aliases_added)
>                  nr += pmu->cpu_json_aliases;
>         else if (pmu->events_table)
> -               nr += pmu_events_table__num_events(pmu->events_table, pmu) - pmu->cpu_json_aliases;
> +               nr += pmu_events_table__num_events(pmu->events_table, pmu) -
> +                       pmu->cpu_common_json_aliases;
>         else
> -               assert(pmu->cpu_json_aliases == 0);
> +               assert(pmu->cpu_json_aliases == 0 && pmu->cpu_common_json_aliases == 0);
>
>         if (perf_pmu__is_tool(pmu))
>                 nr -= tool_pmu__num_skip_events();
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index f5306428c03f..b93014cc3670 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -136,6 +136,11 @@ struct perf_pmu {
>         uint32_t cpu_json_aliases;
>         /** @sys_json_aliases: Number of json event aliases loaded matching the PMU's identifier. */
>         uint32_t sys_json_aliases;
> +       /**
> +        * @cpu_common_json_aliases: Number of json events that overlapped with sysfs when
> +        * loading all sysfs events.
> +        */
> +       uint32_t cpu_common_json_aliases;
>         /** @sysfs_aliases_loaded: Are sysfs aliases loaded from disk? */
>         bool sysfs_aliases_loaded;
>         /**
> --
> 2.34.1
>