Re: [RFC][PATCH v14 3/7] sched: Fix runtime accounting w/ split exec & sched contexts

[Peter Zijlstra]

On Mon, Dec 16, 2024 at 10:09:16PM -0800, John Stultz wrote:
> On Fri, Dec 13, 2024 at 4:10 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Sat, Dec 14, 2024 at 12:37:40AM +0100, Peter Zijlstra wrote:
> > > Would it not be *much* clearer if we do it like:
> > >
> > > static s64 update_curr_se(struct rq *rq, struct sched_entity *donor,
> > >                         struct sched_entity *curr)
> > > {
> > >       ...
> > >       donor->exec_start = now;
> > >       curr->exec_start = now;
> > >       curr->sum_exec_runtime += delta_exec;
> > >       ...
> > > }
> > >
> > > and update the callsites like so:
> > >
> > > update_curr_common()
> > >       update_curr_se(rq, &donor->se, &rq->curr.se)
> > >
> > > update_curr()
> > >       update_curr_se(rq, &curr->se, &curr->se);
> > >
> > >
> > > except, now I'm confused about the update_curr() case. That seems to
> > > always update the execution context, rather than the donor ?
> >
> > Ah no, cfs_rq->curr is the donor.
> 
> Yeah. That is one detail in the current series where the naming can be
> particularly confusing.
> 
> I can go through and rename cfs_rq->curr to cfs_rq->donor (or some
> other name) to make it more clear, but it seems like a ton of churn,
> so I've been hesitant to do so until there was stronger consensus to
> taking the patch series, but maybe we're at that point now?

Nah, it was just me being confused, lets keep down the curn for now.

> But maybe a simpler and more isolated fix is I could just rework
> update_curr_se to just take the rq* and we can derive the donor.se and
> curr.se from that.

You can't; rq only has tasks, while cfs_rq is a hierarchy with many se's
backing a single task :/