Re: [PATCH sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[Andrea Righi <arighi@nvidia.com>]

On Sat, Oct 10, 2026 at 10:03:11AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] sched/ext: Balance callback queued on NULL pick risks lockdep warning during newidle_balance
> --
> 
> commit 82685b13d2a91f21d4676e7837dbf23573a2e3ac
> Author: Andrea Righi <arighi@nvidia.com>
> 
>     sched_ext: Don't leave balance callbacks queued across a pick retry
> 
> This commit modifies the dispatch path in sched_ext to only queue balance
> callbacks when the pick is final. If the core needs to retry or continue the
> pick, which may release the runqueue lock, it instead punts the deferred work
> to irq_work.
> 
> > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > index 3f79156467799..54fa382ad8d78 100644
> > --- a/kernel/sched/ext/ext.c
> > +++ b/kernel/sched/ext/ext.c
> 
> [ ... ]
> 
> > @@ -3433,6 +3450,17 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
> >  	return p;
> >  }
> >  
> > +static struct task_struct *
> > +do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
> > +{
> > +	struct task_struct *p = __do_pick_task_scx(rq, rf, force_scx);
> > +
> > +	/* after a NULL pick from the dl_server the lower classes still pick */
> > +	pick_queue_balance_callbacks(rq, p != RETRY_TASK && (p || !force_scx));
>                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> Does this logic incorrectly identify a NULL pick as final when force_scx is
> false, leading to premature queueing of balance callbacks?
> 
> When a normal sched_ext pick (where force_scx == false) fails to find a task,
> it returns NULL. The boolean logic evaluates to true because p != RETRY_TASK
> is true (since p is NULL) and !force_scx is true. This marks the pick as final
> and queues balance callbacks to rq->balance_callback.
> 
> However, a NULL pick from sched_ext is not final for the CPU, as the core
> scheduler loop in __pick_next_task() will fall through to lower-priority
> scheduling classes, such as fair_sched_class.

This looks like a false positive: fair_sched_class is not a lower priority class
than ext_sched_class, it's the other way around.

Neither pick_task_idle() nor set_next_task_idle() release the rq lock, so the
pick is final at that point and the callbacks are consumed before the rq lock is
released.

> If fair also has no runnable tasks, it triggers newidle_balance(), which
> unconditionally drops the runqueue lock via rq_unpin_lock() and
> raw_spin_rq_unlock().
> 
> If another CPU acquires the lock during this window, rq_pin_lock() will
> trigger a SCHED_WARN_ON because a balance callback is present on the runqueue
> while the lock was unpinned. This defeats the patch's purpose and triggers the
> exact lockdep warning it was intended to fix.

The fair newidle balance runs from pick_task_fair(), which is called before
pick_task_scx() in the same pick iteration. If pick_task_scx() returns
RETRY_TASK and the loop restarts, the pick is not considered final and nothing
is left queued, which is exactly the case this patch is addressing.

The only case where a NULL pick is followed by lower classes is the dl_server
one, that is the force_scx == true case and it's already handled as non-final.

-Andrea