mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
To: Thomas Falcon <thomas.falcon@intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>
Subject: Re: [PATCH v10 4/6] perf tools: Show memory region in perf-c2c subcommand
Date: Thu, 17 Sep 2026 14:04:12 +0800	[thread overview]
Message-ID: <a48ec747-c2d3-496a-9e4e-210352973652@linux.intel.com> (raw)
In-Reply-To: <20260916163127.318885-5-thomas.falcon@intel.com>


On 9/17/2026 12:31 AM, Thomas Falcon wrote:
> From: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> Add memory region field to the cacheline list view to help users
> identify the memory region to which the cacheline belongs. The memory
> region field was included with the introduction of support for the
> Off-module Response facility (OMR) [1] in Intel's Diamond Rapids and
> Nova Lake architectures.
>
> An example of the new perf c2c output including the memory region
> field is shown below:
>
> Shared Data Cache Line Table     (176 entries, sorted on Total HITMs)
>        ------------------------ Cacheline -----------------------      Tot  ------- Load Hitm -------    Total    Total    Total
> Index             Address                    Region  Node  PA cnt     Hitm    Total  LclHitm  RmtHitm  records    Loads   Stores
>     0  0xffffffff98c58ec0                       N/A     0    1208    4.06%       97       97        0     1602     1602        0
> ...
>   145  0xff332b267b5b3880                     Mem-0     0       1    0.13%        3        3        0       12       12        0
>   146  0xff332b267b6b3880        Local-shared-cache     0       1    0.13%        3        3        0       11       11        0
>   147  0xff332b267b9b3880                       N/A     0       1    0.13%        3        3        0        8        8        0
>   148  0xff332b267ba33880        Other-shared-cache     0       1    0.13%        3        3        0        7        7        0
>   149  0xff332b267bab3880        Local-shared-cache     0       1    0.13%        3        3        0        6        6        0
>   150  0xff332b267bbb3880                       N/A     0       1    0.13%        3        3        0        8        8        0
>   151  0xff332b267bd33880    Local-non-shared-cache     0       1    0.13%        3        3        0       11       11        0
>   152  0xff332b267be33880                       N/A     0       3    0.13%        3        3        0       10       10        1
>   153  0xff332b267bf33880    Local-non-shared-cache     0       1    0.13%        3        3        0       10       10        0
>
> [1]: https://lore.kernel.org/all/20260114011750.350569-1-dapeng1.mi@linux.intel.com/
>
> Assisted-by: Sashiko:gemini-3.1-pro-preview
> Assisted-by: GitHub-Copilot:claude-opus-4-8
> Reviewed-by: Ian Rogers <irogers@google.com>
> Reviewed-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Co-developed-by: Thomas Falcon <thomas.falcon@intel.com>
> Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
> ---
> v10: only update mem_region for real cache/memory regions so N/A samples
>      don't overwrite a valid region; add #include <stdio.h> for asprintf
>      (Sashiko)
> v9: Update perf-c2c to include cache region reporting (Dapeng Mi)
> v8: Update developer tags and commit message with real example
>     output (Dapeng Mi)
> v7: fix output_str allocation error handling which introduced
>     a memory leak (Sashiko)
> v6: rebased onto 7.3-rc1
> v5: make the cacheline header span and ui_quirks() width
>     fixup depend on memory-region availability (Namhyung Kim)
> v4: correctly handle output_str memory allocation failure
> v3: make memory region reporting conditional on feature bit
> ---
>  tools/perf/builtin-c2c.c | 129 +++++++++++++++++++++++++++++++++++----
>  tools/perf/util/c2c.h    |   1 +
>  2 files changed, 119 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 6b64c0d6b24f..ad3b63acd0ee 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -14,6 +14,7 @@
>  #include <inttypes.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <stdio.h>
>  
>  #include <asm/bug.h>
>  #include <linux/compiler.h>
> @@ -72,6 +73,7 @@ struct perf_c2c {
>  
>  	bool			 show_src;
>  	bool			 show_all;
> +	bool			 show_mem_region;
>  	bool			 use_stdio;
>  	bool			 stats_only;
>  	bool			 symbol_full;
> @@ -248,6 +250,18 @@ static void c2c_he__set_node(struct c2c_hist_entry *c2c_he,
>  	}
>  }
>  
> +static void c2c_he__set_mem_region(struct c2c_hist_entry *c2c_he,
> +				   unsigned int mem_region)
> +{
> +	if (WARN_ONCE(mem_region > PERF_MEM_REGION_MEM7,
> +		      "WARNING: invalid memory region ID\n"))
> +		return;
> +
> +	/* Update mem_region only if it really accesses cache/memory */
> +	if (mem_region >= PERF_MEM_REGION_L_SHARE)
> +		c2c_he->mem_region = mem_region;

This reminds me whether we should avoid the real memory region is
overwritten by cache region. Since an address could be sampled multiple
times, sometimes it hits in cache and sometimes it misses the cache and
need to really access the memory. Keeping the memory region ID if there is
makes users know exactly where the address belongs to.

Thanks.


> +}
> +
>  static void compute_stats(struct c2c_hist_entry *c2c_he,
>  			  struct c2c_stats *stats,
>  			  u64 weight)
> @@ -306,6 +320,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  	struct addr_location al;
>  	struct mem_info *mi = NULL;
>  	struct callchain_cursor *cursor;
> +	unsigned int mem_region;
>  	int ret;
>  
>  	addr_location__init(&al);
> @@ -333,6 +348,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  	}
>  
>  	c2c_decode_stats(&stats, mi);
> +	mem_region = mem_info__data_src(mi)->mem_region;
>  
>  	he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
>  				  &al, NULL, NULL, mi, NULL,
> @@ -349,6 +365,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  	c2c_he__set_cpu(c2c_he, sample);
>  	c2c_he__set_node(c2c_he, sample);
>  	c2c_he__set_evsel(c2c_he, evsel);
> +	c2c_he__set_mem_region(c2c_he, mem_region);
>  
>  	hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
>  
> @@ -402,6 +419,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  		c2c_he__set_cpu(c2c_he, sample);
>  		c2c_he__set_node(c2c_he, sample);
>  		c2c_he__set_evsel(c2c_he, evsel);
> +		c2c_he__set_mem_region(c2c_he, mem_region);
>  
>  		hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
>  		ret = hist_entry__append_callchain(he, sample);
> @@ -540,6 +558,60 @@ dcacheline_node_count(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  	return scnprintf(hpp->buf, hpp->size, "%*lu", width, c2c_he->paddr_cnt);
>  }
>  
> +static int
> +dcacheline_node_mem_region(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> +			   struct hist_entry *he)
> +{
> +	int width = c2c_width(fmt, hpp, he->hists);
> +	struct c2c_hist_entry *c2c_he;
> +	unsigned int mem_region;
> +	char buf[30];
> +
> +	c2c_he = container_of(he, struct c2c_hist_entry, he);
> +	mem_region = c2c_he->mem_region;
> +
> +	switch (mem_region) {
> +	case PERF_MEM_REGION_NA:
> +	case PERF_MEM_REGION_RSVD:
> +		scnprintf(buf, sizeof(buf), "N/A");
> +		break;
> +	case PERF_MEM_REGION_L_SHARE:
> +		scnprintf(buf, sizeof(buf), "Local-shared-cache");
> +		break;
> +	case PERF_MEM_REGION_L_NON_SHARE:
> +		scnprintf(buf, sizeof(buf), "Local-non-shared-cache");
> +		break;
> +	case PERF_MEM_REGION_O_IO:
> +		scnprintf(buf, sizeof(buf), "Other-IO");
> +		break;
> +	case PERF_MEM_REGION_O_SHARE:
> +		scnprintf(buf, sizeof(buf), "Other-shared-cache");
> +		break;
> +	case PERF_MEM_REGION_O_NON_SHARE:
> +		scnprintf(buf, sizeof(buf), "Other-non-shared-cache");
> +		break;
> +	case PERF_MEM_REGION_MMIO:
> +		scnprintf(buf, sizeof(buf), "MMIO");
> +		break;
> +	case PERF_MEM_REGION_MEM0:
> +	case PERF_MEM_REGION_MEM1:
> +	case PERF_MEM_REGION_MEM2:
> +	case PERF_MEM_REGION_MEM3:
> +	case PERF_MEM_REGION_MEM4:
> +	case PERF_MEM_REGION_MEM5:
> +	case PERF_MEM_REGION_MEM6:
> +	case PERF_MEM_REGION_MEM7:
> +		scnprintf(buf, sizeof(buf), "Mem-%d",
> +			  mem_region - PERF_MEM_REGION_MEM0);
> +		break;
> +	default:
> +		scnprintf(buf, sizeof(buf), "N/A");
> +		break;
> +	}
> +
> +	return scnprintf(hpp->buf, hpp->size, "%*s", width, buf);
> +}
> +
>  static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  			struct hist_entry *he)
>  {
> @@ -1360,6 +1432,14 @@ static struct c2c_dimension dim_dcacheline_node = {
>  	.width		= 4,
>  };
>  
> +static struct c2c_dimension dim_dcacheline_mem_region = {
> +	.header		= HEADER_LOW("Region"),
> +	.name		= "dcacheline_mem_region",
> +	.cmp		= empty_cmp,
> +	.entry		= dcacheline_node_mem_region,
> +	.width		= 24,
> +};
> +
>  static struct c2c_dimension dim_dcacheline_count = {
>  	.header		= HEADER_LOW("PA cnt"),
>  	.name		= "dcacheline_count",
> @@ -1791,6 +1871,7 @@ static struct c2c_dimension dim_dcacheline_num_empty = {
>  
>  static struct c2c_dimension *dimensions[] = {
>  	&dim_dcacheline,
> +	&dim_dcacheline_mem_region,
>  	&dim_dcacheline_node,
>  	&dim_dcacheline_count,
>  	&dim_offset,
> @@ -2854,8 +2935,11 @@ static int ui_quirks(void)
>  	/* Fix the zero line for dcacheline column. */
>  	buf = fill_line(chk_double_cl ? "Double-Cacheline" : "Cacheline",
>  				dim_dcacheline.width +
> +				(c2c.show_mem_region ?
> +					dim_dcacheline_mem_region.width : 0) +
>  				dim_dcacheline_node.width +
> -				dim_dcacheline_count.width + 4);
> +				dim_dcacheline_count.width +
> +				(c2c.show_mem_region ? 6 : 4));
>  	if (!buf)
>  		return -ENOMEM;
>  
> @@ -3107,7 +3191,8 @@ static int perf_c2c__report(int argc, const char **argv)
>  	OPT_END()
>  	};
>  	int err = 0;
> -	const char *output_str, *sort_str = NULL;
> +	const char *sort_str = NULL;
> +	char *output_str = NULL;
>  	struct perf_env *env;
>  
>  	annotation_options__init();
> @@ -3272,9 +3357,16 @@ static int perf_c2c__report(int argc, const char **argv)
>  		goto out_mem2node;
>  	}
>  
> -	if (c2c.display != DISPLAY_SNP_PEER)
> -		output_str = "cl_idx,"
> +	c2c.show_mem_region = perf_header__has_feat(&session->header,
> +						HEADER_MEMORY_RANGES);
> +	if (c2c.show_mem_region)
> +		dim_dcacheline.header.line[0].span = 3;
> +
> +	if (c2c.display != DISPLAY_SNP_PEER) {
> +		if (asprintf(&output_str,
> +			     "cl_idx,"
>  			     "dcacheline,"
> +			     "%s"
>  			     "dcacheline_node,"
>  			     "dcacheline_count,"
>  			     "percent_costly_snoop,"
> @@ -3286,10 +3378,17 @@ static int perf_c2c__report(int argc, const char **argv)
>  			     "ld_fbhit,ld_l1hit,ld_l2hit,"
>  			     "ld_lclhit,lcl_hitm,"
>  			     "ld_rmthit,rmt_hitm,"
> -			     "dram_lcl,dram_rmt";
> -	else
> -		output_str = "cl_idx,"
> +			     "dram_lcl,dram_rmt",
> +			     c2c.show_mem_region ?
> +			     "dcacheline_mem_region," : "") < 0) {
> +			err = -ENOMEM;
> +			goto out_mem2node;
> +		}
> +	} else {
> +		if (asprintf(&output_str,
> +			     "cl_idx,"
>  			     "dcacheline,"
> +			     "%s"
>  			     "dcacheline_node,"
>  			     "dcacheline_count,"
>  			     "percent_costly_snoop,"
> @@ -3301,7 +3400,13 @@ static int perf_c2c__report(int argc, const char **argv)
>  			     "ld_fbhit,ld_l1hit,ld_l2hit,"
>  			     "ld_lclhit,lcl_hitm,"
>  			     "ld_rmthit,rmt_hitm,"
> -			     "dram_lcl,dram_rmt";
> +			     "dram_lcl,dram_rmt",
> +			     c2c.show_mem_region ?
> +			     "dcacheline_mem_region," : "") < 0) {
> +			err = -ENOMEM;
> +			goto out_mem2node;
> +		}
> +	}
>  
>  	if (c2c.display == DISPLAY_TOT_HITM)
>  		sort_str = "tot_hitm";
> @@ -3315,7 +3420,7 @@ static int perf_c2c__report(int argc, const char **argv)
>  	err = c2c_hists__reinit(&c2c.hists, output_str, sort_str, perf_session__env(session));
>  	if (err) {
>  		pr_err("Failed to reinitialize hists\n");
> -		goto out_mem2node;
> +		goto out_str;
>  	}
>  
>  	ui_progress__init(&prog, c2c.hists.hists.nr_entries, "Sorting...");
> @@ -3324,17 +3429,19 @@ static int perf_c2c__report(int argc, const char **argv)
>  	hists__output_resort_cb(&c2c.hists.hists, &prog, resort_shared_cl_cb);
>  	err = hists__iterate_cb(&c2c.hists.hists, resort_cl_cb, perf_session__env(session));
>  	if (err)
> -		goto out_mem2node;
> +		goto out_str;
>  
>  	ui_progress__finish();
>  
>  	if (ui_quirks()) {
>  		pr_err("failed to setup UI\n");
> -		goto out_mem2node;
> +		goto out_str;
>  	}
>  
>  	perf_c2c_display(session);
>  
> +out_str:
> +	free(output_str);
>  out_mem2node:
>  	mem2node__exit(&c2c.mem2node);
>  out_session:
> diff --git a/tools/perf/util/c2c.h b/tools/perf/util/c2c.h
> index 53f024e25d99..f04e78e1a2e3 100644
> --- a/tools/perf/util/c2c.h
> +++ b/tools/perf/util/c2c.h
> @@ -33,6 +33,7 @@ struct c2c_hist_entry {
>  	unsigned long		*nodeset;
>  	struct c2c_stats	*node_stats;
>  	unsigned int		 cacheline_idx;
> +	unsigned int		 mem_region;
>  
>  	struct compute_stats	 cstats;
>  

  reply	other threads:[~2026-09-17  6:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 16:31 [PATCH v10 0/6] perf: Add support for memory region/range reporting Thomas Falcon
2026-09-16 16:31 ` [PATCH v10 1/6] perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf() Thomas Falcon
2026-09-16 16:31 ` [PATCH v10 2/6] perf mem: Add support for printing PERF_MEM_LVLNUM_L0 Thomas Falcon
2026-09-16 16:31 ` [PATCH v10 3/6] perf header: Support memory ranges Thomas Falcon
2026-09-16 16:31 ` [PATCH v10 4/6] perf tools: Show memory region in perf-c2c subcommand Thomas Falcon
2026-09-17  6:04   ` Mi, Dapeng [this message]
2026-09-17 17:37     ` Falcon, Thomas
2026-09-16 16:31 ` [PATCH v10 5/6] perf tools: Show memory region in perf-script subcommand Thomas Falcon
2026-09-16 16:31 ` [PATCH v10 6/6] perf c2c: print memory region data with stdio output Thomas Falcon

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=a48ec747-c2d3-496a-9e4e-210352973652@linux.intel.com \
    --to=dapeng1.mi@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=thomas.falcon@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®