Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Andrea Righi <arighi@nvidia.com>]

Hello,

On Thu, Nov 12, 2026 at 11:34:28AM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 03:47:00PM +0530, K Prateek Nayak wrote:
> 
> > >> In that case, why should RETRY_TASK want to try balance again for the old
> > >> "rq->donor" for which we have already ran balanced for? Shouldn't it simply
> > >> pick the task then? Am I missing something?
> > > 
> > > need_pull_dl_task() has '&& dl_task(prev)', so if prev was FAIR, then it
> > > will not be true and pick will not cause it to pull another task. And
> > > similar for RT, it checks if the current highest prio is less than the
> > > previous prio, and if prev is FAIR that won't be the case.
> > 
> > Ack, but assume:
> > 
> >   /* RT tasks blocks */
> >   pick_task_rt()
> >     balance_rt()
> >     /* Fails */
> > 
> >   pick_task_fair()
> >     sched_balance_newidle()
> >       raw_spin_rq_unlock()
> > 
> >       /* New RT task wakes up */
> >       
> >       raw_spin_rq_lock()
> >       if (rq_modified_above()) /* True */
> >         return -1
> >      return RETRY_TASK;
> > 
> > 
> >   pick_task_rt()
> >     balance_rt()
> >      need_pull_rt_task() / *Still true */
> >        /* Again: Probably unnecessary? */
> > 
> >     /* But it does find an RT task so loop ends */ 
> 
> Ah, yes, in this case we appear to have an unnecessary balance.
> 
> 
> > > So the problem is with ext :/ They need balance and pick to be one. If
> > > we have separate loops there is the possibility of doing a balance but
> > > then not picking it. In that case ext needs to undo the balance and we
> > > don't have anything for that.
> > > 
> > > Humm...
> > > 
> > > Ah! we now have consistent cross-class wakeup_preempt calls. If we have
> > > the following scenario:
> > > 
> > >   - only EXT tasks
> > >   - balance_ext(), moves task from global DSQ to local DSQ
> > >   - wakeup FAIR
> > >       wakeup_preempt
> > >         if (sched_class_above(p->sched_class, rq->next_class))
> > > 	  rq->next_class->wakeup_preempt(rq, p, flags)
> > > 	    // could move task back to global DSQ
> > 
> > This part I'm slightly confused about (on tip:sched/core):
> > 
> >   prev was RT
> > 
> >   pick_task_scx()
> >     do_pick_task_scx()
> > 
> >       rq_modified_begin(rq, &ext_sched_class);
> > 
> >       dispatch_pick()
> >         dispatch_one()
> >           ops.acquire() <-- CPU is back in control of ext
> > 
> >           /* Pulls a task to local DSQ */
> > 
> >           /* A new RT task wakes up */
> > 
> >       if (!force_scx && rq_modified_above(rq, &ext_sched_class)) /* True for normal pick */
> > 	return RETRY_TASK;
> > 
> >   ... goes and picks next RT task
> > 
> > 
> > The ops.release() is only called when put_prev_task_scx() sees next of
> > higher class which means the local dispatch from the pick stays that
> > way until it is picked to run.
> > 
> > So, the question is, should balance_ext() do anything more than what
> > pick is doing today to pull the task if local DSQ is empty? Am I
> > missing something?
> 
> Aah, wakeup_preempt_scx() already does what it needed here. See how it
> calls scx_schedule_reenq_local() when p's sched class isn't ext.

Right, keep in mind that ops.cpu_acquire()/cpu_release() are deprecated,
the balance-to-pick gap was one of the reasons for deprecating them in
a3f5d48222532 ("sched_ext: Allow scx_bpf_reenqueue_local() to be called
from anywhere").

Specifically, wakeup_preempt_scx() handles SCX_ENQ_IMMED tasks: tasks
that, once inserted into a local DSQ, they must either start running
immediately or be reenqueued back to the BPF scheduler through
ops.enqueue(). If the CPU is unavailable because a higher class takes
precedence, they must not remain waiting on that local DSQ. The
scx_schedule_reenq_local() schedules this deferred reenqueuing.

So, from a sched_ext perspective, we don't need an ops.cpu_release()
notification to handle this higher-class wakeup race, since the existing
IMMED reenqueue mechanism already covers it. Separating balance and pick
should be possible provided rq->next_class tracking and the existing
IMMED reenqueue checks are preserved.

Thanks,
-Andrea