Re: [PATCH v5 1/2] perf pmu intel: Generalize SNC cpumask adjustment for multiple platforms

Namhyung Kim
2026-04-10T04:39:56Z

Hello,

On Tue, Apr 07, 2026 at 01:38:42PM -0700, Chun-Tse Shao wrote:
> Prepare for supporting more Intel platforms with sub-NUMA clustering by
> generalizing the GNR specific logic.
>
> Signed-off-by: Chun-Tse Shao <ctshao@google.com>
> ---
> v5:
> Split patch.
>
> v4: lore.kernel.org/20260402205300.1953706-1-ctshao@google.com
> Rebase.
>
> v3: lore.kernel.org/20260212223942.3832857-1-ctshao@google.com
> Fix a typo.
>
> v2: lore.kernel.org/20260205232220.1980168-1-ctshao@google.com
> Split EMR and GNR in the SNC2 IMC cpu map.
>
> v1: lore.kernel.org/20260108184430.1210223-1-ctshao@google.com
>
> tools/perf/arch/x86/util/pmu.c | 44 +++++++++++++++++++++-------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index 0661e0f0b02d..938be36ec0f7 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c
> @@ -23,20 +23,28 @@
> #include "util/env.h"
> #include "util/header.h"
>
> -static bool x86__is_intel_graniterapids(void)
> +static bool x86__is_snc_supported(void)
> {
> - static bool checked_if_graniterapids;
> - static bool is_graniterapids;
> + static bool checked_if_snc_supported;
> + static bool is_supported;
>
> - if (!checked_if_graniterapids) {
> - const char *graniterapids_cpuid = "GenuineIntel-6-A[DE]";
> + if (!checked_if_snc_supported) {
> +
> + /* Graniterapids supports SNC configuration. */
> + static const char *const supported_cpuids[] = {
> + "GenuineIntel-6-A[DE]", /* Graniterapids */
> + };
> char *cpuid = get_cpuid_str((struct perf_cpu){0});
>
> - is_graniterapids = cpuid && strcmp_cpuid_str(graniterapids_cpuid, cpuid) == 0;
> + for (size_t i = 0; i < ARRAY_SIZE(supported_cpuids); i++) {
> + is_supported = cpuid && strcmp_cpuid_str(supported_cpuids[i], cpuid) == 0;
> + if (is_supported)
> + break;
> + }
> free(cpuid);
> - checked_if_graniterapids = true;
> + checked_if_snc_supported = true;
> }
> - return is_graniterapids;
> + return is_supported;
> }
>
> static struct perf_cpu_map *read_sysfs_cpu_map(const char *sysfs_path)
> @@ -133,8 +141,8 @@ static int uncore_imc_snc(struct perf_pmu *pmu)
> // Compute the IMC SNC using lookup tables.
> unsigned int imc_num;
> int snc_nodes = snc_nodes_per_l3_cache();
> - const u8 snc2_map[] = {1, 1, 0, 0, 1, 1, 0, 0};
> - const u8 snc3_map[] = {1, 1, 0, 0, 2, 2, 1, 1, 0, 0, 2, 2};
> + const u8 snc2_map[] = {1, 1, 0, 0};
> + const u8 snc3_map[] = {1, 1, 0, 0, 2, 2};
> const u8 *snc_map;
> size_t snc_map_len;
>
> @@ -157,11 +165,12 @@ static int uncore_imc_snc(struct perf_pmu *pmu)
> pr_warning("Unexpected: unable to compute IMC number '%s'\n", pmu->name);
> return 0;
> }
> - if (imc_num >= snc_map_len) {
> + if (imc_num >= snc_map_len * perf_cpu_map__nr(pmu->cpus)) {
> pr_warning("Unexpected IMC %d for SNC%d mapping\n", imc_num, snc_nodes);
> return 0;

Like sashiko said, I'm curious if it'd work well on 1-socket machine
which may have the same number of uncore IMC PMUs.

Zide, can you confirm?

Thanks,
Namhyung

> }
> - return snc_map[imc_num];
> +
> + return snc_map[imc_num % snc_map_len];
> }
>
> static int uncore_cha_imc_compute_cpu_adjust(int pmu_snc)
> @@ -201,7 +210,7 @@ static int uncore_cha_imc_compute_cpu_adjust(int pmu_snc)
> return cpu_adjust[pmu_snc];
> }
>
> -static void gnr_uncore_cha_imc_adjust_cpumask_for_snc(struct perf_pmu *pmu, bool cha)
> +static void uncore_cha_imc_adjust_cpumask_for_snc(struct perf_pmu *pmu, bool cha)
> {
> // With sub-NUMA clustering (SNC) there is a NUMA node per SNC in the
> // topology. For example, a two socket graniterapids machine may be set
> @@ -301,11 +310,12 @@ void perf_pmu__arch_init(struct perf_pmu *pmu)
> pmu->mem_events = perf_mem_events_intel_aux;
> else
> pmu->mem_events = perf_mem_events_intel;
> - } else if (x86__is_intel_graniterapids()) {
> + } else if (x86__is_snc_supported()) {
> if (strstarts(pmu->name, "uncore_cha_"))
> - gnr_uncore_cha_imc_adjust_cpumask_for_snc(pmu, /*cha=*/true);
> - else if (strstarts(pmu->name, "uncore_imc_"))
> - gnr_uncore_cha_imc_adjust_cpumask_for_snc(pmu, /*cha=*/false);
> + uncore_cha_imc_adjust_cpumask_for_snc(pmu, /*cha=*/true);
> + else if (strstarts(pmu->name, "uncore_imc_") &&
> + !strstarts(pmu->name, "uncore_imc_free_running"))
> + uncore_cha_imc_adjust_cpumask_for_snc(pmu, /*cha=*/false);
> }
> }
> }
> --
> 2.53.0.1213.gd9a14994de-goog
>