Re: [PATCH v11 7/7] sched: Split scheduler and execution contexts

[John Stultz]

On Fri, Jul 12, 2024 at 12:10 PM John Stultz <jstultz@xxxxxxxxxx> wrote:
> On Fri, Jul 12, 2024 at 8:02 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Tue, Jul 09, 2024 at 01:31:50PM -0700, John Stultz wrote:
> > > From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > >
> > > Let's define the scheduling context as all the scheduler state
> > > in task_struct for the task selected to run, and the execution
> > > context as all state required to actually run the task.
> > >
> > > Currently both are intertwined in task_struct. We want to
> > > logically split these such that we can use the scheduling
> > > context of the task selected to be scheduled, but use the
> > > execution context of a different task to actually be run.
> > >
> > > To this purpose, introduce rq_selected() macro to point to the
> > > task_struct selected from the runqueue by the scheduler, and
> > > will be used for scheduler state, and preserve rq->curr to
> > > indicate the execution context of the task that will actually be
> > > run.
> >
> > > * Swapped proxy for selected for clarity
> >
> > I'm not loving this naming...  what does selected even mean? What was
> > wrong with proxy? -- (did we have this conversation before?)
>
> So yeah, this came up earlier:
> https://lore.kernel.org/lkml/CANDhNCr3acrEpBYd2LVkY3At=HCDZxGWqbMMwzVJ-Mn--dv3DA@xxxxxxxxxxxxxx/
>
> My big concern is that the way proxy was used early in the series
> seemed to be inverted from how the term is commonly used.
>
> A proxy is one who takes an action on behalf of someone else.
>
> In this case we have a blocked task that was picked to run, but then
> we run another task in its place. Intuitively, this makes the proxy
> the one that actually runs, not the one that was picked. But the
> earliest versions of the patch had this flipped, and caused lots of
> conceptual confusion in the discussions I had with folks about what
> the patch was doing (as well as my own confusion initially working on
> the patch).
>
> So to avoid this, I've minimized the use of the term proxy in the
> code, trying to use less confusing terms.
>
> To me, selected is a more straightforward term, as it's the task the
> scheduler chose to run (but not necessarily the one that actually
> runs).
> And curr is fine enough, as it is the currently running task.
>
> But I'm not wedded to any particular name.  "selected", "chosen",
> (Valentin suggested "picked" earlier, which I'd be ok with, but wanted
> some sense of consensus before I go through to rework it all), but I
> do think "proxy" for the scheduler context would make the code
> unnecessarily difficult for others to understand.
...

> > I realize this is going to be a lot of typing to fix up, and perhaps
> > there's a reason to not do this. But...
>
> I have no problems reworking this if it helps get this series unstuck!
>
> I might strongly push back against "proxy" as the name for the
> scheduler context, but if it really is your favorite, I'll hold my
> nose to move this along.
>
> Do let me know your final preference and I'll go rework it all asap.

Hey Peter,
  Just wanted to check in on this in case the mail slipped by. I'm
eager to rework the naming and get this re-sent, but I just wanted to
confirm your preference here.

thanks
-john