Re: [PATCH 4/4] selftests/sched_ext: Add test to validate ops.dequeue() semantics

[Daniel Jordan]

Hi Andrea,

Looks clean, runs fine.

Reviewed-by: Daniel Jordan <daniel.m.jordan@xxxxxxxxxx>

All minor stuff, no need for a respin here in v10.

> diff --git a/tools/testing/selftests/sched_ext/dequeue.bpf.c b/tools/testing/selftests/sched_ext/dequeue.bpf.c
> +/*
> + * Per-task state to track lifecycle and validate workflow semantics.
> + * State transitions:
> + *   NONE -> ENQUEUED (on enqueue)
> + *   ENQUEUED -> DISPATCHED (on dispatch dequeue)
> + *   DISPATCHED -> NONE (on property change dequeue or re-enqueue)
> + *   ENQUEUED -> NONE (on property change dequeue before dispatch)

NONE -> DISPATCHED is a valid transition too.

> +void BPF_STRUCT_OPS(dequeue_enqueue, struct task_struct *p, u64 enq_flags)
> +{
> +	struct task_ctx *tctx;
> +	s32 pid = p->pid;
> +
> +	tctx = try_lookup_task_ctx(p);
> +	if (!tctx)
> +		return;
> +
> +	switch (test_scenario) {
> +	case 3:
> +		/*
> +		 * Direct dispatch to the local DSQ.
> +		 *
> +		 * Task bypasses BPF scheduler entirely: no enqueue
> +		 * tracking, no ops.dequeue() callbacks.
> +		 */
> +		scx_bpf_dsq_insert(p, SCX_DSQ_LOCAL, SCX_SLICE_DFL, enq_flags);

Could do with

                tctx->state = TASK_DISPATCHED;

> +		break;
> +	case 4:
> +		/*
> +		 * Direct dispatch to the global DSQ.
> +		 *
> +		 * Task bypasses BPF scheduler entirely: no enqueue
> +		 * tracking, no ops.dequeue() callbacks.
> +		 */
> +		scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);

...and also here

                tctx->state = TASK_DISPATCHED;

to be consistent with how select_cpu sets this state, but I don't think setting
DISPATCHED has any effect in select_cpu or here.

> +void BPF_STRUCT_OPS(dequeue_dequeue, struct task_struct *p, u64 deq_flags)
...
> +	} else {
...
> +		/*
> +		 * Dispatch dequeue should not have %SCX_DEQ_SCHED_CHANGE
> +		 * flag.
> +		 */
> +		if (deq_flags & SCX_DEQ_SCHED_CHANGE)
> +			scx_bpf_error("%d (%s): SCX_DEQ_SCHED_CHANGE in dispatch dequeue seq=%llu",
> +				      p->pid, p->comm, tctx->enqueue_seq);

This 'if' can be deleted.