Re: [PATCH v3 1/2] sched_ext: Add dispatch transaction API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scx_bpf_dsq_insert_commit() writes to p->scx.slice before token validation, causing a data race if the task is running
- [Low] Duplicate kfunc ID registration for scx_bpf_dsq_insert_begin
--

commit 1b5ec039cdfb5b4d5077f7a5cfcc34f6f7393655
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>

sched_ext: Add dispatch transaction API

This commit introduces scx_bpf_dsq_insert_begin() and
scx_bpf_dsq_insert_commit() to provide a dispatch transaction API.
It extends the race detection window for schedulers that perform BPF-side
validity checks without properly synchronized dequeue.

Link: https://lore.kernel.org/r/20260203230639.1259869-1-arighi@nvidia.com/

> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 1d7f206941f0c..05704a6a812a2 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c

[ ... ]

> @@ -8518,13 +8520,72 @@ __bpf_kfunc void scx_bpf_dsq_insert_vtime(struct task_struct *p, u64 dsq_id,
> +__bpf_kfunc bool scx_bpf_dsq_insert_commit(struct task_struct *p,
> +					    u64 dsq_id, u64 enq_flags,
> +					    u64 token,
> +					    const struct bpf_prog_aux *aux)
> +{
> +	struct scx_sched *sch;
> +
> +	guard(rcu)();
> +	sch = scx_prog_sched(aux);
> +	if (unlikely(!sch))
> +		return false;
> +
> +	if (!scx_dsq_insert_preamble(sch, p, dsq_id, &enq_flags))
> +		return false;
> +
> +	p->scx.slice = p->scx.slice ?: 1;

Since token validation happens asynchronously in finish_dispatch() after
ops.dispatch() returns, is it safe to write to p->scx.slice here?

If the transaction API is meant to handle cases where the task might be
claimed by another CPU between begin and commit, the task could be actively
running elsewhere when this function is called.

Could this lockless write race with the active CPU's timer tick executing
task_tick_scx() (which decrements the slice concurrently)?

It appears this could result in a read-modify-write that clobbers the
concurrent decrement and corrupts the task's slice accounting.

[ ... ]

>  BTF_KFUNCS_START(scx_kfunc_ids_enqueue_dispatch)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_insert, KF_IMPLICIT_ARGS | KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_insert___v2, KF_IMPLICIT_ARGS | KF_RCU)
> +BTF_ID_FLAGS(func, scx_bpf_dsq_insert_commit, KF_IMPLICIT_ARGS | KF_RCU)
>  BTF_ID_FLAGS(func, __scx_bpf_dsq_insert_vtime, KF_IMPLICIT_ARGS | KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_insert_vtime, KF_RCU)
> +BTF_ID_FLAGS(func, scx_bpf_dsq_insert_begin, KF_RCU)
>  BTF_KFUNCS_END(scx_kfunc_ids_enqueue_dispatch)

[ ... ]

> @@ -10194,6 +10255,7 @@ BTF_ID_FLAGS(func, scx_bpf_put_cpumask, KF_RELEASE)
>  BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_task_cid, KF_RCU)
> +BTF_ID_FLAGS(func, scx_bpf_dsq_insert_begin, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_cpu_rq, KF_IMPLICIT_ARGS)

This isn't a bug, but is it intentional to register scx_bpf_dsq_insert_begin
in both kfunc ID sets?

Because scx_kfunc_context_filter() permits a kfunc if it is present in either
set, and both sets apply to the scheduler context, the duplicate registration
appears functionally redundant.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509191223.168648-1-yphbchou0911@gmail.com?part=1