[PATCH 1/2] perf tests: Skip zero sample IP in object-code reading test

[PATCH 1/2] perf tests: Skip zero sample IP in object-code reading test

[Dapeng Mi]

The kernel may clear the sample IP to 0 when a user-space address is
expected but a kernel address is sampled due to hardware skid. A zero
sample IP causes the object-code reading test to fail incorrectly.

Skip zero sample IP values in the object-code reading test to avoid
false-positive failures.

Reported-by: kernel test robot <yi1.lai@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202609151503.cb02f753-lkp@intel.com
Fixes: 166f10836a65 ("perf/core: Fix kernel register info leak via hardware skid")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 tools/perf/tests/code-reading.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 130cd6021f93..7b80c4ab6916 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -546,6 +546,11 @@ static int process_sample_event(struct machine *machine, struct evlist *evlist,
 		goto out;
 	}
 
+	if (!sample.ip) {
+		pr_debug("kernel may clear IP to 0 in invalid cases, skip it\n");
+		goto out;
+	}
+
 	thread = machine__findnew_thread(machine, sample.pid, sample.tid);
 	if (!thread) {
 		pr_debug("machine__findnew_thread failed\n");
-- 
2.34.1

[PATCH 2/2] perf c2c: Show all memory regions in the region column

[Dapeng Mi]

The c2c memory-region column currently records only the last memory
region seen for an address. However, a single address can map to
multiple memory regions over the sampling window, so reporting only the
final one loses useful information.

Update the memory-region tracking to accumulate all sampled regions for
the address and display them together in the region column.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 tools/perf/builtin-c2c.c | 80 +++++++++++++++++++++++++---------------
 tools/perf/util/c2c.h    |  3 +-
 2 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index f51fa23f3bf1..9c638fbfc2ea 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -260,7 +260,7 @@ static void c2c_he__set_mem_region(struct c2c_hist_entry *c2c_he,
 
 	/* Update mem_region only if it really accesses memory */
 	if (mem_region >= PERF_MEM_REGION_MMIO)
-		c2c_he->mem_region = mem_region;
+		c2c_he->regionset |= BIT(mem_region);
 }
 
 static void compute_stats(struct c2c_hist_entry *c2c_he,
@@ -565,37 +565,12 @@ dcacheline_node_mem_region(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 {
 	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;
-	}
+	if (WARN_ON_ONCE(!c2c_he->regionstr))
+		return 0;
 
-	return scnprintf(hpp->buf, hpp->size, "%*s", width, buf);
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, c2c_he->regionstr);
 }
 
 static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
@@ -2264,6 +2239,52 @@ static int set_nodestr(struct c2c_hist_entry *c2c_he)
 	return c2c_he->nodestr ? 0 : -ENOMEM;
 }
 
+#define PERF_MEM_REGION_MASK	\
+	GENMASK(PERF_MEM_REGION_MEM7, PERF_MEM_REGION_MEM0)
+#define PERF_MEM_REGION_NUM	\
+	(PERF_MEM_REGION_MEM7 - PERF_MEM_REGION_MEM0 + 1)
+
+static int set_mem_region_str(struct c2c_hist_entry *c2c_he)
+{
+	char buf[32];
+	int len = 0;
+
+	if (c2c_he->regionstr)
+		return 0;
+
+	if (!c2c_he->regionset) {
+		len = scnprintf(buf, sizeof(buf), "N/A");
+		goto out;
+	}
+
+	if (c2c_he->regionset & BIT(PERF_MEM_REGION_MMIO)) {
+		len = scnprintf(buf, sizeof(buf), "MMIO");
+		if (c2c_he->regionset & PERF_MEM_REGION_MASK)
+			len += scnprintf(buf + len, sizeof(buf) - len, ",");
+	}
+
+	if (c2c_he->regionset & PERF_MEM_REGION_MASK) {
+		unsigned long regionset =
+			c2c_he->regionset >> PERF_MEM_REGION_MEM0;
+		int num = hweight_long(regionset);
+
+		len += scnprintf(buf + len, sizeof(buf) - len, "Mem-");
+		if (num > 1)
+			len += scnprintf(buf + len, sizeof(buf) - len, "{");
+		len += bitmap_scnprintf(&regionset, PERF_MEM_REGION_NUM,
+					buf + len, sizeof(buf) - len);
+		if (num > 1)
+			len += scnprintf(buf + len, sizeof(buf) - len, "}");
+	}
+
+out:
+	if (len > dim_dcacheline_mem_region.width)
+		dim_dcacheline_mem_region.width = len;
+	c2c_he->regionstr = strdup(buf);
+
+	return c2c_he->regionstr ? 0 : -ENOMEM;
+}
+
 static void calc_width(struct c2c_hist_entry *c2c_he)
 {
 	struct c2c_hists *c2c_hists;
@@ -2271,6 +2292,7 @@ static void calc_width(struct c2c_hist_entry *c2c_he)
 	c2c_hists = container_of(c2c_he->he.hists, struct c2c_hists, hists);
 	hists__calc_col_len(&c2c_hists->hists, &c2c_he->he);
 	set_nodestr(c2c_he);
+	set_mem_region_str(c2c_he);
 }
 
 static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
diff --git a/tools/perf/util/c2c.h b/tools/perf/util/c2c.h
index 65117108ddb1..e09edfa04710 100644
--- a/tools/perf/util/c2c.h
+++ b/tools/perf/util/c2c.h
@@ -33,7 +33,7 @@ struct c2c_hist_entry {
 	unsigned long		*nodeset;
 	struct c2c_stats	*node_stats;
 	unsigned int		 cacheline_idx;
-	unsigned int		 mem_region;
+	unsigned long		 regionset;
 
 	struct compute_stats	 cstats;
 
@@ -41,6 +41,7 @@ struct c2c_hist_entry {
 	unsigned long		 paddr_cnt;
 	bool			 paddr_zero;
 	char			*nodestr;
+	char			*regionstr;
 
 	/*
 	 * must be at the end,
-- 
2.34.1

Re: [PATCH 1/2] perf tests: Skip zero sample IP in object-code reading test

[sashiko-bot]

> The kernel may clear the sample IP to 0 when a user-space address is
> expected but a kernel address is sampled due to hardware skid. A zero
> sample IP causes the object-code reading test to fail incorrectly.
> 
> Skip zero sample IP values in the object-code reading test to avoid
> false-positive failures.
> 
> Reported-by: kernel test robot <yi1.lai@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202609151503.cb02f753-lkp@intel.com
> Fixes: 166f10836a65 ("perf/core: Fix kernel register info leak via hardware skid")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Sashiko has reviewed this patch and found no issues.

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

Re: [PATCH 2/2] perf c2c: Show all memory regions in the region column

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] perf c2c: memory leak of c2c_he->regionstr in c2c_he_free()
- [Low] perf c2c: calc_width() populates regionstr and width on child
  entries
--

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

perf c2c: Show all memory regions in the region column

This commit updates perf c2c to record a bitmap of all sampled memory
regions per histogram entry and format them into a heap-allocated string
displayed in the memory region column.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index f51fa23f3bf18..9c638fbfc2eaa 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c

[ ... ]

> @@ -2264,6 +2239,52 @@ static int set_nodestr(struct c2c_hist_entry *c2c_he)
>  	return c2c_he->nodestr ? 0 : -ENOMEM;
>  }
>  
> +#define PERF_MEM_REGION_MASK	\
> +	GENMASK(PERF_MEM_REGION_MEM7, PERF_MEM_REGION_MEM0)
> +#define PERF_MEM_REGION_NUM	\
> +	(PERF_MEM_REGION_MEM7 - PERF_MEM_REGION_MEM0 + 1)
> +
> +static int set_mem_region_str(struct c2c_hist_entry *c2c_he)
> +{
> +	char buf[32];
> +	int len = 0;
> +
> +	if (c2c_he->regionstr)
> +		return 0;
> +
> +	if (!c2c_he->regionset) {
> +		len = scnprintf(buf, sizeof(buf), "N/A");
> +		goto out;
> +	}
> +
> +	if (c2c_he->regionset & BIT(PERF_MEM_REGION_MMIO)) {
> +		len = scnprintf(buf, sizeof(buf), "MMIO");
> +		if (c2c_he->regionset & PERF_MEM_REGION_MASK)
> +			len += scnprintf(buf + len, sizeof(buf) - len, ",");
> +	}
> +
> +	if (c2c_he->regionset & PERF_MEM_REGION_MASK) {
> +		unsigned long regionset =
> +			c2c_he->regionset >> PERF_MEM_REGION_MEM0;
> +		int num = hweight_long(regionset);
> +
> +		len += scnprintf(buf + len, sizeof(buf) - len, "Mem-");
> +		if (num > 1)
> +			len += scnprintf(buf + len, sizeof(buf) - len, "{");
> +		len += bitmap_scnprintf(&regionset, PERF_MEM_REGION_NUM,
> +					buf + len, sizeof(buf) - len);
> +		if (num > 1)
> +			len += scnprintf(buf + len, sizeof(buf) - len, "}");
> +	}
> +
> +out:
> +	if (len > dim_dcacheline_mem_region.width)
> +		dim_dcacheline_mem_region.width = len;\
> +	c2c_he->regionstr = strdup(buf);

[Severity: Low]
Does c2c_he_free() leak c2c_he->regionstr?  c2c_he_free() frees
c2c_he->nodestr and the other heap fields with zfree(), but does not free
c2c_he->regionstr.

> +
> +	return c2c_he->regionstr ? 0 : -ENOMEM;
> +}
> +
>  static void calc_width(struct c2c_hist_entry *c2c_he)
>  {
>  	struct c2c_hists *c2c_hists;
> @@ -2271,6 +2292,7 @@ static void calc_width(struct c2c_hist_entry *c2c_he)
>  	c2c_hists = container_of(c2c_he->he.hists, struct c2c_hists, hists);
>  	hists__calc_col_len(&c2c_hists->hists, &c2c_he->he);
>  	set_nodestr(c2c_he);
> +	set_mem_region_str(c2c_he);
>  }

[Severity: Low]
Should set_mem_region_str() only run for top-level cacheline entries (or
when c2c_he->hists is non-NULL)?

calc_width() is also called from filter_cb() for per-cacheline child
entries, which do not display the dcacheline_mem_region column.  Running
set_mem_region_str() on child entries allocates an unused regionstr for
every child entry (including filtered ones) and can inflate
dim_dcacheline_mem_region.width when a child's non-contiguous subset of
regions (such as "Mem-{0,2,4}") formats wider than the parent cacheline's
contiguous range (such as "Mem-{0-4}").

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

Re: [PATCH 2/2] perf c2c: Show all memory regions in the region column

[Mi, Dapeng]

On 11/6/2026 3:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Low] perf c2c: memory leak of c2c_he->regionstr in c2c_he_free()
> - [Low] perf c2c: calc_width() populates regionstr and width on child
>   entries
> --
>
> commit bee454704ac015d02d879348350c49da07d10fb5
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf c2c: Show all memory regions in the region column
>
> This commit updates perf c2c to record a bitmap of all sampled memory
> regions per histogram entry and format them into a heap-allocated string
> displayed in the memory region column.
>
>> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
>> index f51fa23f3bf18..9c638fbfc2eaa 100644
>> --- a/tools/perf/builtin-c2c.c
>> +++ b/tools/perf/builtin-c2c.c
> [ ... ]
>
>> @@ -2264,6 +2239,52 @@ static int set_nodestr(struct c2c_hist_entry *c2c_he)
>>  	return c2c_he->nodestr ? 0 : -ENOMEM;
>>  }
>>  
>> +#define PERF_MEM_REGION_MASK	\
>> +	GENMASK(PERF_MEM_REGION_MEM7, PERF_MEM_REGION_MEM0)
>> +#define PERF_MEM_REGION_NUM	\
>> +	(PERF_MEM_REGION_MEM7 - PERF_MEM_REGION_MEM0 + 1)
>> +
>> +static int set_mem_region_str(struct c2c_hist_entry *c2c_he)
>> +{
>> +	char buf[32];
>> +	int len = 0;
>> +
>> +	if (c2c_he->regionstr)
>> +		return 0;
>> +
>> +	if (!c2c_he->regionset) {
>> +		len = scnprintf(buf, sizeof(buf), "N/A");
>> +		goto out;
>> +	}
>> +
>> +	if (c2c_he->regionset & BIT(PERF_MEM_REGION_MMIO)) {
>> +		len = scnprintf(buf, sizeof(buf), "MMIO");
>> +		if (c2c_he->regionset & PERF_MEM_REGION_MASK)
>> +			len += scnprintf(buf + len, sizeof(buf) - len, ",");
>> +	}
>> +
>> +	if (c2c_he->regionset & PERF_MEM_REGION_MASK) {
>> +		unsigned long regionset =
>> +			c2c_he->regionset >> PERF_MEM_REGION_MEM0;
>> +		int num = hweight_long(regionset);
>> +
>> +		len += scnprintf(buf + len, sizeof(buf) - len, "Mem-");
>> +		if (num > 1)
>> +			len += scnprintf(buf + len, sizeof(buf) - len, "{");
>> +		len += bitmap_scnprintf(&regionset, PERF_MEM_REGION_NUM,
>> +					buf + len, sizeof(buf) - len);
>> +		if (num > 1)
>> +			len += scnprintf(buf + len, sizeof(buf) - len, "}");
>> +	}
>> +
>> +out:
>> +	if (len > dim_dcacheline_mem_region.width)
>> +		dim_dcacheline_mem_region.width = len;\
>> +	c2c_he->regionstr = strdup(buf);
> [Severity: Low]
> Does c2c_he_free() leak c2c_he->regionstr?  c2c_he_free() frees
> c2c_he->nodestr and the other heap fields with zfree(), but does not free
> c2c_he->regionstr.

Yes, it's missed, would be added in the next version.


>
>> +
>> +	return c2c_he->regionstr ? 0 : -ENOMEM;
>> +}
>> +
>>  static void calc_width(struct c2c_hist_entry *c2c_he)
>>  {
>>  	struct c2c_hists *c2c_hists;
>> @@ -2271,6 +2292,7 @@ static void calc_width(struct c2c_hist_entry *c2c_he)
>>  	c2c_hists = container_of(c2c_he->he.hists, struct c2c_hists, hists);
>>  	hists__calc_col_len(&c2c_hists->hists, &c2c_he->he);
>>  	set_nodestr(c2c_he);
>> +	set_mem_region_str(c2c_he);
>>  }
> [Severity: Low]
> Should set_mem_region_str() only run for top-level cacheline entries (or
> when c2c_he->hists is non-NULL)?
>
> calc_width() is also called from filter_cb() for per-cacheline child
> entries, which do not display the dcacheline_mem_region column.  Running
> set_mem_region_str() on child entries allocates an unused regionstr for
> every child entry (including filtered ones) and can inflate
> dim_dcacheline_mem_region.width when a child's non-contiguous subset of
> regions (such as "Mem-{0,2,4}") formats wider than the parent cacheline's
> contiguous range (such as "Mem-{0-4}").

although currently the region column is only supported on the top-level
cacheline entries, but it would be extended the per-cacheline child view.
So let's keep what it is now.


>