Re: [PATCH v8 4/6] perf tools: Show memory region in perf-c2c subcommand

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 9/11/2026 11:58 PM, Falcon, Thomas wrote:
> On Fri, 2026-09-11 at 08:50 +0800, Mi, Dapeng wrote:
>> On 9/11/2026 3:43 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     (181 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  0xffffffffa1658ec0     0x0     0    1302    4.13%      103      103        0     1707     1707        0
>>>     1  0xffffffffa16a9100     0x0     0     991    3.73%       93       93        0     1580     1580       19
>>>     2  0xffffffffa16a91c0     0x0     0      37    3.05%       76       76        0      384      384        4
>>>     3  0xffffffffa0607a00     0x0     0       1    2.65%       66       66        0     1146     1146        5
>>>     4  0xffffffffa16a9200     0x0     0       1    1.72%       43       43        0      174      174        3
>>>     5  0xffffffffa0607a80     0x0     0       1    1.16%       29       29        0      439      439       12
>>>     6  0xffffffffa16aadc0     0x0     0       1    0.72%       18       18        0      158      158        3
>>>     7  0xff30ea3c37934640     N/A     0      51    0.64%       16       16        0       60       60        8
>>>     8  0xff30ea3c373b4640     N/A     0      64    0.48%       12       12        0       69       69       11
>>>     9  0xff30ea3c387b4640     0x0     0      56    0.48%       12       12        0       59       59        9
>>>    10  0xff30ea3c37db4640     N/A     0      49    0.44%       11       11        0       50       50       10
>> It looks good. Thanks.
>>
>>
>>> [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
>>> Codeveloped-by: Thomas Falcon <thomas.falcon@intel.com>
>> The tag name should be "Co-developed-by" instead of "Codeveloped-by".
>> Besides, the tag should be moved to the place where is after my SoB and
>> before your SoB. :)
> Ah, sorry about that, I will fix it up soon. See further comments below...
>
> ...
>
>>> +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;
>>> +
>>> +	if (mem_region == PERF_MEM_REGION_NA)
>>> +		scnprintf(buf, sizeof(buf),  "N/A");
>>> +	/* mem_region could only be >= PERF_MEM_REGION_MMIO */
>>> +	else if (mem_region == PERF_MEM_REGION_MMIO)
>>> +		scnprintf(buf, sizeof(buf), "MMIO");
>>> +	else
>>> +		scnprintf(buf, sizeof(buf), "0x%x",
>>> +			  mem_region - PERF_MEM_REGION_MEM0);
>>> +
>>> +	return scnprintf(hpp->buf, hpp->size, "%*s", width, buf);
>>> +}
>> It seems the previous comment is missed? 
>>
>> "Is the mem-region column print guarded by HEADER_MEMORY_RANGES as well?"
>>
>> I'm not quite sure about this, please double check. Thanks.
> Sorry, I missed your two comments below your first regarding developer tags.
>
> If you refer to this change below, dcacheline_mem_region is printed only if c2c.show_mem_region (perf_header__has_feat(&session->header, HEADER_MEMORY_RANGES)) == true, otherwise it is not included in
> output_str. I will update perf-c2c to include cache regions as well to be more consistent with perf-script in a new version and fix the tag mistakes.

Thanks. We need to ensure dcacheline_mem_region is printed only when the
header exists. Other it would cause misleading for users.

If we plan to add cache region in the "region" column, we could need to add
"mem" prefix before the memory region ID which avoids the ambiguity.
Besides, we'd better change the region id to decimal format after adding
the "mem" prefix. 



>
> Tom
>
> -	if (c2c.display != DISPLAY_SNP_PEER)
> -		output_str = "cl_idx,"perf_header__has_feat(&session->header,
> +						HEADER_MEMORY_RANGES);
> +	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,"
> @@ -3285,10 +3346,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,"
> @@ -3300,7 +3368,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;
> +		}
> +	}
>>
>>> +
>>>  static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>>>  			struct hist_entry *he)
>>>  {
>>> @@ -1359,6 +1400,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",
>>> @@ -1790,6 +1839,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,
>>> @@ -2853,8 +2903,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;
>>>  
>>> @@ -3106,7 +3159,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();
>>> @@ -3271,9 +3325,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,"
>>> @@ -3285,10 +3346,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,"
>>> @@ -3300,7 +3368,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";
>>> @@ -3314,7 +3388,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...");
>>> @@ -3323,17 +3397,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;
>>>