| 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
| ||
|
Message-ID: <0d3bc045e06411cf8fb8dc8bcd7b9af7b216baef.camel@intel.com>
Date: Wed, 5 Mar 2025 14:53:15 +0000
From: "Falcon, Thomas" <thomas.falcon@...el.com>
To: "irogers@...gle.com" <irogers@...gle.com>
CC: "alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>,
"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
"peterz@...radead.org" <peterz@...radead.org>, "acme@...nel.org"
<acme@...nel.org>, "mingo@...hat.com" <mingo@...hat.com>, "Hunter, Adrian"
<adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...el.com>,
"namhyung@...nel.org" <namhyung@...nel.org>, "jolsa@...nel.org"
<jolsa@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "kan.liang@...ux.intel.com"
<kan.liang@...ux.intel.com>, "mark.rutland@....com" <mark.rutland@....com>
Subject: Re: [PATCH v5] perf script: Fix output type for dynamically allocated
core PMU's
On Tue, 2025-03-04 at 20:30 -0800, Ian Rogers wrote:
> On Mon, Mar 3, 2025 at 5:39 PM 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>
> > Signed-off-by: Thomas Falcon <thomas.falcon@...el.com>
> > ---
> > 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/
> > ---
> > tools/perf/builtin-script.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-
> > script.c
> > index d797cec4f054..4d2764df48a0 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -385,6 +385,19 @@ static int evsel_script__fprintf(struct
> > evsel_script *es, FILE *fp)
> > st.st_size / 1024.0 / 1024.0, es->filename,
> > es->samples);
> > }
> >
> > +static bool is_core_pmu_type(unsigned int type)
> > +{
> > + struct perf_pmu *pmu = NULL;
> > +
> > + if (perf_pmus__num_core_pmus() > 1) {
>
> On ARM it isn't a given that if there is 1 core PMU the type will be
> PERF_TYPE_RAW, on x86 this is normally the case.
>
> > + while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> > + if (pmu->type == type)
> > + return true;
> > + }
> > + }
> > + return type == PERF_TYPE_RAW;
> > +}
> > +
> > static inline int output_type(unsigned int type)
> > {
> > switch (type) {
> > @@ -395,6 +408,9 @@ static inline int output_type(unsigned int
> > type)
> > return type;
> > }
> >
> > + if (is_core_pmu_type(type))
>
> Given this is called by evsel__output_type I think you can do:
> ```
> static inline int output_type(unsigned int type)
> {
> switch (type) {
> case PERF_TYPE_SYNTH:
> return OUTPUT_TYPE_SYNTH;
> default:
> if (type < PERF_TYPE_MAX)
> return type;
> }
>
> return OUTPUT_TYPE_OTHER;
> }
>
> static inline int evsel__output_type(struct evsel *evsel)
> {
> int output_type = evsel->script_output_type;
>
> if (output_type == OUTPUT_TYPE_UNSET) {
> output_type = output_type(evsel->core.attr.type);
> if (output_type == OUTPUT_TYPE_OTHER) {
> struct perf_pmu *pmu = evsel__find_pmu(evsel);
>
> if (pmu && pmu->is_core)
> output_type = PERF_TYPE_RAW;
> }
> evsel->script_output_type = output_type;
>
> return output_type;
> }
> ```
> The evsel__find_pmu will cache the PMU for future callers.
>
> Thanks,
> Ian
Thanks! I will send a v6 with your suggestions.
Tom
>
> > + return PERF_TYPE_RAW;
> > +
> > return OUTPUT_TYPE_OTHER;
> > }
> >
> > --
> > 2.48.1
> >
Powered by blists - more mailing lists