[PATCH v5] perf script: Fix output type for dynamically allocated core PMU's

Thomas Falcon posted 1 patch 11 months, 1 week ago
There is a newer version of this series
tools/perf/builtin-script.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
[PATCH v5] perf script: Fix output type for dynamically allocated core PMU's
Posted by Thomas Falcon 11 months, 1 week ago
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@intel.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.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) {
+		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))
+		return PERF_TYPE_RAW;
+
 	return OUTPUT_TYPE_OTHER;
 }
 
-- 
2.48.1
Re: [PATCH v5] perf script: Fix output type for dynamically allocated core PMU's
Posted by Ian Rogers 11 months, 1 week ago
On Mon, Mar 3, 2025 at 5:39 PM Thomas Falcon <thomas.falcon@intel.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@intel.com>
> Signed-off-by: Thomas Falcon <thomas.falcon@intel.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

> +               return PERF_TYPE_RAW;
> +
>         return OUTPUT_TYPE_OTHER;
>  }
>
> --
> 2.48.1
>
Re: [PATCH v5] perf script: Fix output type for dynamically allocated core PMU's
Posted by Falcon, Thomas 11 months, 1 week ago
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@intel.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@intel.com>
> > Signed-off-by: Thomas Falcon <thomas.falcon@intel.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
> >