Re: [PATCH v16 4/7] sched: Fix runtime accounting w/ split exec & sched contexts

[Juri Lelli]

Hi John,

On 11/04/25 23:02, John Stultz wrote:
> Without proxy-exec, we normally charge the "current" task for
> both its vruntime as well as its sum_exec_runtime.
> 
> With proxy, however, we have two "current" contexts: the
> scheduler context and the execution context. We want to charge
> the execution context rq->curr (ie: proxy/lock holder) execution
> time to its sum_exec_runtime (so it's clear to userland the
> rq->curr task *is* running).
> 
> Then instead of charging the execution context (rq->curr) for
> the vruntime, we charge the vruntime against the scheduler context
> (rq->donor) task, because that is the time it is donating when
> it is used as the scheduler-context.
> 
> If the donor and curr tasks are the same, then it's the same as
> without proxy.
> 
> Cc: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> Cc: Qais Yousef <qyousef@xxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Juri Lelli <juri.lelli@xxxxxxxxxx>
> Cc: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> Cc: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
> Cc: Valentin Schneider <vschneid@xxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Ben Segall <bsegall@xxxxxxxxxx>
> Cc: Zimuzo Ezeozue <zezeozue@xxxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Waiman Long <longman@xxxxxxxxxx>
> Cc: Boqun Feng <boqun.feng@xxxxxxxxx>
> Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> Cc: Metin Kaya <Metin.Kaya@xxxxxxx>
> Cc: Xuewen Yan <xuewen.yan94@xxxxxxxxx>
> Cc: K Prateek Nayak <kprateek.nayak@xxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> Cc: Suleiman Souhlal <suleiman@xxxxxxxxxx>
> Cc: kernel-team@xxxxxxxxxxx
> Signed-off-by: John Stultz <jstultz@xxxxxxxxxx>
> ---

...

> +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?

Thanks,
Juri