mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tim Chen <tim.c.chen@linux.intel.com>
To: "Chen, Yu C" <yu.c.chen@intel.com>, Hui Su <sh_def@163.com>
Cc: 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: Thu, 03 Sep 2026 14:30:25 -0700	[thread overview]
Message-ID: <08f682ba75a4700694d6f94719b367b4d26c032f.camel@linux.intel.com> (raw)
In-Reply-To: <4593a7a4-cde1-499c-bba8-2fbe24f35422@intel.com>

On Thu, 2026-09-03 at 20:41 +0800, Chen, Yu C wrote:
> Hi Su,
> 
> On 9/3/2026 12:11 PM, Hui Su wrote:
> > Proxy execution separates the scheduling context in rq->donor from the
> > execution context in rq->curr. sched_tick() invokes task_tick() for the
> > donor's scheduling class.
> > 
> > task_tick_numa() is currently called from task_tick_fair(). This works
> > when the donor is a fair task, but not when a fair task executes on
> > behalf of an RT or deadline donor. In that case the donor's task_tick()
> > still updates the execution task's sum_exec_runtime through
> > update_curr_common(), but task_tick_fair() is not invoked and NUMA scan
> > work for the execution task is not driven.
> 
> Thanks for bringing this up. Previously Prateek has suggested to fix the 
> rq->donor
> issue [1] and unfortunately I missed the task_tick_cache() part.
> 
> Regarding above line in the commit log, although I agree that 
> task_tick_numa()
> should be moved one level up, I did not quite get the reason why 
> sum_exec_runtime
> is mentioned here, could you please elaborate a little more?
> I guess what you mean is that, in task_tick_numa(), the 
> curr->se.sum_exec_runtime
> is used to check if there is a timeout to launch the task_numa_work(), so
> curr->se.sum_exec_runtime has to be up-to-date. With proxy execution, the
> se.sum_exec_runtime is only accumulated in rq->curr rather than rq->donor,
> so passing a "paused" rq->donor.sum_exec_runtime to task_tick_numa() is
> inaccurate?
> 
> 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.

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);
 }
 
 #define MIN_NR_TASKS_DURING_FORCEIDLE	2

> 
> On the other hand, as Prateek mentioned in [1], it seems that 
> sum_exec_runtime
> might not the reason for passing rq->curr, but it could be:
> "with "rq->curr->mm" being the one that is being used on CPU",
> both sched_cache and NUMA balance fit Prateek's conclusion.

I agree with you on this.

Thanks.

Tim

  reply	other threads:[~2026-09-03 21:30 UTC|newest]

Thread overview: 15+ 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 [this message]
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-04 16:10       ` Chen Yu
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=08f682ba75a4700694d6f94719b367b4d26c032f.camel@linux.intel.com \
    --to=tim.c.chen@linux.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=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=yu.c.chen@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®