Re: [PATCH 6/6] sched_ext: Fix rq->core_pick corruption under core scheduling

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Under core scheduling, a pending kick-sync operation is silently ignored and lost when a sibling CPU performs the core-wide pick, breaking the wait guarantees of the scx_bpf_kick_cpu() kfunc.
--

--- Patch [6]: [PATCH 6/6] sched_ext: Fix rq->core_pick corruption under core scheduling ---
commit 80aba2ffb0bb3ad3c3cd2d54f2eb79683a1d623d
Author: Tejun Heo <[email protected]>

sched_ext: Fix rq->core_pick corruption under core scheduling

This commit addresses an issue in sched_ext under core scheduling where
dispatching could drop the rq lock and corrupt the core-wide selection state.
It fixes this by restarting the selection when dispatch releases the lock,
ensuring picks are committed atomically.

Link: https://github.com/sched-ext/scx/issues/3715

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index ebe40953acf01..c704588854879 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -3241,6 +3242,70 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
>  	return verdict;
>  }
>  
> +#ifdef CONFIG_SCHED_CORE
> +/*
> + * Dispatch for a pick when core scheduling is enabled. The selection picks for
> + * all SMT siblings and the rq_i->core_pick state it builds must stay atomic
> + * throughout. If the dispatch released the rq lock, anything can have happened
> + * in between - return %SCX_DSP_RETRY to restart the selection against current
> + * state.
> + */
> +static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *rf,
> +					       struct task_struct *prev)
> +{
> +	enum scx_dsp_verdict verdict;
> +	u32 seq = rq->scx.lock_drop_seq;
> +
> +	/* another dispatch is in flight on @rq, let that handle it */
> +	if (rq->scx.flags & SCX_RQ_IN_BALANCE)
> +		return SCX_DSP_NONE;
> +
> +	rq_unpin_lock(rq, rf);
> +
> +	verdict = balance_one(rq, prev);
> +
> +	if (cpu_of(rq) == smp_processor_id()) {
> +		maybe_queue_balance_callback(rq);
> +
> +		/* see dispatch_pick() */
> +		if (unlikely(rq->scx.kick_sync_pending)) {
> +			rq->scx.kick_sync_pending = false;
> +			queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
> +					       kick_sync_wait_bal_cb);
> +		}
> +	} else if (unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {

[Severity: High]
Does this silently drop pending kick-syncs for remote sibling runqueues under
core scheduling?

Because the kick_sync_pending check is under the cpu_of(rq) ==
smp_processor_id() branch, if a sibling CPU performs the core-wide pick for
this remote runqueue, the check is skipped.

The core picker then sets rq->core_pick for this runqueue. When the kicked
sibling CPU enters __schedule(), it hits the core-scheduling fast-path and
bypasses do_pick_task_scx(). As a result, dispatch_core_pick() is never
executed locally.

Could this lead to the kick_sync_wait_bal_cb balance callback never being
queued, breaking the synchronization guarantees of scx_bpf_kick_cpu() and
leaving the BPF scheduler operating on incorrect assumptions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6