* [PATCH v1] perf stat: Fix uncore aggregation number
@ 2025-06-11 23:32 Chun-Tse Shao
2025-06-11 23:43 ` Namhyung Kim
2025-06-12 0:16 ` Ian Rogers
0 siblings, 2 replies; 10+ messages in thread
From: Chun-Tse Shao @ 2025-06-11 23:32 UTC (permalink / raw)
To: linux-kernel
Cc: Chun-Tse Shao, Ian Rogers, peterz, mingo, acme, namhyung,
mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang,
linux-perf-users
Follow up:
lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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
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@google.com>
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-11 23:32 [PATCH v1] perf stat: Fix uncore aggregation number Chun-Tse Shao @ 2025-06-11 23:43 ` Namhyung Kim 2025-06-12 0:16 ` Ian Rogers 1 sibling, 0 replies; 10+ messages in thread From: Namhyung Kim @ 2025-06-11 23:43 UTC (permalink / raw) To: Chun-Tse Shao Cc: linux-kernel, Ian Rogers, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users Hello, On Wed, Jun 11, 2025 at 04:32:16PM -0700, Chun-Tse Shao wrote: > Follow up: > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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 > 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@google.com> > Signed-off-by: Chun-Tse Shao <ctshao@google.com> Acked-by: Namhyung Kim <namhyung@kernel.org> > --- > 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 */ Probably we need to delete this comment. Thanks, namhyung > + 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-11 23:32 [PATCH v1] perf stat: Fix uncore aggregation number Chun-Tse Shao 2025-06-11 23:43 ` Namhyung Kim @ 2025-06-12 0:16 ` Ian Rogers 2025-06-12 3:18 ` Chun-Tse Shao 1 sibling, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-06-12 0:16 UTC (permalink / raw) To: Chun-Tse Shao Cc: linux-kernel, peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > Follow up: > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > Signed-off-by: Chun-Tse Shao <ctshao@google.com> Tested-by: Ian Rogers <irogers@google.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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-12 0:16 ` Ian Rogers @ 2025-06-12 3:18 ` Chun-Tse Shao 2025-06-12 5:12 ` Ian Rogers 0 siblings, 1 reply; 10+ messages in thread From: Chun-Tse Shao @ 2025-06-12 3:18 UTC (permalink / raw) To: Ian Rogers Cc: linux-kernel, peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users Thanks for your test, Ian! I wonder if `nr_pmus` makes sense, since the column would be shared with multiple different pmus. WDYT? -CT On Wed, Jun 11, 2025 at 5:16 PM Ian Rogers <irogers@google.com> wrote: > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > Follow up: > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> Added Namhyung's ack from the previous email. Acked-by: Namhyung Kim <namhyung@kernel.org> > > Tested-by: Ian Rogers <irogers@google.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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-12 3:18 ` Chun-Tse Shao @ 2025-06-12 5:12 ` Ian Rogers 2025-06-12 22:55 ` Chun-Tse Shao 0 siblings, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-06-12 5:12 UTC (permalink / raw) To: Chun-Tse Shao Cc: linux-kernel, peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > Follow up: > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > Added Namhyung's ack from the previous email. > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-12 5:12 ` Ian Rogers @ 2025-06-12 22:55 ` Chun-Tse Shao 2025-06-20 22:12 ` Namhyung Kim 0 siblings, 1 reply; 10+ messages in thread From: Chun-Tse Shao @ 2025-06-12 22:55 UTC (permalink / raw) To: Ian Rogers Cc: linux-kernel, peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users Hi Ian, I actually renamed it to `aggr_nr` in v2 patch so it should be better aligned to json mode, which is using `aggregate-nunber`. But anyway I think any name other than `cpus` is better. v2 patch: lore.kernel.org/20250612225324.3315450-1-ctshao@google.com Thanks, CT On Wed, Jun 11, 2025 at 10:12 PM Ian Rogers <irogers@google.com> wrote: > > On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > > > Follow up: > > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > > > Added Namhyung's ack from the previous email. > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-12 22:55 ` Chun-Tse Shao @ 2025-06-20 22:12 ` Namhyung Kim 2025-06-23 18:17 ` Ian Rogers 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2025-06-20 22:12 UTC (permalink / raw) To: Chun-Tse Shao Cc: Ian Rogers, linux-kernel, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users Hi CT, On Thu, Jun 12, 2025 at 03:55:59PM -0700, Chun-Tse Shao wrote: > Hi Ian, I actually renamed it to `aggr_nr` in v2 patch so it should be > better aligned to json mode, which is using `aggregate-nunber`. But > anyway I think any name other than `cpus` is better. > v2 patch: lore.kernel.org/20250612225324.3315450-1-ctshao@google.com I think "aggregation-count" is a better name, maybe abbreviated to "ag_cnt". Can we rename the JSON as well? I'm not sure if it's documented somewhere. Thanks, Namhyung > > > On Wed, Jun 11, 2025 at 10:12 PM Ian Rogers <irogers@google.com> wrote: > > > > On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > > > > > Follow up: > > > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > > > > > Added Namhyung's ack from the previous email. > > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-20 22:12 ` Namhyung Kim @ 2025-06-23 18:17 ` Ian Rogers 2025-06-23 23:31 ` Namhyung Kim 0 siblings, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-06-23 18:17 UTC (permalink / raw) To: Namhyung Kim Cc: Chun-Tse Shao, linux-kernel, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users On Fri, Jun 20, 2025 at 3:12 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Hi CT, > > On Thu, Jun 12, 2025 at 03:55:59PM -0700, Chun-Tse Shao wrote: > > Hi Ian, I actually renamed it to `aggr_nr` in v2 patch so it should be > > better aligned to json mode, which is using `aggregate-nunber`. But > > anyway I think any name other than `cpus` is better. > > v2 patch: lore.kernel.org/20250612225324.3315450-1-ctshao@google.com > > I think "aggregation-count" is a better name, maybe abbreviated to > "ag_cnt". Can we rename the JSON as well? I'm not sure if it's > documented somewhere. Fwiw, I still think "counters" is a clearer, more intention revealing definition than "aggregation count" or "aggregation number". The term counter appears in the perf-stat and perf_event_open man page. In the perf-stat man page we have a pattern of: --per-XXX Aggregate counts per XXX for system-wide mode measurements. This is a useful mode to .... To enable this mode, use --per-XXX in addition to -a. (system-wide). The output includes the ... This is useful to gauge the amount of aggregation. It seems that the aggregated value of the counters could be confused for being an "aggregation count" or "aggregation number" given the man page definition. I think "counters" to some extent avoids this, we giving the number of counters aggregated. The perf-stat man page could certainly be clearer :-) Thanks, Ian > Thanks, > Namhyung > > > > > > > On Wed, Jun 11, 2025 at 10:12 PM Ian Rogers <irogers@google.com> wrote: > > > > > > On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > > > > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > > > > > > > Follow up: > > > > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > > > > > > > Added Namhyung's ack from the previous email. > > > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > > > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-23 18:17 ` Ian Rogers @ 2025-06-23 23:31 ` Namhyung Kim 2025-06-24 22:24 ` Chun-Tse Shao 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2025-06-23 23:31 UTC (permalink / raw) To: Ian Rogers Cc: Chun-Tse Shao, linux-kernel, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users On Mon, Jun 23, 2025 at 11:17:20AM -0700, Ian Rogers wrote: > On Fri, Jun 20, 2025 at 3:12 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > Hi CT, > > > > On Thu, Jun 12, 2025 at 03:55:59PM -0700, Chun-Tse Shao wrote: > > > Hi Ian, I actually renamed it to `aggr_nr` in v2 patch so it should be > > > better aligned to json mode, which is using `aggregate-nunber`. But > > > anyway I think any name other than `cpus` is better. > > > v2 patch: lore.kernel.org/20250612225324.3315450-1-ctshao@google.com > > > > I think "aggregation-count" is a better name, maybe abbreviated to > > "ag_cnt". Can we rename the JSON as well? I'm not sure if it's > > documented somewhere. > > Fwiw, I still think "counters" is a clearer, more intention revealing > definition than "aggregation count" or "aggregation number". The term > counter appears in the perf-stat and perf_event_open man page. In the > perf-stat man page we have a pattern of: > > --per-XXX > Aggregate counts per XXX for system-wide mode measurements. This is a > useful mode to .... To enable this mode, use --per-XXX > in addition to -a. (system-wide). The output includes the ... > This is useful to gauge the amount of aggregation. > > It seems that the aggregated value of the counters could be confused > for being an "aggregation count" or "aggregation number" given the man > page definition. I think "counters" to some extent avoids this, we > giving the number of counters aggregated. The perf-stat man page could > certainly be clearer :-) I'm fine with "counters" (and "ctrs"). Thanks, Namhyung > > > > > > On Wed, Jun 11, 2025 at 10:12 PM Ian Rogers <irogers@google.com> wrote: > > > > > > > > On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > > > > > > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > > > > > > > > > Follow up: > > > > > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > > > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > > > > > > > > > Added Namhyung's ack from the previous email. > > > > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > > > > > > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] perf stat: Fix uncore aggregation number 2025-06-23 23:31 ` Namhyung Kim @ 2025-06-24 22:24 ` Chun-Tse Shao 0 siblings, 0 replies; 10+ messages in thread From: Chun-Tse Shao @ 2025-06-24 22:24 UTC (permalink / raw) To: Namhyung Kim Cc: Ian Rogers, linux-kernel, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, linux-perf-users Thanks for your suggestions, Namhyung and Ian. I submitted v3 patch: https://lore.kernel.org/linux-perf-users/20250624221545.1711008-1-ctshao@google.com BTW, I just realized the --metric-only option makes the output unaligned. It is hard to show in an email but if you try: `perf stat -M uncore_frequency,uncore_frequency --metric-only -I 1000 --per-socket` You would see the problem. I will take a look once I have a chance. -CT On Mon, Jun 23, 2025 at 4:31 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, Jun 23, 2025 at 11:17:20AM -0700, Ian Rogers wrote: > > On Fri, Jun 20, 2025 at 3:12 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > Hi CT, > > > > > > On Thu, Jun 12, 2025 at 03:55:59PM -0700, Chun-Tse Shao wrote: > > > > Hi Ian, I actually renamed it to `aggr_nr` in v2 patch so it should be > > > > better aligned to json mode, which is using `aggregate-nunber`. But > > > > anyway I think any name other than `cpus` is better. > > > > v2 patch: lore.kernel.org/20250612225324.3315450-1-ctshao@google.com > > > > > > I think "aggregation-count" is a better name, maybe abbreviated to > > > "ag_cnt". Can we rename the JSON as well? I'm not sure if it's > > > documented somewhere. > > > > Fwiw, I still think "counters" is a clearer, more intention revealing > > definition than "aggregation count" or "aggregation number". The term > > counter appears in the perf-stat and perf_event_open man page. In the > > perf-stat man page we have a pattern of: > > > > --per-XXX > > Aggregate counts per XXX for system-wide mode measurements. This is a > > useful mode to .... To enable this mode, use --per-XXX > > in addition to -a. (system-wide). The output includes the ... > > This is useful to gauge the amount of aggregation. > > > > It seems that the aggregated value of the counters could be confused > > for being an "aggregation count" or "aggregation number" given the man > > page definition. I think "counters" to some extent avoids this, we > > giving the number of counters aggregated. The perf-stat man page could > > certainly be clearer :-) > > I'm fine with "counters" (and "ctrs"). > > Thanks, > Namhyung > > > > > > > > > On Wed, Jun 11, 2025 at 10:12 PM Ian Rogers <irogers@google.com> wrote: > > > > > > > > > > On Wed, Jun 11, 2025 at 8:18 PM Chun-Tse Shao <ctshao@google.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@google.com> wrote: > > > > > > > > > > > > > > On Wed, Jun 11, 2025 at 4:36 PM Chun-Tse Shao <ctshao@google.com> wrote: > > > > > > > > > > > > > > > > Follow up: > > > > > > > > lore.kernel.org/CAP-5=fVDF4-qYL1Lm7efgiHk7X=_nw_nEFMBZFMcsnOOJgX4Kg@mail.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@google.com> > > > > > > > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > > > > > > > > > > > Added Namhyung's ack from the previous email. > > > > > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > > > > > > > > > > > > > > > > > Tested-by: Ian Rogers <irogers@google.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 > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-06-24 22:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-11 23:32 [PATCH v1] perf stat: Fix uncore aggregation number Chun-Tse Shao 2025-06-11 23:43 ` Namhyung Kim 2025-06-12 0:16 ` Ian Rogers 2025-06-12 3:18 ` Chun-Tse Shao 2025-06-12 5:12 ` Ian Rogers 2025-06-12 22:55 ` Chun-Tse Shao 2025-06-20 22:12 ` Namhyung Kim 2025-06-23 18:17 ` Ian Rogers 2025-06-23 23:31 ` Namhyung Kim 2025-06-24 22:24 ` Chun-Tse Shao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).