Re: [PATCH v25 1/9] sched: Make class_schedulers avoid pushing current, and get rid of proxy_tag_curr()

John Stultz
2026-03-17T04:49:52Z

On Sun, Mar 15, 2026 at 9:27 AM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>
> Back to my concern with the queuing of the balance_callback, and the
> deadline and RT folks can keep me honest here, consider the following:
>
> CPU0
> ====
>
> ======> Task A (prio: 80)
> ...
>
> mutex_lock(Mutex0)
> ... /* Executing critical section. */
>
> =====> Interrupt: Wakes up Task B (prio: 50); B->blocked_on = Mutex0;
> resched_curr()
> <===== Interrupt return
> preempt_schedule_irq()
> schedule()
> put_prev_set_next_Task(A, B)
> rq->donor = B
> if (task_is_blocked(B)
> next = find_proxy_task() /* Return Task A */
> rq->curr = A
> queue_balance_callback()
> do_balance_callbacks()
> /* Finds A as task_on_cpu(); Does nothing. */
>
> ... /* returns from schedule */
> ... /* continues with critical section */
>
> mutex_unlock(Mutex0)
> mutex_handoff(B /* Task B */)
> preempt_disable()
> try_to_wake_up()
> resched_curr()
> preempt_enable()
> preempt_schedule()
> proxy_force_return()
> /* Returns to same CPU */
>
> /*
> * put_prev_set_next_task() is skipped since
> * rq->donor context is same. no balance
> * callbacks are queued. Task A still on the
> * push list.
> */
> rq->donor = B
> rq->curr = B
> =======> sched_out: Task A
>
> !!! No balance callback; Task A still on push list. !!!
>
> <======= sched_in: Task B

Hrm. I'm feeling like I'm a little lost here, specifically after
proxy_force_return(), since it doesn't exist yet at this point in the
patch series. But assuming we're at the "Handle blocked-waiter
migration" point in the series, I'd think it would be something like:

rq->donor= B
rq->curr = A
<< task A >>
mutex_unlock(Mutex0)
mutex_handoff(B /* Task B */)
preempt_disable()
try_to_wake_up()
resched_curr()
preempt_enable()
preempt_schedule()
__schedule()
find_proxy_task()
proxy_force_return()
return NULL
pick_again:
next = pick_next_task()
__pick_next_task() /* Returns B */
rq->donor =B
rq->curr = B
context_switch()
<<switch to B >>
finish_task_switch()
finish_lock_switch()
__balance_callbacks()

Your point "put_prev_set_next_task() is skipped since rq->donor
context is same" wasn't initially obvious to me, as the fair scheduler
does have a (p == prev) check, but it doesn't enqueue balance
callbacks. And for RT/DL/SCX we should be using the pick_task()
method, which calls put_prev_set_next_task() in __pick_next_task().
But indeed, *inside* of put_prev_set_next_task() we return early if
(next == prev).

So I see your concern and agree.

> So what I'm getting to is, if we find that rq->donor has not changed
> with sched_proxy_exec() but rq->curr has changed during schedule(), we
> should forcefully do a:
>
> prev->sched_class->put_prev_task(rq, rq->donor, rq->donor /* or rq->idle / NULL ? */);
> next->sched_class->set_next_task(rq, rq->donor, true /* to queue balance callback. */);
>
> That way, when we do set_nex_task(), we see if we potentially have
> tasks in the push list and queue a balance callback since the
> task_on_cpu() condition may no longer apply to the tasks left behind
> on the list.
>
> Thoughts?

Yeah. I wonder if we can express this inside of
put_prev_set_next_task(). Reworking the shortcut to maybe:
if (next == prev && next != rq->curr)

I probably need to think on this tomorrow, as I suspect the above has
some holes, but it seems like it would catch the cases that would
matter (maybe the issue is it catches too much - we'd probably also
trip it if we A boosted B, and then we hit schedule and again chose A
to boost B, which we probably could have skipped).

I guess adding a new helper function to manually do the
put_prev/set_next could be added to the top level __schedule() logic
in the (prev != next) case, though we'll have to preserve the
prev_donor on the stack probably.

thanks
-john