mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Chen, Yu C" <yu.c.chen@intel.com>
Cc: Hui Su <sh_def@163.com>,
	mingo@redhat.com, tim.c.chen@linux.intel.com,
	kprateek.nayak@amd.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	vschneid@redhat.com, connoro@google.com, jstultz@google.com,
	linux-kernel@vger.kernel.org, arighi@nvidia.com
Subject: Re: [PATCH v3 1/2] sched/numa: Drive NUMA task tick from execution context
Date: Tue, 8 Sep 2026 12:44:07 +0200	[thread overview]
Message-ID: <20260908104407.GD687043@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <a89a2aef-f374-4fd6-aafe-ca21d21c980d@intel.com>

On Tue, Sep 08, 2026 at 06:02:26PM +0800, Chen, Yu C wrote:
> On 9/8/2026 4:35 PM, Peter Zijlstra wrote:
> > What about something like so?
> > 
> 
> Thanks Peter for taking a look at this. I think this version achieves
> better decoupling since the logic is added per scheduling class,
> rather than inserting random hooks into the core scheduler.

Right :-)

> > -static void task_tick_scx(struct rq *rq, struct task_struct *curr, int queued)
> > +static void task_tick_scx(struct rq *rq, int queued)
> >   {
> > +	struct task_struct *curr = rq->donor;
> 
> struct task_struct *donor = rq->donor

Nah, it wants to be curr, like the argument was called before. I'll fix
that class check.

> >   	struct scx_sched *sch = scx_task_sched(curr);
> > +	if (donor->sched_class != &ext_sched_class)
> > +		return;
> > +
> >   	update_curr_scx(rq);
> >   	/*
> 
> [ ... ]
> 
> > -static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
> > +static void task_tick_fair(struct rq *rq, int queued)
> >   {
> > -	struct sched_entity *se = &curr->se;
> > +	struct task_struct *curr = rq->curr, *donor = rq->donor;
> > -	if (se->on_rq) {
> > -		unsigned long weight = NICE_0_LOAD;
> > -		struct cfs_rq *cfs_rq;
> > +	if (donor->sched_class == &fair_sched_class) {
> > +		struct sched_entity *se = &donor->se;
> > -		for_each_sched_entity(se) {
> > -			cfs_rq = cfs_rq_of(se);
> > -			entity_tick(cfs_rq, se, queued);
> > +		if (se->on_rq) {
> > +			unsigned long weight = NICE_0_LOAD;
> > +			struct cfs_rq *cfs_rq;
> > -			weight = __calc_prop_weight(cfs_rq, se, weight);
> > +			for_each_sched_entity(se) {
> > +				cfs_rq = cfs_rq_of(se);
> > +				entity_tick(cfs_rq, se, queued);
> > +
> > +				weight = __calc_prop_weight(cfs_rq, se, weight);
> > +			}
> > +
> > +			se = &donor->se;
> > +			reweight_eevdf(cfs_rq, se, weight, se->on_rq);
> >   		}
> > -		se = &curr->se;
> > -		reweight_eevdf(cfs_rq, se, weight, se->on_rq);
> > +		if (queued)
> > +			return;
> > +
> > +		update_misfit_status(donor, rq);
> > +		check_update_overutilized_status(task_rq(donor));
> > +
> > +		task_tick_core(rq, donor);
> >   	}
> > -	if (queued)
> > -		return;
> 
> The queued check might still be needed: if donor is not a fair task, we
> still
> want to skip the numa balancing/sched_cache for hrtick event?

Oh right. I have a patch pending that renames that thing. But yes, I
very much rushed this PoC patch without minding the details very much.

I also wondered if we should have task_tick() do curr_class->task_tick()
last, rather than first. But I couldn't immediately find a compelling
argument either way around.

Updated...

---
 kernel/sched/core.c      | 16 +++++++++++++---
 kernel/sched/deadline.c  |  7 ++++++-
 kernel/sched/ext/ext.c   |  6 +++++-
 kernel/sched/fair.c      | 49 ++++++++++++++++++++++++++----------------------
 kernel/sched/idle.c      |  5 +++--
 kernel/sched/rt.c        |  6 +++++-
 kernel/sched/sched.h     |  2 +-
 kernel/sched/stop_task.c |  2 +-
 8 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 853afc869715..1f42bb856337 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -910,6 +910,16 @@ static void __used hrtick_clear(struct rq *rq)
 		hrtimer_cancel(&rq->hrtick_timer);
 }
 
+static inline void task_tick(struct rq *rq, int queued)
+{
+	const struct sched_class *curr_class = rq->curr->sched_class,
+				*donor_class = rq->donor->sched_class;
+
+	curr_class->task_tick(rq, queued);
+	if (sched_proxy_exec() && donor_class != curr_class)
+		donor_class->task_tick(rq, queued);
+}
+
 /*
  * High-resolution timer tick.
  * Runs from hardirq context with interrupts disabled.
@@ -923,7 +933,7 @@ static enum hrtimer_restart hrtick(struct hrtimer *timer)
 
 	rq_lock(rq, &rf);
 	update_rq_clock(rq);
-	rq->donor->sched_class->task_tick(rq, rq->donor, 1);
+	task_tick(rq, 1);
 	rq_unlock(rq, &rf);
 
 	return HRTIMER_NORESTART;
@@ -5800,7 +5810,7 @@ void sched_tick(void)
 	if (dynamic_preempt_lazy() && tif_test_bit(TIF_NEED_RESCHED_LAZY))
 		resched_curr(rq);
 
-	donor->sched_class->task_tick(rq, donor, 0);
+	task_tick(rq, 0);
 	if (sched_feat(LATENCY_WARN))
 		resched_latency = cpu_resched_latency(rq);
 	calc_global_load_tick(rq);
@@ -5896,7 +5906,7 @@ static void sched_tick_remote(struct work_struct *work)
 				u64 delta = rq_clock_task(rq) - curr->se.exec_start;
 				WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 30);
 			}
-			curr->sched_class->task_tick(rq, curr, 0);
+			task_tick(rq, 0);
 
 			calc_load_nohz_remote(rq);
 		}
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index de6a361a87c7..65b83ff81201 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2876,8 +2876,13 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_s
  * and everything must be accessed through the @rq and @curr passed in
  * parameters.
  */
-static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
+static void task_tick_dl(struct rq *rq, int queued)
 {
+	struct task_struct *p = rq->donor;
+
+	if (p->sched_class != &dl_sched_class)
+		return;
+
 	update_curr_dl(rq);
 
 	update_dl_rq_load_avg(rq_clock_pelt(rq), rq, 1);
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 51de1d8b72a1..9c760d46f8dd 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3789,10 +3789,14 @@ void scx_tick(struct rq *rq)
 	update_other_load_avgs(rq);
 }
 
-static void task_tick_scx(struct rq *rq, struct task_struct *curr, int queued)
+static void task_tick_scx(struct rq *rq, int queued)
 {
+	struct task_struct *curr = rq->donor;
 	struct scx_sched *sch = scx_task_sched(curr);
 
+	if (curr->sched_class != &ext_sched_class)
+		return;
+
 	update_curr_scx(rq);
 
 	/*
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b8bd308c2d5b..bbb239a22c5e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -15046,37 +15046,42 @@ static inline void task_tick_core(struct rq *rq, struct task_struct *curr) {}
  * and everything must be accessed through the @rq and @curr passed in
  * parameters.
  */
-static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
+static void task_tick_fair(struct rq *rq, int queued)
 {
-	struct sched_entity *se = &curr->se;
+	struct task_struct *curr = rq->curr, *donor = rq->donor;
 
-	if (se->on_rq) {
-		unsigned long weight = NICE_0_LOAD;
-		struct cfs_rq *cfs_rq;
+	if (donor->sched_class == &fair_sched_class) {
+		struct sched_entity *se = &donor->se;
 
-		for_each_sched_entity(se) {
-			cfs_rq = cfs_rq_of(se);
-			entity_tick(cfs_rq, se, queued);
+		if (se->on_rq) {
+			unsigned long weight = NICE_0_LOAD;
+			struct cfs_rq *cfs_rq;
 
-			weight = __calc_prop_weight(cfs_rq, se, weight);
+			for_each_sched_entity(se) {
+				cfs_rq = cfs_rq_of(se);
+				entity_tick(cfs_rq, se, queued);
+
+				weight = __calc_prop_weight(cfs_rq, se, weight);
+			}
+
+			se = &donor->se;
+			reweight_eevdf(cfs_rq, se, weight, se->on_rq);
 		}
 
-		se = &curr->se;
-		reweight_eevdf(cfs_rq, se, weight, se->on_rq);
+		if (!queued) {
+			update_misfit_status(donor, rq);
+			check_update_overutilized_status(task_rq(donor));
+
+			task_tick_core(rq, donor);
+		}
 	}
 
-	if (queued)
-		return;
+	if (curr->sched_class == &fair_sched_class && !queued) {
+		if (static_branch_unlikely(&sched_numa_balancing))
+			task_tick_numa(rq, curr);
 
-	if (static_branch_unlikely(&sched_numa_balancing))
-		task_tick_numa(rq, curr);
-
-	task_tick_cache(rq, curr);
-
-	update_misfit_status(curr, rq);
-	check_update_overutilized_status(task_rq(curr));
-
-	task_tick_core(rq, curr);
+		task_tick_cache(rq, curr);
+	}
 }
 
 /*
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index eb73b65ce6c4..077eed68869e 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -535,9 +535,10 @@ dequeue_task_idle(struct rq *rq, struct task_struct *p, int flags)
  * and everything must be accessed through the @rq and @curr passed in
  * parameters.
  */
-static void task_tick_idle(struct rq *rq, struct task_struct *curr, int queued)
+static void task_tick_idle(struct rq *rq, int queued)
 {
-	update_curr_idle(rq);
+	if (rq->donor->sched_class == &idle_sched_class)
+		update_curr_idle(rq);
 }
 
 static void switching_to_idle(struct rq *rq, struct task_struct *p)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 85303add726d..5e4ff18589c2 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2538,10 +2538,14 @@ static inline void watchdog(struct rq *rq, struct task_struct *p) { }
  * and everything must be accessed through the @rq and @curr passed in
  * parameters.
  */
-static void task_tick_rt(struct rq *rq, struct task_struct *p, int queued)
+static void task_tick_rt(struct rq *rq, int queued)
 {
+	struct task_struct *p = rq->donor;
 	struct sched_rt_entity *rt_se = &p->rt;
 
+	if (p->sched_class != &rt_sched_class)
+		return;
+
 	update_curr_rt(rq);
 	update_rt_rq_load_avg(rq_clock_pelt(rq), rq, 1);
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6c3ad70e58b8..ca6ef6f0dcb4 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2727,7 +2727,7 @@ struct sched_class {
 	 * sched_tick: rq->lock
 	 * sched_tick_remote: rq->lock
 	 */
-	void (*task_tick)(struct rq *rq, struct task_struct *p, int queued);
+	void (*task_tick)(struct rq *rq, int queued);
 	/*
 	 * sched_cgroup_fork: p->pi_lock
 	 */
diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
index c909ca0d8c87..c3ad1eb4bc5f 100644
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -71,7 +71,7 @@ static void put_prev_task_stop(struct rq *rq, struct task_struct *prev, struct t
  * and everything must be accessed through the @rq and @curr passed in
  * parameters.
  */
-static void task_tick_stop(struct rq *rq, struct task_struct *curr, int queued)
+static void task_tick_stop(struct rq *rq, int queued)
 {
 }
 

  reply	other threads:[~2026-09-08 10:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04  8:52 [PATCH v3 0/2] sched: Fix execution-context tick handling under proxy execution Hui Su
2026-09-04  8:52 ` [PATCH v3 1/2] sched/numa: Drive NUMA task tick from execution context Hui Su
2026-09-04 15:52   ` Chen Yu
2026-09-04 17:17   ` Tim Chen
2026-09-08  7:40   ` Chen, Yu C
2026-09-08  8:35   ` Peter Zijlstra
2026-09-08 10:02     ` Chen, Yu C
2026-09-08 10:44       ` Peter Zijlstra [this message]
2026-09-08 12:10         ` Hui Su
2026-09-09  8:37           ` Peter Zijlstra
2026-09-04  8:52 ` [PATCH v3 2/2] sched/cache: Drive cache " Hui Su
2026-09-08  7:45   ` Chen, Yu C

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=20260908104407.GD687043@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arighi@nvidia.com \
    --cc=bsegall@google.com \
    --cc=connoro@google.com \
    --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=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=sh_def@163.com \
    --cc=tim.c.chen@linux.intel.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®