linux-kernel - Re: [PATCH v3] 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] [day] [month] [year] [list]
Message-ID: <361703990c1518ec6dbe81d9e140dd4edf32e951.camel@intel.com>
Date: Mon, 24 Feb 2025 18:57:53 +0000
From: "Falcon, Thomas" <thomas.falcon@...el.com>
To: "acme@...nel.org" <acme@...nel.org>
CC: "ravi.bangoria@....com" <ravi.bangoria@....com>,
	"alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>,
	"kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>, "irogers@...gle.com"
	<irogers@...gle.com>, "peterz@...radead.org" <peterz@...radead.org>,
	"mark.rutland@....com" <mark.rutland@....com>, "mingo@...hat.com"
	<mingo@...hat.com>, "Hunter, Adrian" <adrian.hunter@...el.com>,
	"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
	"namhyung@...nel.org" <namhyung@...nel.org>, "jolsa@...nel.org"
	<jolsa@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] perf script: Fix output type for dynamically allocated
 core PMU's

On Fri, 2024-12-20 at 17:37 -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Dec 20, 2024 at 08:30:11PM +0000, Falcon, Thomas wrote:
> > On Fri, 2024-12-20 at 16:16 -0300, Arnaldo Carvalho de Melo wrote:
> > > On Fri, Dec 13, 2024 at 03:54:21PM -0600, Thomas Falcon wrote:
> > > > 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
> > > > ---
> > > >  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 9e47905f75a6..685232883f9c 100644
> > > > --- a/tools/perf/builtin-script.c
> > > > +++ b/tools/perf/builtin-script.c
> > > > @@ -384,6 +384,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 output_type_many_core_pmus(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 false;
> > > > +}
> > > > +
> > > >  static inline int output_type(unsigned int type)
> > > >  {
> > > >  	switch (type) {
> > > > @@ -394,6 +407,9 @@ static inline int output_type(unsigned int
> > > > type)
> > > >  			return type;
> > > >  	}
> > > >  
> > > > +	if (output_type_many_core_pmus(type))
> > > > +		return PERF_TYPE_RAW;
> > > > +
> > > >  	return OUTPUT_TYPE_OTHER;
> > > >  }
> > > 
> > > Can you please test the patch below so that we don't do this
> > > while
> > > loop
> > > in all calls to output_type when we have more than one core pmu?
> > > 
> > > I haven't tested this patch, so please see if your patch on top
> > > of it
> > > produces the desired result.
> > 
> > Hi Arnaldo, it looks good to me.
> 
> Thanks, added an
> 
> Acked-by: Thomas Falcon <thomas.falcon@...el.com>
> 
> To that cset,
> 
> Doing some test build test accross a number of distros now, will push
> that out and then try to see if your patch applies cleanly or do the
> adjustment to apply it.
> 
> - Arnaldo

Hi Arnaldo, were you able to apply this patch? Is there anything you
need from me?

Thanks,
Tom