Re: [PATCH v10 4/6] perf tools: Show memory region in perf-c2c subcommand
From: Falcon, Thomas
Date: Thu Sep 17 2026 - 15:22:33 EST
On Thu, 2026-09-17 at 14:04 +0800, Mi, Dapeng wrote:
>
> On 9/17/2026 12:31 AM, Thomas Falcon wrote:
> > From: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx/
> >
> > Assisted-by: Sashiko:gemini-3.1-pro-preview
> > Assisted-by: GitHub-Copilot:claude-opus-4-8
> > Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
> > Reviewed-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> > Co-developed-by: Thomas Falcon <thomas.falcon@xxxxxxxxx>
> > Signed-off-by: Thomas Falcon <thomas.falcon@xxxxxxxxx>
> > ---
> > 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.
Would you prefer something like this?
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index ad3b63acd0ee..8acf6f632156 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -257,8 +257,14 @@ static void c2c_he__set_mem_region(struct c2c_hist_entry *c2c_he,
"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)
+ /*
+ * Update mem_region only if it really accesses memory.
+ * Do not overwrite a memory region with a cache region.
+ */
+
+ if (mem_region >= PERF_MEM_REGION_MMIO ||
+ (mem_region >= PERF_MEM_REGION_L_SHARE &&
+ !(c2c_he->mem_region >= PERF_MEM_REGION_MMIO)))
c2c_he->mem_region = mem_region;
}
Tom
>
> 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;
> >