Re: [PATCH v3 2/2] selftests/sched_ext: Add dispatch_cookie test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `scx_bpf_dsq_insert_commit()` is incorrectly called with `SCX_SLICE_DFL` as the `enq_flags` argument in the test.
--

commit 080e78551f77aa8abf00b5d2d1e0d0b5fa62c1b7
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>

selftests/sched_ext: Add dispatch_cookie test

This commit adds a new BPF scheduler selftest to verify the behavior of the
scx_bpf_dsq_insert_begin() and scx_bpf_dsq_insert_commit() APIs.

> diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
> new file mode 100644
> index 0000000000000..ed31cddcda7b6
> --- /dev/null
> +++ b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
[ ... ]
> +void BPF_STRUCT_OPS(dispatch_cookie_dispatch, s32 cpu,
> +		    struct task_struct *prev)
> +{
[ ... ]
> +	stored = bpf_map_lookup_elem(&last_token, &pid);
> +	if (stored) {
> +		token = *stored;
> +		bpf_map_delete_elem(&last_token, &pid);
> +		scx_bpf_dsq_insert_commit(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, token);

Is SCX_SLICE_DFL intended to be used as the enq_flags argument here?

Looking at the function signature in kernel/sched/ext.c:

bool scx_bpf_dsq_insert_commit(struct task_struct *p, u64 dsq_id,
                               u64 enq_flags, u64 token, ...)

It appears scx_bpf_dsq_insert_commit() does not take a slice parameter
like scx_bpf_dsq_insert() does.

Since SCX_SLICE_DFL is a time slice duration, passing it into the enq_flags
parameter might inject undocumented garbage bits into the scheduler's
enqueue operation. Does this unintentionally alter the enqueue behavior?

> +		scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, 0);
> +		__sync_fetch_and_add(&nr_tx_stale, 1);
> +	} else {
> +		token = scx_bpf_dsq_insert_begin(p);
> +		if (scx_bpf_dsq_insert_commit(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, token)) {

There is another instance here where SCX_SLICE_DFL is passed as the third
argument to scx_bpf_dsq_insert_commit().

> +			__sync_fetch_and_add(&nr_tx_dispatched, 1);
> +			bpf_map_update_elem(&last_token, &pid, &token, BPF_ANY);
> +		} else {
> +			scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, 0);
> +		}
> +	}
> +
> +	bpf_task_release(p);
> +}

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