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

[K Prateek Nayak <kprateek.nayak@amd.com>]

(+ Andrea)

On 11/12/2026 3:03 PM, Peter Zijlstra wrote:
> 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.

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 */ 

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

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?


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

Indeed ;-)

-- 
Thanks and Regards,
Prateek