From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
Cc: Thomas Falcon <thomas.falcon@intel.com>,
sashiko-bot@kernel.org, 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 v12 4/6] perf tools: Show memory region in perf-c2c subcommand
Date: Thu, 24 Sep 2026 19:29:10 +0200 [thread overview]
Message-ID: <arVd5uRfZWRzpaaE@x2> (raw)
In-Reply-To: <arVdq-jyFVdX5QNa@x2>
On Thu, Sep 24, 2026 at 07:28:13PM +0200, Arnaldo Carvalho de Melo wrote:
> On Thu, Sep 24, 2026 at 08:23:42AM +0800, Mi, Dapeng wrote:
> > Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> Is this needed since you're the author of the patch? I'm keeping just
> the Signed-off-by,
I'll keep it since I saw a Co-developed-by tag, meaning maybe there were
changes after you initially created this patch...
- Arnaldo
> >
> > On 9/24/2026 1:09 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:
> > >
> > > # Index Address Region Node PA cnt Hitm Total LclHitm RmtHitm records Loads
> > > # ..... .................. ...... .... ...... ....... ....... ....... ....... ....... .......
> > > #
> > > 0 0xffffffff84eb0040 N/A 0 1205 23.25% 544 544 0 1677 1677
> > > 1 0xffffffff84f03a80 N/A 0 1 18.12% 424 424 0 585 585
> > > [...]
> > > 19 0xffffffff845bc840 Mem-0 0 13 0.30% 7 7 0 31 31
> > > 20 0xff3bbba284520a00 N/A 0 10 0.26% 6 6 0 11 11
> > > 21 0xff3bbbe0bc833500 N/A 0 24 0.26% 6 6 0 30 30
> > >
> > > [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>
> > > ---
> > > v11: perf-c2c: revert to only recording a real memory region for a
> > > cacheline
> > > 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 | 114 +++++++++++++++++++++++++++++++++++----
> > > tools/perf/util/c2c.h | 1 +
> > > 2 files changed, 104 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> > > index 6b64c0d6b24f..a42423ca990d 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 memory */
> > > + if (mem_region >= PERF_MEM_REGION_MMIO)
> > > + c2c_he->mem_region = mem_region;
> > > +}
> > > +
> > > 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,45 @@ 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[20];
> > > +
> > > + 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_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 +1417,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 = 6,
> > > +};
> > > +
> > > static struct c2c_dimension dim_dcacheline_count = {
> > > .header = HEADER_LOW("PA cnt"),
> > > .name = "dcacheline_count",
> > > @@ -1791,6 +1856,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 +2920,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 +3176,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 +3342,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 +3363,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 +3385,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 +3405,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 +3414,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;
> > >
next prev parent reply other threads:[~2026-09-24 17:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260923161043.95C171F000FF@smtp.kernel.org>
2026-09-23 17:08 ` [PATCH v12 0/6] perf: Add support for memory region/range reporting Thomas Falcon
2026-09-23 17:08 ` [PATCH v12 1/6] perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf() Thomas Falcon
2026-09-23 17:08 ` [PATCH v12 2/6] perf mem: Add support for printing PERF_MEM_LVLNUM_L0 Thomas Falcon
2026-09-23 17:09 ` [PATCH v12 3/6] perf header: Support memory ranges Thomas Falcon
2026-09-23 17:09 ` [PATCH v12 4/6] perf tools: Show memory region in perf-c2c subcommand Thomas Falcon
2026-09-24 0:23 ` Mi, Dapeng
2026-09-24 17:28 ` Arnaldo Carvalho de Melo
2026-09-24 17:29 ` Arnaldo Carvalho de Melo [this message]
2026-09-23 17:09 ` [PATCH v12 5/6] perf tools: Show memory region in perf-script subcommand Thomas Falcon
2026-09-23 17:09 ` [PATCH v12 6/6] perf c2c: print memory region data with stdio output Thomas Falcon
2026-09-24 17:35 ` [PATCH v12 0/6] perf: Add support for memory region/range reporting Arnaldo Carvalho de Melo
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=arVd5uRfZWRzpaaE@x2 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=dapeng1.mi@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=sashiko-bot@kernel.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®