linux-kernel - Re: [PATCH v6] perf script: Fix output type for dynamically allocated core PMU's

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=fUb0yhDHKxsMjqYTtJUfA-Me7kFHE66XRxQ+-OUSGsmLQ@mail.gmail.com>
Date: Wed, 5 Mar 2025 11:22:53 -0800
From: Ian Rogers <irogers@...gle.com>
To: Thomas Falcon <thomas.falcon@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...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>, Kan Liang <kan.liang@...ux.intel.com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, 
	Kan Liang <kan.liang@...el.com>
Subject: Re: [PATCH v6] perf script: Fix output type for dynamically allocated
 core PMU's

On Wed, Mar 5, 2025 at 8:39 AM Thomas Falcon <thomas.falcon@...el.com> wrote:
>
> This patch was originally posted here:
>
> https://lore.kernel.org/all/20241213215421.661139-1-thomas.falcon@intel.com/
>
> I have rebased on top of Arnaldo's patch here:
>
> https://lore.kernel.org/all/Z2XCi3PgstSrV0SE@x1/
>
> The original commit message:
> "
> perf script output may show different fields on different core PMU's
> that exist on heterogeneous platforms. For example,
>
> perf record -e "{cpu_core/mem-loads-aux/,cpu_core/event=0xcd,\
> umask=0x01,ldlat=3,name=MEM_UOPS_RETIRED.LOAD_LATENCY/}:upp"\
> -c10000 -W -d -a -- sleep 1
>
> perf script:
>
> chromium-browse   46572 [002] 544966.882384:      10000         cpu_core/MEM_UOPS_RETIRED.LOAD_LATENCY/: 7ffdf1391b0c     10268100142 \
>  |OP LOAD|LVL L1 hit|SNP None|TLB L1 or L2 hit|LCK No|BLK    N/A    5   7    0   7fad7c47425d [unknown] (/usr/lib64/libglib-2.0.so.0.8000.3)
>
> perf record -e cpu_atom/event=0xd0,umask=0x05,ldlat=3,\
> name=MEM_UOPS_RETIRED.LOAD_LATENCY/upp -c10000 -W -d -a -- sleep 1
>
> perf script:
>
> gnome-control-c  534224 [023] 544951.816227:      10000 cpu_atom/MEM_UOPS_RETIRED.LOAD_LATENCY/:   7f0aaaa0aae0  [unknown] (/usr/lib64/libglib-2.0.so.0.8000.3)
>
> Some fields, such as data_src, are not included by default.
>
> The cause is that while one PMU may be assigned a type such as
> PERF_TYPE_RAW, other core PMU's are dynamically allocated at boot time.
> If this value does not match an existing PERF_TYPE_X value,
> output_type(perf_event_attr.type) will return OUTPUT_TYPE_OTHER.
>
> Instead search for a core PMU with a matching perf_event_attr type
> and, if one is found, return PERF_TYPE_RAW to match output of other
> core PMU's.
> "
>
> Suggested-by: Kan Liang <kan.liang@...el.com>
> Suggested-by: Ian Rogers <irogers@...gle.com>
> Signed-off-by: Thomas Falcon <thomas.falcon@...el.com>

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

Thanks,
Ian

> ---
> v2: restrict pmu lookup to platforms with more than one core pmu
> v3: only scan core pmu list
> v4: rebase on top of Arnaldo's patch
> v5: update based on Namhyung's feedback here
> https://lore.kernel.org/lkml/Z8YcOidenzGofq7R@google.com/
> v6: update based on Ian's feedback here
> https://lore.kernel.org/linux-perf-users/CAP-5=fVwrG5ZViysO7qKYUw0326WFfa7nOzwe70drYfHq=AXhA@mail.gmail.com/
> ---
>
>  tools/perf/builtin-script.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index d797cec4f054..253c4ca395e1 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -400,10 +400,20 @@ static inline int output_type(unsigned int type)
>
>  static inline int evsel__output_type(struct evsel *evsel)
>  {
> -       if (evsel->script_output_type == OUTPUT_TYPE_UNSET)
> -               evsel->script_output_type = output_type(evsel->core.attr.type);
> +       int type = evsel->script_output_type;
>
> -       return evsel->script_output_type;
> +       if (type == OUTPUT_TYPE_UNSET) {
> +               type = output_type(evsel->core.attr.type);
> +               if (type == OUTPUT_TYPE_OTHER) {
> +                       struct perf_pmu *pmu = evsel__find_pmu(evsel);
> +
> +                       if (pmu && pmu->is_core)
> +                               type = PERF_TYPE_RAW;
> +               }
> +               evsel->script_output_type = type;
> +       }
> +
> +       return type;
>  }
>
>  static bool output_set_by_user(void)
> --
> 2.48.1
>