linux-kernel - Re: [RFC PATCH v10 3/8] perf stat: Fork and launch perf record when perf stat needs to get retire latency value for a metric.

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: <Zl-WCEhhxBtiGGhn@google.com>
Date: Tue, 4 Jun 2024 15:32:40 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: "Wang, Weilin" <weilin.wang@...el.com>, Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	"Hunter, Adrian" <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Taylor, Perry" <perry.taylor@...el.com>,
	"Alt, Samantha" <samantha.alt@...el.com>,
	"Biggers, Caleb" <caleb.biggers@...el.com>
Subject: Re: [RFC PATCH v10 3/8] perf stat: Fork and launch perf record when
 perf stat needs to get retire latency value for a metric.

On Tue, Jun 04, 2024 at 08:00:26PM +0000, Wang, Weilin wrote:

> > On Thu, May 30, 2024 at 11:46 PM Wang, Weilin <weilin.wang@...el.com>
> > > > > +       /*
> > > > > +        * Only set retire_latency value to the first CPU and thread.
> > > > > +        */
> > > > > +       if (cpu_map_idx == 0 && thread == 0) {
> > > > > +       /* Lost precision when casting from double to __u64. Any
> > > > improvement? */
> > > >
> > > > As I said before I think you can set t->val * 1000 and then
> > > > set the evsel->scale to 1e3 or 1e-3.
> > >
> > > Hi Namhyung,
> > >
> > > Sorry if this is a repeated message. I thought I replied to your suggestion
> > > on this last time. I'm thinking we should keep it like this for now and make
> > > this change unless we find the precision loss is critical. Because I thought
> > > we don't want to add special code to handle the calculation and final print
> > > to keep code simple.
> > >
> > > I kept this comment here so that we don't forget about it. Please let me
> > > know if you'd like me to remove it.
> > 
> > Please see print_counter_aggrdata().  It's the generic code to print
> > the event value and it'll display the value multiplied by the scale
> > (default to 1.0).  So you can keep precision as long as you set the
> > scale properly (1e-3).
> 
> I could see the retire_latency is printed correctly after set the evsel->scale to 1e-3
> and multiply the t->val * 1000. However, this scale is not used in metric calculations. 
> We need to add code in metric calculation or display part to scale it as well. Is that 
> acceptable or do you have other suggestions? 

Hmm.. I don't know if other metric already dealt with the scale like with
RAPL events.. If not, I think it's reasonable to add that to the metric
calculation.

Ian, what do you think?

Thanks,
Namhyung