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

K Prateek Nayak
2026-03-17T05:41:37Z

Hello John,

On 3/17/2026 10:19 AM, John Stultz wrote:
> 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.

Yeah I had to look a little bit ahead to poke holes here. Sorry about
that!

> 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

Also this needs to be done after find_proxy_task() since
"donor->blocked_on" needs to be cleared to queue callbacks else we'll
bail out on task_is_blocked() check in set_next_task.*() with
PROXY_WAKING when it is done as a part of pick_next_task().

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

Ack! Doing it when the execution context changes with donor context
remaining the same would be the most optimal.

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

That seems like the best option to me too.

Also, deadline, RT, fair, and idle don't really care about the "next"
argument of put_prev_task() and the only one that does care is
put_prev_task_scx() to call switch_class() callback so putting it as
either NULL or "rq->donor" should be safe.

--
Thanks and Regards,
Prateek