linux-kernel - Re: [PATCH v16 4/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: <CANDhNCq1X9dcLCwb9Uod=C-i7giwqWkTso+a6S8n+wXCghq+MQ@mail.gmail.com>
Date: Wed, 16 Apr 2025 16:30:24 -0700
From: John Stultz <jstultz@...gle.com>
To: Juri Lelli <juri.lelli@...hat.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Joel Fernandes <joelagnelf@...dia.com>, 
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>, 
	Peter Zijlstra <peterz@...radead.org>, 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>, 
	Suleiman Souhlal <suleiman@...gle.com>, kernel-team@...roid.com
Subject: Re: [PATCH v16 4/7] sched: Fix runtime accounting w/ split exec &
 sched contexts

On Mon, Apr 14, 2025 at 2:28 AM Juri Lelli <juri.lelli@...hat.com> wrote:
> On 11/04/25 23:02, John Stultz wrote:
> > +static s64 update_se_times(struct rq *rq, struct sched_entity *se)
> >  {
> >       u64 now = rq_clock_task(rq);
> >       s64 delta_exec;
> >
> > -     delta_exec = now - curr->exec_start;
> > +     delta_exec = now - se->exec_start;
> >       if (unlikely(delta_exec <= 0))
> >               return delta_exec;
> >
> > -     curr->exec_start = now;
> > -     curr->sum_exec_runtime += delta_exec;
> > +     se->exec_start = now;
> > +     if (entity_is_task(se)) {
> > +             struct task_struct *running = rq->curr;
> > +             /*
> > +              * If se is a task, we account the time against the running
> > +              * task, as w/ proxy-exec they may not be the same.
> > +              */
> > +             running->se.exec_start = now;
> > +             running->se.sum_exec_runtime += delta_exec;
> > +     } else {
> > +             /* If not task, account the time against se */
> > +             se->sum_exec_runtime += delta_exec;
> > +     }
>
> ...
>
> > @@ -1213,7 +1224,7 @@ s64 update_curr_common(struct rq *rq)
> >       struct task_struct *donor = rq->donor;
> >       s64 delta_exec;
> >
> > -     delta_exec = update_curr_se(rq, &donor->se);
> > +     delta_exec = update_se_times(rq, &donor->se);
> >       if (likely(delta_exec > 0))
> >               update_curr_task(donor, delta_exec);
>
> Considering that we calculate delta_exec in updated_se_times using
> exec_start of the sched_entity passed as argument, is it correct to use
> donor in the above?

Sorry, I'm not sure I quite understand your concern here.  Why are you
thinking using donor might be problematic here?  We're passing the
donor->se in to calculate the delta_exec.

I'll grant that "update_curr_common" maybe isn't the best name for the
calling function anymore, as we're really only working on the donor
here. Is that what your concern stems from?

Apologies for not quite understanding right away. I really appreciate
your review and feedback!

thanks
-john