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

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

[Thomas Falcon]

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>
---
 tools/perf/builtin-script.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 9e47905f75a6..e091fa1be052 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -386,6 +386,8 @@ static int evsel_script__fprintf(struct evsel_script *es, FILE *fp)
 
 static inline int output_type(unsigned int type)
 {
+	struct perf_pmu *pmu;
+
 	switch (type) {
 	case PERF_TYPE_SYNTH:
 		return OUTPUT_TYPE_SYNTH;
@@ -394,6 +396,10 @@ static inline int output_type(unsigned int type)
 			return type;
 	}
 
+	pmu = perf_pmus__find_by_type(type);
+	if (pmu && pmu->is_core)
+		return PERF_TYPE_RAW;
+
 	return OUTPUT_TYPE_OTHER;
 }
 
-- 
2.47.1

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

[Ravi Bangoria]

Hi Thomas,

> @@ -386,6 +386,8 @@ static int evsel_script__fprintf(struct evsel_script *es, FILE *fp)
>  
>  static inline int output_type(unsigned int type)
>  {
> +	struct perf_pmu *pmu;
> +
>  	switch (type) {
>  	case PERF_TYPE_SYNTH:
>  		return OUTPUT_TYPE_SYNTH;
> @@ -394,6 +396,10 @@ static inline int output_type(unsigned int type)
>  			return type;
>  	}
>  
> +	pmu = perf_pmus__find_by_type(type);
> +	if (pmu && pmu->is_core)
> +		return PERF_TYPE_RAW;

Minor nit ...

output_type() seems to be getting called a lot. For ex, for a perf.data
with 4530 samples, output_type() was called 181246 times when I ran
"perf script". IIUC, this pmu lookup is unnecessary for homogeneous
platforms? If so, can we make it conditional?

Thanks,
Ravi

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

[Liang, Kan]

On 2024-12-13 2:07 a.m., Ravi Bangoria wrote:
> Hi Thomas,
> 
>> @@ -386,6 +386,8 @@ static int evsel_script__fprintf(struct evsel_script *es, FILE *fp)
>>  
>>  static inline int output_type(unsigned int type)
>>  {
>> +	struct perf_pmu *pmu;
>> +
>>  	switch (type) {
>>  	case PERF_TYPE_SYNTH:
>>  		return OUTPUT_TYPE_SYNTH;
>> @@ -394,6 +396,10 @@ static inline int output_type(unsigned int type)
>>  			return type;
>>  	}
>>  
>> +	pmu = perf_pmus__find_by_type(type);
>> +	if (pmu && pmu->is_core)
>> +		return PERF_TYPE_RAW;
> 
> Minor nit ...
> 
> output_type() seems to be getting called a lot. For ex, for a perf.data
> with 4530 samples, output_type() was called 181246 times when I ran
> "perf script". IIUC, this pmu lookup is unnecessary for homogeneous
> platforms? If so, can we make it conditional?
> 

Agreed. I think the search should only be applied for the
perf_pmus__num_core_pmus() > 1 case.

We should just need to search the core_pmus when comparing the type.

Thanks,
Kan

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

[Falcon, Thomas]

On Fri, 2024-12-13 at 13:34 -0500, Liang, Kan wrote:
> 
> 
> On 2024-12-13 2:07 a.m., Ravi Bangoria wrote:
> > Hi Thomas,
> > 
> > > @@ -386,6 +386,8 @@ static int evsel_script__fprintf(struct
> > > evsel_script *es, FILE *fp)
> > >  
> > >  static inline int output_type(unsigned int type)
> > >  {
> > > +	struct perf_pmu *pmu;
> > > +
> > >  	switch (type) {
> > >  	case PERF_TYPE_SYNTH:
> > >  		return OUTPUT_TYPE_SYNTH;
> > > @@ -394,6 +396,10 @@ static inline int output_type(unsigned int
> > > type)
> > >  			return type;
> > >  	}
> > >  
> > > +	pmu = perf_pmus__find_by_type(type);
> > > +	if (pmu && pmu->is_core)
> > > +		return PERF_TYPE_RAW;
> > 
> > Minor nit ...
> > 
> > output_type() seems to be getting called a lot. For ex, for a
> > perf.data
> > with 4530 samples, output_type() was called 181246 times when I ran
> > "perf script". IIUC, this pmu lookup is unnecessary for homogeneous
> > platforms? If so, can we make it conditional?
> > 
> 
> Agreed. I think the search should only be applied for the
> perf_pmus__num_core_pmus() > 1 case.
> 
> We should just need to search the core_pmus when comparing the type.
> 
Thanks, will send v2 soon.

Tom

> Thanks,
> Kan
>