mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf-next v2] selftests/bpf: Wait for wq callback before checking ok_sleepable
@ 2026-11-02  2:36 Jonathan Bastari Thamrin via B4 Relay
  2026-11-02  6:15 ` Alexei Starovoitov
  0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Bastari Thamrin via B4 Relay @ 2026-11-02  2:36 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan
  Cc: bpf, linux-kselftest, linux-kernel, Mykyta Yatsenko, qikeyu2017,
	Jonathan Bastari Thamrin

From: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

wq test fails with ok_sleepable reading 0 instead of 2 as workqueue
callback hasn't executed before check.

This happens as callback runs later on a kworker and 50us isn't always
enough.

Change from 50us sleep to waiting for callback by checking wq_cb_run.
wq_cb_sleepable() now increments wq_cb_run after setting ok_sleepable
and test loops with sched_yield() until wq_cb_run is set before
checking.

Before (usleep(50)): 3 fails out of 5000 runs
Diagnosing issue (usleep(0)): 23 out of 100 runs
After: 0 out of 2000 runs
Tested using vmtest on x86_64, test pinned to one CPU with `yes` load

Link: https://github.com/kernel-patches/vmtest/issues/455
Signed-off-by: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
---
Changes in v2:
- Wait for callback via a wq_cb_run flag instead of polling with a
  timeout, similar to test_struct_ops_assoc.c (Kumar)
- Link to v1: https://lore.kernel.org/r/20261005-wq-poll-fix-v1-1-99ce4bac59f4@gmail.com
---
 tools/testing/selftests/bpf/prog_tests/wq.c |  5 +++--
 tools/testing/selftests/bpf/progs/wq.c      | 10 ++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/wq.c b/tools/testing/selftests/bpf/prog_tests/wq.c
index 97926e35aaf2..95ed4b69c0f5 100644
--- a/tools/testing/selftests/bpf/prog_tests/wq.c
+++ b/tools/testing/selftests/bpf/prog_tests/wq.c
@@ -28,8 +28,9 @@ void serial_test_wq(void)
 	ASSERT_OK(err, "test_run");
 	ASSERT_EQ(topts.retval, 0, "test_run");
 
-	usleep(50); /* 10 usecs should be enough, but give it extra */
-
+	/* wait for wq callback to finish */
+	while (!READ_ONCE(wq_skel->bss->wq_cb_run))
+		sched_yield();
 	ASSERT_EQ(wq_skel->bss->ok_sleepable, (1 << 1), "ok_sleepable");
 clean_up:
 	wq__destroy(wq_skel);
diff --git a/tools/testing/selftests/bpf/progs/wq.c b/tools/testing/selftests/bpf/progs/wq.c
index 25be2cd9d42c..8148db35effe 100644
--- a/tools/testing/selftests/bpf/progs/wq.c
+++ b/tools/testing/selftests/bpf/progs/wq.c
@@ -52,6 +52,7 @@ struct {
 
 __u32 ok;
 __u32 ok_sleepable;
+__u32 wq_cb_run;
 
 static int test_elem_callback(void *map, int *key,
 		int (callback_fn)(void *map, int *key, void *value))
@@ -130,11 +131,12 @@ static int wq_cb_sleepable(void *map, int *key, void *value)
 	struct elem *data = (struct elem *)value;
 	int offset = data->ok_offset;
 
-	if (*key != offset)
-		return 0;
+	if (*key == offset) {
+		bpf_kfunc_call_test_sleepable();
+		ok_sleepable |= (1 << offset);
+	}
 
-	bpf_kfunc_call_test_sleepable();
-	ok_sleepable |= (1 << offset);
+	wq_cb_run++;
 	return 0;
 }
 

---
base-commit: a30c772627b2ef4bc715332b1b04938a914556cd
change-id: 20261005-wq-poll-fix-f46c47dbaf03

Best regards,
-- 
Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH bpf-next v2] selftests/bpf: Wait for wq callback before checking ok_sleepable
  2026-11-02  2:36 [PATCH bpf-next v2] selftests/bpf: Wait for wq callback before checking ok_sleepable Jonathan Bastari Thamrin via B4 Relay
@ 2026-11-02  6:15 ` Alexei Starovoitov
  0 siblings, 0 replies; 2+ messages in thread
From: Alexei Starovoitov @ 2026-11-02  6:15 UTC (permalink / raw)
  To: thamrinjonathan, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan
  Cc: bpf, linux-kselftest, linux-kernel, Mykyta Yatsenko, qikeyu2017

On Sun, Nov 01, 2026 at 06:36 PM Jonathan Bastari Thamrin via B4 Relay <devnull+thamrinjonathan.gmail.com@kernel.org> wrote:
>  	ASSERT_OK(err, "test_run");
>  	ASSERT_EQ(topts.retval, 0, "test_run");
>
> -	usleep(50); /* 10 usecs should be enough, but give it extra */
> -
> +	/* wait for wq callback to finish */
> +	while (!READ_ONCE(wq_skel->bss->wq_cb_run))
> +		sched_yield();

This loop can spin forever.
The two ASSERTs above it don't stop the test. When retval != 0 nothing
starts the wq and wq_cb_run stays 0.

  what goes wrong                  today   with the patch
  -------------------------------  ------  --------------
  test_run fails or retval != 0    FAIL    hang
  the callback doesn't run         FAIL    hang
  the callback gets a wrong key    FAIL    FAIL

The second row is what this test is for.
The watchdog doesn't help. watchdog_timer_func() sends one SIGSEGV.
crash_handler() prints the stack and returns. The loop goes on.

Waiting for the callback is right. Waiting forever is not.
Keep the loop bounded, like prog_tests/ksock_wq.c.
Then there is no need for wq_cb_run and for the changes in progs/wq.c.

Something like this (not even compile tested):

diff --git a/tools/testing/selftests/bpf/prog_tests/wq.c b/tools/testing/selftests/bpf/prog_tests/wq.c
index 97926e3..c4de70b 100644
--- a/tools/testing/selftests/bpf/prog_tests/wq.c
+++ b/tools/testing/selftests/bpf/prog_tests/wq.c
@@ -7,7 +7,7 @@
 void serial_test_wq(void)
 {
 	struct wq *wq_skel = NULL;
-	int err, prog_fd;
+	int err, prog_fd, i;

 	LIBBPF_OPTS(bpf_test_run_opts, topts);

@@ -25,11 +25,13 @@ void serial_test_wq(void)

 	prog_fd = bpf_program__fd(wq_skel->progs.test_syscall_array_sleepable);
 	err = bpf_prog_test_run_opts(prog_fd, &topts);
-	ASSERT_OK(err, "test_run");
-	ASSERT_EQ(topts.retval, 0, "test_run");
-
-	usleep(50); /* 10 usecs should be enough, but give it extra */
+	if (!ASSERT_OK(err, "test_run") ||
+	    !ASSERT_EQ(topts.retval, 0, "test_run"))
+		goto clean_up;

+	/* The callback runs in a kworker. Fail instead of a hang if it doesn't. */
+	for (i = 0; i < 10000 && !READ_ONCE(wq_skel->bss->ok_sleepable); i++)
+		usleep(1000);
 	ASSERT_EQ(wq_skel->bss->ok_sleepable, (1 << 1), "ok_sleepable");
 clean_up:
 	wq__destroy(wq_skel);

pw-bot: cr

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-02  6:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02  2:36 [PATCH bpf-next v2] selftests/bpf: Wait for wq callback before checking ok_sleepable Jonathan Bastari Thamrin via B4 Relay
2026-11-02  6:15 ` Alexei Starovoitov

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®