* [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests
@ 2026-11-06 9:28 Tiezhu Yang
2026-11-06 9:28 ` [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test Tiezhu Yang
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Tiezhu Yang @ 2026-11-06 9:28 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: linux-perf-users, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test 2026-11-06 9:28 [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests Tiezhu Yang @ 2026-11-06 9:28 ` Tiezhu Yang 2026-11-06 9:36 ` sashiko-bot 2026-11-06 9:28 ` [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests Tiezhu Yang 2026-11-10 2:52 ` [PATCH v1 0/2] perf tests: Fix and improve " Namhyung Kim 2 siblings, 1 reply; 8+ messages in thread From: Tiezhu Yang @ 2026-11-06 9:28 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim Cc: linux-perf-users, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test 2026-11-06 9:28 ` [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test Tiezhu Yang @ 2026-11-06 9:36 ` sashiko-bot 2026-11-06 9:51 ` Tiezhu Yang 0 siblings, 1 reply; 8+ messages in thread From: sashiko-bot @ 2026-11-06 9:36 UTC (permalink / raw) To: Tiezhu Yang; +Cc: linux-perf-users 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test 2026-11-06 9:36 ` sashiko-bot @ 2026-11-06 9:51 ` Tiezhu Yang 2026-11-09 4:54 ` Namhyung Kim 0 siblings, 1 reply; 8+ messages in thread From: Tiezhu Yang @ 2026-11-06 9:51 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-perf-users 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test 2026-11-06 9:51 ` Tiezhu Yang @ 2026-11-09 4:54 ` Namhyung Kim 0 siblings, 0 replies; 8+ messages in thread From: Namhyung Kim @ 2026-11-09 4:54 UTC (permalink / raw) To: Tiezhu Yang; +Cc: sashiko-reviews, linux-perf-users On Fri, Nov 06, 2026 at 05:51:35PM +0800, Tiezhu Yang wrote: > 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. Yep, I'll add this. Thanks, Namhyung ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests 2026-11-06 9:28 [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests Tiezhu Yang 2026-11-06 9:28 ` [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test Tiezhu Yang @ 2026-11-06 9:28 ` Tiezhu Yang 2026-11-06 9:34 ` sashiko-bot 2026-11-10 2:52 ` [PATCH v1 0/2] perf tests: Fix and improve " Namhyung Kim 2 siblings, 1 reply; 8+ messages in thread From: Tiezhu Yang @ 2026-11-06 9:28 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim Cc: linux-perf-users, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests 2026-11-06 9:28 ` [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests Tiezhu Yang @ 2026-11-06 9:34 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-11-06 9:34 UTC (permalink / raw) To: Tiezhu Yang; +Cc: linux-perf-users > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests 2026-11-06 9:28 [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests Tiezhu Yang 2026-11-06 9:28 ` [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test Tiezhu Yang 2026-11-06 9:28 ` [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests Tiezhu Yang @ 2026-11-10 2:52 ` Namhyung Kim 2 siblings, 0 replies; 8+ messages in thread From: Namhyung Kim @ 2026-11-10 2:52 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Tiezhu Yang Cc: linux-perf-users, linux-kernel On Fri, 06 Nov 2026 17:28:06 +0800, Tiezhu Yang wrote: > 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 > > [...] Applied to perf-tools-next, thanks! Best regards, Namhyung ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-10 2:52 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-06 9:28 [PATCH v1 0/2] perf tests: Fix and improve watchpoint tests Tiezhu Yang 2026-11-06 9:28 ` [PATCH v1 1/2] perf tests: Assign return value of ioctl in wp_modify test Tiezhu Yang 2026-11-06 9:36 ` sashiko-bot 2026-11-06 9:51 ` Tiezhu Yang 2026-11-09 4:54 ` Namhyung Kim 2026-11-06 9:28 ` [PATCH v1 2/2] perf tests: Use TEST_ASSERT_EQUAL for all watchpoint tests Tiezhu Yang 2026-11-06 9:34 ` sashiko-bot 2026-11-10 2:52 ` [PATCH v1 0/2] perf tests: Fix and improve " Namhyung Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox