[PATCH v1 0/2] perf tests: Fix and improve watchpoint tests

[PATCH v1 0/2] perf tests: Fix and improve watchpoint tests

[Tiezhu Yang]

This patch series aims to fix a potential logic bug and improve
the debuggability of the watchpoint tests in the perf tool.

Tiezhu Yang (2):
  perf tests: Assign return value of ioctl in wp_modify test
  perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests

 tools/perf/tests/wp.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

-- 
2.42.0

[PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test

[Tiezhu Yang]

In test__wp_modify(), the return value of ioctl PERF_EVENT_IOC_ENABLE
was not being captured. As a result, the subsequent error handling was
checking a stalled value of `ret` instead of the actual result of the
enablement operation.

Fix this by properly assigning the return value of the ioctl call to
`ret` so that the test can correctly catch and report failures when
enabling the watchpoint event.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/perf/tests/wp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
index 69b31f00eed0..f0b829b82f2b 100644
--- a/tools/perf/tests/wp.c
+++ b/tools/perf/tests/wp.c
@@ -182,7 +182,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
 	WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
 
 	/* Enable the event */
-	ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
+	ret = ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
 	if (ret < 0) {
 		pr_debug("Failed to enable event\n");
 		close(fd);
-- 
2.42.0

[PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests

[Tiezhu Yang]

Currently, WP_TEST_ASSERT_VAL uses TEST_ASSERT_VAL which only prints
the failure message but drops the actual and expected counters. This
makes debugging hard when a watchpoint test fails due to arch-specific
hardware behavior or counter mismatches.

Rename WP_TEST_ASSERT_VAL to WP_TEST_ASSERT_EQUAL and use the built-in
TEST_ASSERT_EQUAL macro. This ensures that both the expected and actual
counter values are automatically printed upon failure.

Here is the log when executing "./perf test -v 'Read Only Watchpoint'".

Before:
  FAILED tests/wp.c:90 RO watchpoint

After:
  FAILED tests/wp.c:90 RO watchpoint (2 != 1)

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/perf/tests/wp.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
index f0b829b82f2b..253e551f10d2 100644
--- a/tools/perf/tests/wp.c
+++ b/tools/perf/tests/wp.c
@@ -13,11 +13,11 @@
 #include "cloexec.h"
 #include "../perf-sys.h"
 
-#define WP_TEST_ASSERT_VAL(fd, text, val)       \
-do {                                            \
-	long long count;                        \
-	wp_read(fd, &count, sizeof(long long)); \
-	TEST_ASSERT_VAL(text, count == val);    \
+#define WP_TEST_ASSERT_EQUAL(fd, text, val)		\
+do {							\
+	long long count;				\
+	wp_read(fd, &count, sizeof(long long));		\
+	TEST_ASSERT_EQUAL(text, (int)count, (int)val);	\
 } while (0)
 
 #ifdef __i386__
@@ -87,10 +87,10 @@ static int test__wp_ro(struct test_suite *test __maybe_unused,
 		return fd == -ENODEV ? TEST_SKIP : -1;
 
 	tmp = data1;
-	WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "RO watchpoint", 1);
 
 	data1 = tmp1 + tmp;
-	WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "RO watchpoint", 1);
 
 	close(fd);
 	return 0;
@@ -111,10 +111,10 @@ static int test__wp_wo(struct test_suite *test __maybe_unused,
 		return fd == -ENODEV ? TEST_SKIP : -1;
 
 	tmp = data1;
-	WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0);
+	WP_TEST_ASSERT_EQUAL(fd, "WO watchpoint", 0);
 
 	data1 = tmp1 + tmp;
-	WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "WO watchpoint", 1);
 
 	close(fd);
 	return 0;
@@ -136,10 +136,10 @@ static int test__wp_rw(struct test_suite *test __maybe_unused,
 		return fd == -ENODEV ? TEST_SKIP : -1;
 
 	tmp = data1;
-	WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "RW watchpoint", 1);
 
 	data1 = tmp1 + tmp;
-	WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 2);
+	WP_TEST_ASSERT_EQUAL(fd, "RW watchpoint", 2);
 
 	close(fd);
 	return 0;
@@ -160,7 +160,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
 		return fd == -ENODEV ? TEST_SKIP : -1;
 
 	data1 = tmp;
-	WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "Modify watchpoint", 1);
 
 	/* Modify watchpoint with disabled = 1 */
 	get__perf_event_attr(&new_attr, HW_BREAKPOINT_W, (void *)&data2[0],
@@ -179,7 +179,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
 	}
 
 	data2[1] = tmp; /* Not Counted */
-	WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
+	WP_TEST_ASSERT_EQUAL(fd, "Modify watchpoint", 1);
 
 	/* Enable the event */
 	ret = ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
@@ -190,10 +190,10 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
 	}
 
 	data2[1] = tmp; /* Counted */
-	WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 2);
+	WP_TEST_ASSERT_EQUAL(fd, "Modify watchpoint", 2);
 
 	data2[2] = tmp; /* Not Counted */
-	WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 2);
+	WP_TEST_ASSERT_EQUAL(fd, "Modify watchpoint", 2);
 
 	close(fd);
 	return 0;
-- 
2.42.0

Re: [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests

[sashiko-bot]

> Currently, WP_TEST_ASSERT_VAL uses TEST_ASSERT_VAL which only prints
> the failure message but drops the actual and expected counters. This
> makes debugging hard when a watchpoint test fails due to arch-specific
> hardware behavior or counter mismatches.
> 
> Rename WP_TEST_ASSERT_VAL to WP_TEST_ASSERT_EQUAL and use the built-in
> TEST_ASSERT_EQUAL macro. This ensures that both the expected and actual
> counter values are automatically printed upon failure.
> 
> Here is the log when executing "./perf test -v 'Read Only Watchpoint'".
> 
> Before:
>   FAILED tests/wp.c:90 RO watchpoint
> 
> After:
>   FAILED tests/wp.c:90 RO watchpoint (2 != 1)
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106092808.6914-1-yangtiezhu@loongson.cn?part=2

Re: [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf tests: missing Fixes trailer for test__wp_modify() ioctl
  check
--

commit 36cc478a3007ff125acd9f095ef1c72b59deb4aa
Author: Tiezhu Yang <yangtiezhu@loongson.cn>

> perf tests: Assign return value of ioctl in wp_modify test
>
> In test__wp_modify(), the return value of ioctl PERF_EVENT_IOC_ENABLE
> was not being captured. As a result, the subsequent error handling was
> checking a stalled value of `ret` instead of the actual result of the
> enablement operation.
>
> Fix this by properly assigning the return value of the ioctl call to
> `ret` so that the test can correctly catch and report failures when
> enabling the watchpoint event.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

[Severity: Low]
This fixes a missing return value assignment introduced when the
watchpoint test was added.  Should the commit message include a Fixes
tag?

  Fixes: 2e85d5979e8d ("perf test: Add watchpoint test")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106092808.6914-1-yangtiezhu@loongson.cn?part=1

Re: [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test

[Tiezhu Yang]

On 11/6/26 5:36 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:

...

> [Severity: Low]
> This fixes a missing return value assignment introduced when the
> watchpoint test was added.  Should the commit message include a Fixes
> tag?
> 
>    Fixes: 2e85d5979e8d ("perf test: Add watchpoint test")

This is just a simple test case fix, so I'll let the maintainers
decide if a Fixes tag is necessary when applying the patch.

Thanks,
Tiezhu