All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dapeng Mi" <dapeng1.mi@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [Patch v2 5/5] perf c2c: Rename "Region" column to "Range" for consistency
Date: Mon, 09 Nov 2026 01:09:49 +0000	[thread overview]
Message-ID: <sashiko-outbox-204282@kernel.org> (raw)
In-Reply-To: <20261109004402.1403713-6-dapeng1.mi@linux.intel.com>

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf c2c: Range column header conflates region IDs with MRRM
  range indices
--

commit 359275a3e2aec93ecd4fbf0366bae0325d3a6862
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf c2c: Rename "Region" column to "Range" for consistency

This commit renames the header of dim_dcacheline_mem_region and
dim_offset_mem_region from "Region" to "Range" and reduces their column
width from 6 to 5.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index a2dcc65d6b3ce..94fca3e4fc0a0 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -1395,11 +1395,11 @@ static struct c2c_dimension dim_dcacheline_node = {
>  };
>  
>  static struct c2c_dimension dim_dcacheline_mem_region = {
> -	.header		= HEADER_LOW("Region"),
> +	.header		= HEADER_LOW("Range"),
>  	.name		= "dcacheline_mem_region",
>  	.cmp		= empty_cmp,
>  	.entry		= dcacheline_node_mem_region,
> -	.width		= 6,
> +	.width		= 5,
>  };
>  
>  static struct c2c_dimension dim_dcacheline_count = {
> @@ -1429,11 +1429,11 @@ static struct c2c_dimension dim_offset_node = {
>  };
>  
>  static struct c2c_dimension dim_offset_mem_region = {
> -	.header		= HEADER_LOW("Region"),
> +	.header		= HEADER_LOW("Range"),
>  	.name		= "offset_mem_region",
>  	.cmp		= empty_cmp,
>  	.entry		= dcacheline_node_mem_region,
> -	.width		= 6,
> +	.width		= 5,
>  };

[Severity: Low]
Does renaming this column to "Range" conflate hardware memory region IDs
with the range indices printed in the Memory Ranges table?

set_mem_region_str() formats the sampled PERF_MEM_REGION_MEM0..MEM7 bits as
numeric region IDs (0..7), whereas print_memory_ranges_info() prints each
ACPI MRRM entry with both a Range index and region IDs:

tools/perf/builtin-c2c.c:print_memory_ranges_info() {
    ...
		fprintf(out, "Range %d: [0x%016" PRIx64 "-0x%016" PRIx64 "] Node %d, local region id %u, remote region id %u\n",
			i, r->base, r->base + r->length - 1, r->node,
			r->local_region_id, r->remote_region_id);
    ...
}

When multiple physical address ranges map to the same region ID (for
example, Range 0 and Range 1 on local region id 0, and Range 2 on local
region id 1), labeling the region ID column "Range" causes a reported value
of 1 to look like Range 1 rather than region id 1.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109004402.1403713-1-dapeng1.mi@linux.intel.com?part=5

  reply	other threads:[~2026-11-09  1:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  0:43 [Patch v2 0/5] perf: fix test error and improve c2c memory-region display Dapeng Mi
2026-11-09  0:43 ` [Patch v2 1/5] perf tests: Skip zero sample IP in object-code reading test Dapeng Mi
2026-11-09  0:55   ` sashiko-bot
2026-11-09  0:43 ` [Patch v2 2/5] perf c2c: Fix top-level histogram ownership check Dapeng Mi
2026-11-09  0:55   ` sashiko-bot
2026-11-09  0:44 ` [Patch v2 3/5] perf c2c: Show memory region in the per-cacheline view Dapeng Mi
2026-11-09  0:59   ` sashiko-bot
2026-11-09  0:44 ` [Patch v2 4/5] perf c2c: Show all memory regions in the region column Dapeng Mi
2026-11-09  0:59   ` sashiko-bot
2026-11-09  0:44 ` [Patch v2 5/5] perf c2c: Rename "Region" column to "Range" for consistency Dapeng Mi
2026-11-09  1:09   ` sashiko-bot [this message]
2026-11-09  1:32     ` Mi, Dapeng

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=sashiko-outbox-204282@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dapeng1.mi@linux.intel.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.