Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: <thamrinjonathan@gmail.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Eduard Zingerman" <eddyz87@gmail.com>,
	"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Song Liu" <song@kernel.org>,
	"Yonghong Song" <yonghong.song@linux.dev>,
	"Jiri Olsa" <jolsa@kernel.org>,
	"Emil Tsalapatis" <emil@etsalapatis.com>,
	"Ihor Solodrai" <ihor.solodrai@linux.dev>,
	"Shuah Khan" <shuah@kernel.org>
Cc: <bpf@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	"Mykyta Yatsenko" <yatsenko@meta.com>, <qikeyu2017@gmail.com>
Subject: Re: [PATCH bpf-next v2] selftests/bpf: Wait for wq callback before checking ok_sleepable
Date: Mon, 02 Nov 2026 06:15:11 +0000	[thread overview]
Message-ID: <DMKI6C7FPCCV.2Y4YLRGWNSKW1@gmail.com> (raw)
In-Reply-To: <20261102-wq-poll-fix-v2-1-48b7d12815b8@gmail.com>

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

      reply	other threads:[~2026-11-02  6:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMKI6C7FPCCV.2Y4YLRGWNSKW1@gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=qikeyu2017@gmail.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=thamrinjonathan@gmail.com \
    --cc=yatsenko@meta.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox