linux-kernel - Re: [PATCH 1/2] perf x86/topdown: Fix topdown leader sampling test error on hybrid

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=fUyVYBfrDQ9BfiW-JhWgMQ39cNOM4wjWcE5HhqSigEE-w@mail.gmail.com>
Date: Tue, 4 Mar 2025 22:37:33 -0800
From: Ian Rogers <irogers@...gle.com>
To: "Falcon, Thomas" <thomas.falcon@...el.com>
Cc: "alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>, 
	"peterz@...radead.org" <peterz@...radead.org>, "acme@...nel.org" <acme@...nel.org>, 
	"dapeng1.mi@...ux.intel.com" <dapeng1.mi@...ux.intel.com>, "mingo@...hat.com" <mingo@...hat.com>, 
	"Hunter, Adrian" <adrian.hunter@...el.com>, "namhyung@...nel.org" <namhyung@...nel.org>, 
	"kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>, "Mi, Dapeng1" <dapeng1.mi@...el.com>
Subject: Re: [PATCH 1/2] perf x86/topdown: Fix topdown leader sampling test
 error on hybrid

On Tue, Mar 4, 2025 at 8:49 AM Falcon, Thomas <thomas.falcon@...el.com> wrote:
>
> On Mon, 2025-02-24 at 08:33 +0000, Dapeng Mi wrote:
> > When running topdown leader smapling test on Intel hybrid platforms,
> > such as LNL/ARL, we see the below error.
> >
> > Topdown leader sampling test
> > Topdown leader sampling [Failed topdown events not reordered
> > correctly]
> >
> > It indciates the below command fails.
> >
> > perf record -o "${perfdata}" -e "{instructions,slots,topdown-
> > retiring}:S" true
> >
> > The root cause is that perf tool creats a perf event for each PMU
> > type
> > if it can create.
> >
> > As for this command, there would be 5 perf events created,
> > cpu_atom/instructions/,cpu_atom/topdown_retiring/,
> > cpu_core/slots/,cpu_core/instructions/,cpu_core/topdown-retiring/
> >
> > For these 5 events, the 2 cpu_atom events are in a group and the
> > other 3
> > cpu_core events are in another group.
> >
> > When arch_topdown_sample_read() traverses all these 5 events, events
> > cpu_atom/instructions/ and cpu_core/slots/ don't have a same group
> > leade, and then return false directly and lead to cpu_core/slots/
> > event
> > is used to sample and this is not allowed by PMU driver.
> >
> > It's a overkill to return false directly if "evsel->core.leader !=
> >  leader->core.leader" since there could be multiple groups in the
> > event
> > list.
> >
> > Just "continue" instead of "return false" to fix this issue.
> >
> > Fixes: 1e53e9d1787b ("perf x86/topdown: Correct leader selection with
> > sample_read enabled")
> > Signed-off-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
>
> Tested-by: Thomas Falcon <thomas.falcon@...el.com>

On an Alderlake:
Tested-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> > ---
> >  tools/perf/arch/x86/util/topdown.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/arch/x86/util/topdown.c
> > b/tools/perf/arch/x86/util/topdown.c
> > index f63747d0abdf..d1c654839049 100644
> > --- a/tools/perf/arch/x86/util/topdown.c
> > +++ b/tools/perf/arch/x86/util/topdown.c
> > @@ -81,7 +81,7 @@ bool arch_topdown_sample_read(struct evsel *leader)
> >        */
> >       evlist__for_each_entry(leader->evlist, evsel) {
> >               if (evsel->core.leader != leader->core.leader)
> > -                     return false;
> > +                     continue;
> >               if (evsel != leader &&
> > arch_is_topdown_metrics(evsel))
> >                       return true;
> >       }
> >
> > base-commit: 4bac7fb5862740087825eda3ed6168e91da8b7e6
>