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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCoMz9m_U_86utexFp8+QgVr8rEdGwOygBbNEpwtsByxeA@mail.gmail.com>
Date: Mon, 16 Dec 2024 22:09:16 -0800
From: John Stultz <jstultz@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Joel Fernandes <joelaf@...gle.com>, 
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>, 
	Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>, 
	Dietmar Eggemann <dietmar.eggemann@....com>, Valentin Schneider <vschneid@...hat.com>, 
	Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, 
	Zimuzo Ezeozue <zezeozue@...gle.com>, Mel Gorman <mgorman@...e.de>, Will Deacon <will@...nel.org>, 
	Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>, 
	"Paul E. McKenney" <paulmck@...nel.org>, Metin Kaya <Metin.Kaya@....com>, 
	Xuewen Yan <xuewen.yan94@...il.com>, K Prateek Nayak <kprateek.nayak@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, Daniel Lezcano <daniel.lezcano@...aro.org>, kernel-team@...roid.com
Subject: Re: [RFC][PATCH v14 3/7] sched: Fix runtime accounting w/ split exec
 & sched contexts

On Fri, Dec 13, 2024 at 4:10 PM Peter Zijlstra <peterz@...radead.org> 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?

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.

Thoughts?
thanks
-john