Re: [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: lost wakeup in scx_userland due to nr_queued TOCTOU race
--

commit 58872b4f72d7bfd3655086348da64ace07a2573a
Author: Wanwu Li <liwanwu@kylinos.cn>

tools/sched_ext: Increment nr_queued when a task is queued to user space

userland_update_idle() decides whether a CPU going idle must wake the
user-space scheduler with "if (nr_queued || nr_scheduled)", where
nr_queued is documented to be incremented when a task is queued. But
enqueue_task_in_user_space() never increments it, so tasks sitting in the
enqueued map are invisible to a CPU going idle.

Fixes: cc4448d0856d ("tools/sched_ext: add scx_userland scheduler")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>

> diff --git a/tools/sched_ext/scx_userland.bpf.c b/tools/sched_ext/scx_userland.bpf.c
> index f29862b89386b..648c6bd230eb2 100644
> --- a/tools/sched_ext/scx_userland.bpf.c
> +++ b/tools/sched_ext/scx_userland.bpf.c
> @@ -197,6 +197,7 @@ static void enqueue_task_in_user_space(struct task_struct *p, u64 enq_flags)
>  		scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);
>  	} else {
>  		__sync_fetch_and_add(&nr_user_enqueues, 1);
> +		__sync_fetch_and_add(&nr_queued, 1);

[Severity: High]
Will this introduce a race condition with the user-space scheduler that 
leads to a lost wakeup?

When the kernel atomically increments nr_queued here, the user-space
scheduler unconditionally clobbers it without any synchronization:

tools/sched_ext/scx_userland.c:drain_enqueued_map() {
    ...
    if (bpf_map_lookup_and_delete_elem(enqueued_fd, NULL, &task)) {
        skel->bss->nr_queued = 0;
        ...
}

If drain_enqueued_map() finds the map empty, but just before it runs
skel->bss->nr_queued = 0, a concurrent kernel BPF program enqueues a new
task and increments nr_queued to 1, the user space code will overwrite
it back to 0.

Since both nr_queued and usersched_needed would end up cleared, could this
cause idle CPUs to fail to wake the scheduler, leaving the newly enqueued
task stranded?

>  		set_usersched_needed();
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924143659.268595-1-liwanwu@kylinos.cn?part=1