Re: [PATCH v3] perf pmu intel: Adjust cpumaks for sub-NUMA clusters on graniterapids

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Thu, May 15, 2025 at 03:35:41PM -0700, Ian Rogers wrote:
> On Thu, May 15, 2025 at 2:01 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >
> > On 2025-05-15 2:14 p.m., Ian Rogers wrote:
> > > On graniterapids the cache home agent (CHA) and memory controller
> > > (IMC) PMUs all have their cpumask set to per-socket information. In
> > > order for per NUMA node aggregation to work correctly the PMUs cpumask
> > > needs to be set to CPUs for the relevant sub-NUMA grouping.
> > >
> > > For example, on a 2 socket graniterapids machine with sub NUMA
> > > clustering of 3, for uncore_cha and uncore_imc PMUs the cpumask is
> > > "0,120" leading to aggregation only on NUMA nodes 0 and 3:
> > > ```
> > > $ perf stat --per-node -e 'UNC_CHA_CLOCKTICKS,UNC_M_CLOCKTICKS' -a sleep 1
> > >
> > >  Performance counter stats for 'system wide':
> > >
> > > N0        1    277,835,681,344      UNC_CHA_CLOCKTICKS
> > > N0        1     19,242,894,228      UNC_M_CLOCKTICKS
> > > N3        1    277,803,448,124      UNC_CHA_CLOCKTICKS
> > > N3        1     19,240,741,498      UNC_M_CLOCKTICKS
> > >
> > >        1.002113847 seconds time elapsed
> > > ```
> > >
> > > By updating the PMUs cpumasks to "0,120", "40,160" and "80,200" then
> > > the correctly 6 NUMA node aggregations are achieved:
> > > ```
> > > $ perf stat --per-node -e 'UNC_CHA_CLOCKTICKS,UNC_M_CLOCKTICKS' -a sleep 1
> > >
> > >  Performance counter stats for 'system wide':
> > >
> > > N0        1     92,748,667,796      UNC_CHA_CLOCKTICKS
> > > N0        0      6,424,021,142      UNC_M_CLOCKTICKS
> > > N1        0     92,753,504,424      UNC_CHA_CLOCKTICKS
> > > N1        1      6,424,308,338      UNC_M_CLOCKTICKS
> > > N2        0     92,751,170,084      UNC_CHA_CLOCKTICKS
> > > N2        0      6,424,227,402      UNC_M_CLOCKTICKS
> > > N3        1     92,745,944,144      UNC_CHA_CLOCKTICKS
> > > N3        0      6,423,752,086      UNC_M_CLOCKTICKS
> > > N4        0     92,725,793,788      UNC_CHA_CLOCKTICKS
> > > N4        1      6,422,393,266      UNC_M_CLOCKTICKS
> > > N5        0     92,717,504,388      UNC_CHA_CLOCKTICKS
> > > N5        0      6,421,842,618      UNC_M_CLOCKTICKS
> >
> > Is the second coloum  the number of units?
> > If so, it's wrong.
> >
> > On my GNR with SNC-2, I observed the similar issue.
> >
> > $ sudo ./perf stat -e 'UNC_M_CLOCKTICKS' --per-node -a sleep 1
> >  Performance counter stats for 'system wide':
> >
> > N0        0      6,405,811,284      UNC_M_CLOCKTICKS
> > N1        1      6,405,895,988      UNC_M_CLOCKTICKS
> > N2        0      6,152,906,692      UNC_M_CLOCKTICKS
> > N3        1      6,063,415,630      UNC_M_CLOCKTICKS
> >
> > It's supposed to be 4?
> 
> Agreed it is weird, but it is what has historically been displayed.
> The number is the aggregation number:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat-display.c?h=perf-tools-next#n307
> which comes from:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat-display.c?h=perf-tools-next#n135
> which comes from:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next#n435
> However, I think it is missing updates from:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next#n526
> but there is a comment there saying "don't increase aggr.nr for
> aliases" and all the uncore events are aliases. I don't understand
> what the aggregation number is supposed to be, it is commented as
> "number of entries (CPUs) aggregated":
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.h?h=perf-tools-next#n26
> it would seem to make sense in the CHA case with SNC3 and 42 evsels
> per NUMA node that the value should be 42. Maybe Namhyung (who did the
> evsel__merge_aggr_counters clean up) knows why it is this way but in
> my eyes it just seems like something that has been broken for a long
> time.

I think it's the number of aggregated entries (FDs?).

For core events, it's the number of CPUs for the given aggregation as it
collects from all CPUs.  But it'd be confusing with uncore events which
have cpumask to collect data from a few CPUs.

On my laptop, --per-socket gives different numbers depending on the
events/PMUs.

For core events, it's 4.

  $ sudo ./perf stat -a --per-socket -e cycles sleep 1
  
   Performance counter stats for 'system wide':
  
  S0        4        225,297,257      cycles                                                                
  
         1.002581995 seconds time elapsed

While uncore events give 1.

  $ sudo ./perf stat -a --per-socket -e unc_mc0_rdcas_count_freerun sleep 1
  
   Performance counter stats for 'system wide':
  
  S0        1         23,665,510      unc_mc0_rdcas_count_freerun                                           
  
         1.002148012 seconds time elapsed

Thanks,
Namhyung