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

John Stultz
2026-03-17T18:35:45Z

On Tue, Mar 17, 2026 at 12:53 AM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>
>
> Hello John,
>
> On 3/17/2026 11:34 AM, John Stultz wrote:
> > On Mon, Mar 16, 2026 at 10:41 PM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
> >> On 3/17/2026 10:19 AM, John Stultz wrote:
> >>>
> >>> 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.
> >
> > Ack.
> > Here's the change I'm testing tonight (against 6.18):
> > https://github.com/johnstultz-work/linux-dev/commit/0cc72a4923143f496e33711cbcc1afdf6d861ca6
>
> Thanks a ton for pushing out to WIP! Only nit. would be a:
>
> s/rq->donor/prev_donor/
>
> on the lines with {put_prev/set_next}_task() to save on the
> additional dereference since they are both the same (but maybe
> the complier figures that out on its own?)

While the rq->donor and prev_donor are the same, I sort of preferred using:
prev_donor->sched_class->put_prev_task(rq, prev_donor, rq->donor);
rq->donor->sched_class->set_next_task(rq, rq->donor, true);

As it matches the familiar prev/next pattern used elsewhere (to me it
visually makes more sense and doesn't look as confusing).
I'd hope the compilers woudl sort out they could save the derefernce,
but maybe I can avoid extra derefernces and still preserve the pattern
with a local "donor" variable? That should make sure its simpler for
the compiler to optimize when they are equivalent.

> Also would a sched_proxy_exec() check within that function
> make sense to skip evaluation of that branch entirely when
> proxy exec is disabled via cmdline?

So after Peter's feedback that the sched_proxy_exec() checks weren't
optimizing out the way I'd hope, I'm hesitant to add another check
there. Seems the donor == prev_donor check would amount to the same
amount of work?

> > Feel free to suggest a better name for the helper function. It feels a
> > little clunky (and sort of sad right after getting rid of the clunky
> > proxy_tag_curr(), to re-add something so similar).
>
> I know and I'm sorry about that (T_T)

Oh, no worries! Again, I really appreciate you catching these subtle issues!
thanks
-john