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

[sashiko-bot@kernel.org]

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.

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.

> +
> +	return p;
> +}
> +
>  static struct task_struct *pick_task_scx(struct rq *rq, struct rq_flags *rf)
>  {
>  	return do_pick_task_scx(rq, rf, false);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010095208.276806-1-arighi@nvidia.com?part=1