All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Chun-Tse Shao" <ctshao@google.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v6 2/2] perf pmu intel: Adjust cpumasks for sub-NUMA clusters on Sapphire Rapids and Emerald Rapids
Date: Fri, 15 May 2026 18:23:04 +0000	[thread overview]
Message-ID: <20260515182305.8D9AEC2BCB0@smtp.kernel.org> (raw)
In-Reply-To: <20260515172710.428474-2-ctshao@google.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing IMC cpumask adjustment and silent failure for SNC4 configurations on Sapphire Rapids.
--

commit 312ea8a1894ae6a4d9318072afccaf0abbbb29bd
Author: Chun-Tse Shao <ctshao@google.com>

perf pmu intel: Adjust cpumasks for sub-NUMA clusters on Sapphire Rapids and Emerald Rapids

This commit adjusts the cpumasks for sub-NUMA clusters on Sapphire Rapids
and Emerald Rapids processors. It updates the detection logic and the
mapping arrays to support SNC configurations for these architectures.

> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index b8fab260c4238..09caec0a32a57 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c
[ ... ]
> @@ -142,23 +146,43 @@ 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();
[ ... ]
> +	/* snc_map is not inited yet. We only look up once to avoid expensive operations. */
> +	if (!snc_map) {
> +		switch (snc_nodes) {
> +		case 2:
[ ... ]
> +			break;
> +		case 3:
> +			snc_map = snc3_map;
> +			snc_map_len = ARRAY_SIZE(snc3_map);
> +			break;
> +		default:
> +			/* Error or no lookup support for SNC with >3 nodes. */
> +			return 0;

Sapphire Rapids natively supports 4-way sub-NUMA clustering, which would
make snc_nodes evaluate to 4. Does this regression silently skip IMC
cpumask adjustments for SNC4 configurations?

Because this default case returns 0 immediately, it entirely bypasses the
newly added warning block below.

> +		}
> +
> +		if (!snc_map) {
> +			pr_warning("Unexpected: can not find snc map config");
> +			return 0;
> +		}

Could this leave users unaware that the IMC PMUs are unmapped, even though
CHA PMUs are correctly adjusted for SNC4 elsewhere?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515172710.428474-1-ctshao@google.com?part=2

  reply	other threads:[~2026-05-15 18:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 17:26 [PATCH v6 1/2] perf pmu intel: Generalize SNC cpumask adjustment for multiple platforms Chun-Tse Shao
2026-05-15 17:26 ` [PATCH v6 2/2] perf pmu intel: Adjust cpumasks for sub-NUMA clusters on Sapphire Rapids and Emerald Rapids Chun-Tse Shao
2026-05-15 18:23   ` sashiko-bot [this message]
2026-05-15 17:59 ` [PATCH v6 1/2] perf pmu intel: Generalize SNC cpumask adjustment for multiple platforms sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260515182305.8D9AEC2BCB0@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ctshao@google.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.