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

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Nov 12, 2026 at 12:16:06PM +0530, K Prateek Nayak wrote:
> Hello Peter,
> 
> On 11/7/2026 5:29 PM, Peter Zijlstra wrote:
> > Ever since commit 50653216e4ff ("sched: Add support to pick functions to take
> > rf"), we have the unfortunate situation that both sched_class::balance() and
> > sched_class::pick_task() have overlapping functionality in that they drop
> > rq->lock and balance tasks.
> > 
> > Additionally, prev_balance() is only called for a single RQ in the core-sched
> > case, resulting in 'missed' balance opportunities in this case.
> 
> So I have a fundamental question - looking at the deadline and rt balance
> callbacks, the balance is only done if rq->donor was taken off of rq
> (blocked / throttled) and we need to pull a waiting task that might be of
> higher priority.
> 
> Now, if the balance (say for RT) failed, and later, a different class' (fair's)
> balance callback dropped the lock during which a RT task was woken up on the
> CPU, I believe the wakeup path would have already done the balance while
> considering this CPU for wakeup right?

Indeed.

> 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.

> > Just like pick_task(), prev_balance() would iterate the classes and stop when
> > there is a runnable task found. The only difference is that pick_task() starts
> > from the top class, while prev_balance() would start from prev->class.
> > 
> > This means that if prev was an RT task, we'd get an extra visit to balance_dl()
> > -- and similary, if prev is fair, we'd visit balance_{dl,rt}() both. However,
> > this is not a problem, because both need_pull_{dl,rt}_task() will DTRT.
> 
> Balance still sees old donor and it can still try a pull if the newly
> woken task is on a lower priority than the rq->donor. Is that desirable?
> 
> > Therefore, move balance_{rt,dl}() into pick_task_{rt,dl}() and remove
> > sched_class::balance().
> 
> Until commit 50653216e4f ("sched: Add support to pick functions to take rf")
> that was how pick worked and we should perhaps stick to that. commit 67692435c41
> ("sched: Rework pick_next_task() slow-path"), made it such that none of the picks
> could actually return RETRY_TASK and we are actually moving back to the same
> state as 67692435c41~1 with these changes.

I know :-(

> IMO, we should re-introduce balance_fair() for newidle_balance, and drain all
> the delayed task if !h_nr_runnable && h_nr_queued to avoid pick hitting the idle
> label altogether.
> 
> Doing newidle balance for every idle rq's core pick seems too aggressive. I
> think we should let idle balance / balance callback handle those movements after
> rq->core_cookie is stable.
> 
> This also allows for us to now do rq->next_class->pick_task() and that should
> always return a runnable task and we don't have to ever juggle rq_lock between
> picks which has already led to a bunch of issues.

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
  - pick FAIR
  
This will need some surgery, but might just work if we have
prev_balance() set the rq->next_class.

I suppose I'll go try this, the end result is much nicer.