linux-kernel - Re: [PATCH v1] perf stat: Fix uncore aggregation number

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: <CAP-5=fWvnALUJiHrb_xzHXjseD88HF7LExs4N_Okg+UguuXsXg@mail.gmail.com>
Date: Wed, 11 Jun 2025 22:12:34 -0700
From: Ian Rogers <irogers@...gle.com>
To: Chun-Tse Shao <ctshao@...gle.com>
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org, mingo@...hat.com, 
	acme@...nel.org, namhyung@...nel.org, mark.rutland@....com, 
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org, adrian.hunter@...el.com, 
	kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1] perf stat: Fix uncore aggregation number

On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@...gle.com> wrote:
>
> Thanks for your test, Ian!
>
> I wonder if `nr_pmus` makes sense, since the column would be shared
> with multiple different pmus. WDYT?

So each PMU in sysfs has a cpumask that specifies which CPUs perf
should pass to perf_event_open. For example, on a two socket machine
the cpumask will typically have the first CPU of each socket. If the
cpumask (or cpus) file isn't present then the cpumask is implicitly
all online CPUs. Given that the aggregation number is the number of
CPUs in the cpumask multiplied by the number of PMUs, I think the most
neutral name is probably "counters" possibly shortened down to "ctrs".
I suspect others have better suggestions :-)

Thanks,
Ian

> -CT
>
> On Wed, Jun 11, 2025 at 5:16 PM Ian Rogers <irogers@...gle.com> wrote:
> >
> > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@...gle.com> wrote:
> > >
> > > Follow up:
> > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@...l.gmail.com/
> > >
> > > The patch adds unit aggregation during evsel merge the aggregated uncore
> > > counters.
> > >
> > > Tested on a 2-socket machine with SNC3, uncore_imc_[0-11] and
> > > cpumask="0,120"
> > > Before:
> > >   perf stat -e clockticks -I 1000 --per-socket
> > >   #           time socket cpus             counts unit events
> > >        1.001085024 S0        1         9615386315      clockticks
> > >        1.001085024 S1        1         9614287448      clockticks
> > >   perf stat -e clockticks -I 1000 --per-node
> > >   #           time node   cpus             counts unit events
> > >        1.001029867 N0        1         3205726984      clockticks
> > >        1.001029867 N1        1         3205444421      clockticks
> > >        1.001029867 N2        1         3205234018      clockticks
> > >        1.001029867 N3        1         3205224660      clockticks
> > >        1.001029867 N4        1         3205207213      clockticks
> > >        1.001029867 N5        1         3205528246      clockticks
> > > After:
> > >   perf stat -e clockticks -I 1000 --per-socket
> > >   #           time socket cpus             counts unit events
> >
> > I wonder if there is a better column heading than "cpus" given that
> > these are imc PMUs.
> >
> > >        1.001022937 S0       12         9621463177      clockticks
> > >        1.001022937 S1       12         9619804949      clockticks
> > >   perf stat -e clockticks -I 1000 --per-node
> > >   #           time node   cpus             counts unit events
> > >        1.001029867 N0        4         3206782080      clockticks
> > >        1.001029867 N1        4         3207025354      clockticks
> > >        1.001029867 N2        4         3207067946      clockticks
> > >        1.001029867 N3        4         3206871733      clockticks
> > >        1.001029867 N4        4         3206199005      clockticks
> > >        1.001029867 N5        4         3205525058      clockticks
> > >
> > > Suggested-by: Ian Rogers <irogers@...gle.com>
> > > Signed-off-by: Chun-Tse Shao <ctshao@...gle.com>
>
> Added Namhyung's ack from the previous email.
> Acked-by: Namhyung Kim <namhyung@...nel.org>
>
> >
> > Tested-by: Ian Rogers <irogers@...gle.com>
> >
> > Thanks,
> > Ian
> >
> > > ---
> > >  tools/perf/util/stat.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> > > index 355a7d5c8ab8..52266d773353 100644
> > > --- a/tools/perf/util/stat.c
> > > +++ b/tools/perf/util/stat.c
> > > @@ -527,6 +527,7 @@ static int evsel__merge_aggr_counters(struct evsel *evsel, struct evsel *alias)
> > >                 struct perf_counts_values *aggr_counts_b = &ps_b->aggr[i].counts;
> > >
> > >                 /* NB: don't increase aggr.nr for aliases */
> > > +               ps_a->aggr[i].nr += ps_b->aggr[i].nr;
> > >
> > >                 aggr_counts_a->val += aggr_counts_b->val;
> > >                 aggr_counts_a->ena += aggr_counts_b->ena;
> > > --
> > > 2.50.0.rc1.591.g9c95f17f64-goog
> > >