All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Yu <yu.c.chen@intel.com>
To: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Hui Su <sh_def@163.com>, K Prateek Nayak <kprateek.nayak@amd.com>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Valentin Schneider <vschneid@redhat.com>,
	John Stultz <jstultz@google.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	<linux-kernel@vger.kernel.org>,
	"chen.yu@linux.dev" <chen.yu@linux.dev>
Subject: Re: [PATCH v2 1/2] sched/numa: Drive NUMA task tick from execution context
Date: Sat, 5 Sep 2026 00:10:27 +0800	[thread overview]
Message-ID: <aprtc9-xD2qCZeW0@fengwei-dev> (raw)
In-Reply-To: <08f682ba75a4700694d6f94719b367b4d26c032f.camel@linux.intel.com>

On Thu, Sep 03, 2026 at 02:30:25PM -0700, Tim Chen wrote:
> On Thu, 2026-09-03 at 20:41 +0800, Chen, Yu C wrote:

[ ... ]

> > But I also see that in task_tick_core(), the sum_exec_runtime is also 
> > leveraged
> > to calculate the delta "wall time" via __entity_slice_used():
> > se->sum_exec_runtime - se->prev_sum_exec_runtime
> > does it mean task_tick_core() also needs to be bring one level up to 
> > sched_tick()
> > and passed with rq->curr?
> 
> I think task_tick_core() needs to stay with the donor's context
> as it is the scheduling context.
> 
> task_tick_core() is not about the execution context -- 
> it decides whether the current scheduling context has used
> up enough of its slice to let a force-idled SMT sibling run. That
> slice belongs to the donor, so the donor is the right task to pass.
> 
> There is a separate issue lurking here, task_tick_core() measures consumed slice as
> se->sum_exec_runtime - se->prev_sum_exec_runtime. Under proxy the
> donor's sum_exec_runtime does not advance (update_se() charges the
> runtime to rq->curr instead), so that delta stays near zero and the
> force-idle resched may never trigger. 
> 
> Passing rq->curr does not fix it either. __entity_slice_used() takes
> the runtime and the slice from the same entity, so passing rq->curr
> just compares the running task against its own slice. But this check
> is about the donor: it asks whether the scheduling context that owns
> the CPU has used up its slice. The running task is only borrowing the
> CPU through proxy, so its slice is not the one we care about here.
>

Got it, I see.

> Maybe something like below (only compile tested) to fix the issue.
> That said, this is somewhat orthogonal to the issue that the execution context
> series is trying to solve. It should be fixed separately.
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8dff37059faf..cd240bf52d03 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -14748,10 +14748,29 @@ static void rq_offline_fair(struct rq *rq)
>  static inline bool
>  __entity_slice_used(struct sched_entity *se, int min_nr_tasks)
>  {
> -	u64 rtime = se->sum_exec_runtime - se->prev_sum_exec_runtime;
> -	u64 slice = se->slice;
> +	u64 vslice, vused;
>  
> -	return (rtime * min_nr_tasks > slice);
> +	/*
> +	 * @se is the scheduling context (rq->donor). Under proxy execution
> +	 * it need not be the task executing on the CPU, so its
> +	 * sum_exec_runtime is not advanced and cannot be used to tell how
> +	 * much of its slice it has consumed. Its vruntime, however, is
> +	 * advanced by update_curr() with the proxy runtime, and its EEVDF
> +	 * deadline reflects the granted slice, so measure the consumed
> +	 * fraction in virtual time instead.
> +	 *
> +	 * This is equivalent to the previous real-time comparison in the
> +	 * non-proxy case: both @vused and @vslice are scaled by the same
> +	 * weight factor, so the ratio (and thus the min_nr_tasks test) is
> +	 * unchanged.
> +	 */
> +	if (vruntime_cmp(se->vruntime, ">=", se->deadline))
> +		return true;
> +
> +	vslice = calc_delta_fair(se->slice, se);
> +	vused = vslice - (se->deadline - se->vruntime);
> +
> +	return (vused * min_nr_tasks > vslice);
>  }

This fix looks good to me. And just one minor question that I'm
trying to figure out:

Consider that there is only one running task p on one of the SMT siblings.
The original comparison is between:
se->sum_exec_runtime - se->prev_sum_exec_runtime vs slice,
and since there is only one runnable task, p continues to run
without any preemption, so se->prev_sum_exec_runtime remains
unchanged, while se->sum_exec_runtime moves forward. Therefore,
the duration delta of se->sum_exec_runtime - se->prev_sum_exec_runtime
could expand to many slices in theory.
After switching to the vruntime-based comparison, even
if p has not been preempted, se->deadline together with se->vruntime
will move forward by update_dealine(). That is to say, we now only
consider the delta within one slice. This seems to tighten the
restriction for task_tick_core() to trigger a force reschedule.
But Overall I think it should not be a good deal to check
within a slice period.

thanks,
Chenyu

  parent reply	other threads:[~2026-09-04 16:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03  4:11 [PATCH v2 0/2] sched: Fix execution-context tick handling under proxy execution Hui Su
2026-09-03  4:11 ` [PATCH v2 1/2] sched/numa: Drive NUMA task tick from execution context Hui Su
2026-09-03 12:41   ` Chen, Yu C
2026-09-03 21:30     ` Tim Chen
2026-09-04  5:16       ` Hui Su
2026-09-04 14:10       ` Hui Su
2026-09-04 20:15         ` Tim Chen
2026-09-05 13:58           ` Hui Su
2026-09-08  5:38             ` Chen, Yu C
2026-09-08 16:01               ` Tim Chen
2026-09-09 10:55                 ` Hui Su
2026-09-04 16:10       ` Chen Yu [this message]
2026-09-04 20:24         ` Tim Chen
2026-09-03  4:11 ` [PATCH v2 2/2] sched/cache: Drive cache " Hui Su
2026-09-03  4:37   ` K Prateek Nayak
2026-09-03  4:51     ` Hui Su
2026-09-03 17:23   ` Tim Chen
2026-09-04  4:03     ` Hui Su

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aprtc9-xD2qCZeW0@fengwei-dev \
    --to=yu.c.chen@intel.com \
    --cc=chen.yu@linux.dev \
    --cc=dietmar.eggemann@arm.com \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sh_def@163.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.