[PATCH bpf 0/2] selftests/bpf: Fix task work false failures and cleanup

[PATCH bpf 0/2] selftests/bpf: Fix task work false failures and cleanup

[Yun Lu]

From: Yun Lu <luyun@kylinos.cn>

Hi,

This series fixes two independent issues in the BPF task work
selftests.

1/2 removes timing-dependent assumptions from the stress
test while retaining its callback accounting checks. 

2/2 routes a task_work__open() failure through the common cleanup
path so that the pipe fds are closed and the forked child is woken
and reaped.

The stress test was run 50 times normally and 50 times with single-CPU
affinity, with all runs passing. A single-scheduler diagnostic also
reproduced the original zero-contention false failure and passed after
the fix.

Thanks,
Yun Lu

Yun Lu (2):
  selftests/bpf: Fix timing-dependent assertions in task work stress test
  selftests/bpf: Clean up child when task_work__open() fails

 .../bpf/prog_tests/task_work_stress.c         | 6 ++----
 .../selftests/bpf/prog_tests/test_task_work.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH bpf 1/2] selftests/bpf: Fix timing-dependent assertions in task work stress test

[Yun Lu]

From: Yun Lu <luyun@kylinos.cn>

task_work_run() assumes every stress run observes both scheduler
contention and, when the delete thread is enabled, cancellation of at
least one pending callback.

Neither outcome is guaranteed. Contention is observed only when another
scheduler selects the same one of 128 map values while its task work is
outstanding. A valid run can therefore have no scheduling errors. This
was reproduced with a single scheduler: more than one million callbacks
were scheduled and completed while schedule_error remained zero.

Likewise, a successful map deletion may remove a value after its
callback has completed. delete_success can therefore be nonzero without
callback_success being smaller than callback_scheduled.

Drop the assertion that schedule_error is nonzero. Keep requiring exact
callback accounting when deletion is disabled. With deletion enabled,
allow equal counts but still reject callback_success greater than
callback_scheduled.

Fixes: c6ae18e0af5e ("selftests/bpf: add bpf task work stress tests")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/task_work_stress.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/task_work_stress.c b/tools/testing/selftests/bpf/prog_tests/task_work_stress.c
index 450d17d91a56..d61636ed5e3d 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_work_stress.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_work_stress.c
@@ -105,13 +105,11 @@ static void task_work_run(bool enable_delete)
 		pthread_join(tid_del, NULL);
 
 	ASSERT_GT(skel->bss->callback_scheduled, 0, "work scheduled");
-	/* Some scheduling attempts should have failed due to contention */
-	ASSERT_GT(skel->bss->schedule_error, 0, "schedule error");
 
 	if (enable_delete) {
-		/* If delete thread is enabled, it has cancelled some callbacks */
 		ASSERT_GT(skel->bss->delete_success, 0, "delete success");
-		ASSERT_LT(skel->bss->callback_success, skel->bss->callback_scheduled, "callbacks");
+		/* Deletion may race after the task work has already run. */
+		ASSERT_LE(skel->bss->callback_success, skel->bss->callback_scheduled, "callbacks");
 	} else {
 		/* Without delete thread number of scheduled callbacks is the same as fired */
 		ASSERT_EQ(skel->bss->callback_success, skel->bss->callback_scheduled, "callbacks");
-- 
2.43.0

Re: [PATCH bpf 1/2] selftests/bpf: Fix timing-dependent assertions in task work stress test

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

> task_work_run() assumes every stress run observes both scheduler
> contention and, when the delete thread is enabled, cancellation of at
> least one pending callback.
>
> Neither outcome is guaranteed. Contention is observed only when another
> scheduler selects the same one of 128 map values while its task work is
> outstanding. A valid run can therefore have no scheduling errors. This
> was reproduced with a single scheduler: more than one million callbacks
> were scheduled and completed while schedule_error remained zero.

This isn't a bug, but would it read more clearly to say something like 'the
task_work_stress test helper' (or name the file) instead of the bare
task_work_run() in the opening sentence?  There are three functions with
that name in the tree: kernel/task_work.c, prog_tests/test_task_work.c, and
the static helper being changed here in prog_tests/task_work_stress.c, so
the bare name could be mistaken for the kernel function on first reading.

This isn't a bug, but was nthreads reduced locally to get the
single-scheduler run quoted here? The test hardcodes 'int nthreads = 16;'
(prog_tests/task_work_stress.c:50) and the only environment knob is
BPF_TASK_WORK_TEST_TIME for the duration. A reader trying to reproduce the
quoted numbers has no way to get there without editing the file. It might
help to say so explicitly, or give the 16-thread numbers, since the test
has no knob for the thread count.

> Likewise, a successful map deletion may remove a value after its
> callback has completed. delete_success can therefore be nonzero without
> callback_success being smaller than callback_scheduled.
>
> Drop the assertion that schedule_error is nonzero. Keep requiring exact
> callback accounting when deletion is disabled. With deletion enabled,
> allow equal counts but still reject callback_success greater than
> callback_scheduled.
>
> Fixes: c6ae18e0af5e ("selftests/bpf: add bpf task work stress tests")
> Signed-off-by: Yun Lu <luyun@kylinos.cn>


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35199472874

Re: [PATCH bpf 1/2] selftests/bpf: Fix timing-dependent assertions in task work stress test

[luyun]

在 2026/9/17 17:00, bot+bpf-ci@kernel.org 写道:
>> task_work_run() assumes every stress run observes both scheduler
>> contention and, when the delete thread is enabled, cancellation of at
>> least one pending callback.
>>
>> Neither outcome is guaranteed. Contention is observed only when another
>> scheduler selects the same one of 128 map values while its task work is
>> outstanding. A valid run can therefore have no scheduling errors. This
>> was reproduced with a single scheduler: more than one million callbacks
>> were scheduled and completed while schedule_error remained zero.
> This isn't a bug, but would it read more clearly to say something like 'the
> task_work_stress test helper' (or name the file) instead of the bare
> task_work_run() in the opening sentence?  There are three functions with
> that name in the tree: kernel/task_work.c, prog_tests/test_task_work.c, and
> the static helper being changed here in prog_tests/task_work_stress.c, so
> the bare name could be mistaken for the kernel function on first reading.
Agreed. The opening sentence will be reworded to "The task_work_run()
helper in prog_tests/task_work_stress.c assumes ...", so it cannot be
mistaken for the kernel function of the same name.
> This isn't a bug, but was nthreads reduced locally to get the
> single-scheduler run quoted here? The test hardcodes 'int nthreads = 16;'
> (prog_tests/task_work_stress.c:50) and the only environment knob is
> BPF_TASK_WORK_TEST_TIME for the duration. A reader trying to reproduce the
> quoted numbers has no way to get there without editing the file. It might
> help to say so explicitly, or give the 16-thread numbers, since the test
> has no knob for the thread count.
Yes, that run came from a modified copy of the test with the scheduler
thread count reduced to 1. v2 will state this explicitly: "reproduced
with a modified copy of the test running a single scheduler thread
(nthreads is hardcoded to 16 and the test has no knob for it)".

These are commit message changes only; the code will be identical to
v1, and 2/2 is unchanged. If there are no further comments on the two
patches, I will send out v2 later.

Thanks,
Yun Lu
>> Likewise, a successful map deletion may remove a value after its
>> callback has completed. delete_success can therefore be nonzero without
>> callback_success being smaller than callback_scheduled.
>>
>> Drop the assertion that schedule_error is nonzero. Keep requiring exact
>> callback accounting when deletion is disabled. With deletion enabled,
>> allow equal counts but still reject callback_success greater than
>> callback_scheduled.
>>
>> Fixes: c6ae18e0af5e ("selftests/bpf: add bpf task work stress tests")
>> Signed-off-by: Yun Lu <luyun@kylinos.cn>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35199472874

Re: [PATCH bpf 1/2] selftests/bpf: Fix timing-dependent assertions in task work stress test

[Alexei Starovoitov]

On Thu, Sep 17, 2026 at 04:08 PM Yun Lu <luyun_611@163.com> wrote:
> From: Yun Lu <luyun@kylinos.cn>
>
> task_work_run() assumes every stress run observes both scheduler
> contention and, when the delete thread is enabled, cancellation of at
> least one pending callback.
>
> Neither outcome is guaranteed. Contention is observed only when another
> scheduler selects the same one of 128 map values while its task work is
> outstanding. A valid run can therefore have no scheduling errors. This
> was reproduced with a single scheduler: more than one million callbacks
> were scheduled and completed while schedule_error remained zero.

So it was "reproduced" by changing nthreads from 16 to 1 ?
That's not a fix. With 16 threads hammering 128 entries for a second
schedule_error > 0 and callback_success < callback_scheduled are the
point of the test. They check that the -EBUSY path in
bpf_task_work_acquire_ctx() and the cancel path in
bpf_task_work_cancel_and_free() were actual
ly exercised. Without them
the stress test only checks that nothing crashed.
If the unmodified test fails somewhere, show the log.

Patch 2 is fine on its own. Send it alone against bpf-next.

pw-bot: cr

Re: [PATCH bpf 1/2] selftests/bpf: Fix timing-dependent assertions in task work stress test

[luyun]

在 2026/9/17 22:39, Alexei Starovoitov 写道:
> On Thu, Sep 17, 2026 at 04:08 PM Yun Lu <luyun_611@163.com> wrote:
>> From: Yun Lu <luyun@kylinos.cn>
>>
>> task_work_run() assumes every stress run observes both scheduler
>> contention and, when the delete thread is enabled, cancellation of at
>> least one pending callback.
>>
>> Neither outcome is guaranteed. Contention is observed only when another
>> scheduler selects the same one of 128 map values while its task work is
>> outstanding. A valid run can therefore have no scheduling errors. This
>> was reproduced with a single scheduler: more than one million callbacks
>> were scheduled and completed while schedule_error remained zero.
> So it was "reproduced" by changing nthreads from 16 to 1 ?
> That's not a fix. With 16 threads hammering 128 entries for a second
> schedule_error > 0 and callback_success < callback_scheduled are the
> point of the test. They check that the -EBUSY path in
> bpf_task_work_acquire_ctx() and the cancel path in
> bpf_task_work_cancel_and_free() were actual
> ly exercised. Without them
> the stress test only checks that nothing crashed.
> If the unmodified test fails somewhere, show the log.

Hi, Alexei


Thanks for the clarification. You are right that the zero-contention
result was obtained only after locally changing nthreads from 16 to 1,
so it does not reproduce a failure of the unmodified stress test.

I reran the unmodified test with nthreads = 16 for 100 iterations in
the default environment, 100 iterations with all threads restricted to
one CPU, and 100 iterations in a single-vCPU QEMU guest. All runs
passed, so I do not have a failure log for the upstream test
configuration.


>
> Patch 2 is fine on its own. Send it alone against bpf-next.


OK, I will drop patch 1 and resend patch 2 alone against bpf-next.


Thanks,

Yun Lu


>
> pw-bot: cr

[PATCH bpf 2/2] selftests/bpf: Clean up child when task_work__open() fails

[Yun Lu]

From: Yun Lu <luyun@kylinos.cn>

task_work_run() forks a child that blocks reading a pipe until the
parent wakes it. If task_work__open() fails afterwards, the parent
returns without closing either pipe fd or waiting for the child.
Because the parent retains the write end, the child remains blocked in
read() until test_progs exits.

Route this failure through the common cleanup path. At this point
cleanup is safe: pe_fd is -1, link is NULL, task_work__destroy() accepts
NULL, and the pid > 0 branch closes the read end, wakes the child,
closes the write end and waits for it.

This is the last early return after a successful fork. The fork failure
path already closes both pipe fds after commit 5730dacb3f17
("selftests/bpf: Task_work selftest cleanup fixes").

Fixes: 39fd74dfd5d2 ("selftests/bpf: BPF task work scheduling tests")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/test_task_work.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
index 774b31a5f6ca..fe0cb1702eaa 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_task_work.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
@@ -85,7 +85,7 @@ static void task_work_run(const char *prog_name, const char *map_name)
 
 	skel = task_work__open();
 	if (!ASSERT_OK_PTR(skel, "task_work__open"))
-		return;
+		goto cleanup;
 
 	bpf_object__for_each_program(prog, skel->obj) {
 		bpf_program__set_autoload(prog, false);
-- 
2.43.0