The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] sched/fair: Restart hrtick after same-task repicks
@ 2026-08-13 21:23 Shubhang Kaushik (Ampere)
  2026-08-26  1:13 ` Shubhang
  2026-08-26  4:00 ` Zhan Xusheng
  0 siblings, 2 replies; 3+ messages in thread
From: Shubhang Kaushik (Ampere) @ 2026-08-13 21:23 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Shubhang Kaushik,
	Christoph Lameter
  Cc: linux-kernel, Shubhang Kaushik (Ampere)

Fair hrtick is implemented with a one-shot timer, so each precise
preemption point has to be programmed explicitly. The usual fair path
does this from set_next_task_fair(), which calls hrtick_start_fair().

The missed path is:

  hrtick
    -> task_tick_fair(..., queued=1)
       -> entity_tick()
	  -> resched_curr()
    -> schedule()
       -> pick_task_fair() picks current again
       -> put_prev_set_next_task()
	  -> next == prev
	  -> return

Since set_next_task_fair() is skipped, hrtick_start_fair() is not called
and no new fair hrtick is started.

Record when a queued fair hrtick may need a restart, and consume that
state only from the same-task fast path. Limit this to cases where more
than one fair entity is runnable and all queued fair entities are
runnable, avoiding extra hrticks for delayed-dequeue and pipe-like cases
where queued entities are not all competing for CPU time.

Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
---
On v7.2-rc7 mainline (3aa1dcaa4f6f), with HRTICK enabled,
base_slice_ns=3000000, and two CPU-bound tasks pinned to one CPU, the
nice-0 task's runtime intervals above 8ms dropped from 228 in a 10s
perf sched capture to 34-38 across repeated runs with this change.

A similar missed hrtick start was previously reported for the older
pick_next_task_fair() flow:
  Message-ID: <20241111074841.8802-1-shijie@os.amperecomputing.com>
---
 kernel/sched/core.c  |  2 ++
 kernel/sched/fair.c  | 29 ++++++++++++++++++++++++++++-
 kernel/sched/sched.h | 21 ++++++++++++++++++++-
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 96226707c2f6135341aa779b8262f113e103d8ad..5ec8c3f752fa48149469907edb595ede0769e94a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1013,12 +1013,14 @@ static inline void hrtick_schedule_exit(struct rq *rq)
 		__hrtimer_rearm_deferred();
 
 	rq->hrtick_sched = HRTICK_SCHED_NONE;
+	rq->hrtick_rearm_fair = false;
 }
 
 static void hrtick_rq_init(struct rq *rq)
 {
 	INIT_CSD(&rq->hrtick_csd, __hrtick_start, rq);
 	rq->hrtick_sched = HRTICK_SCHED_NONE;
+	rq->hrtick_rearm_fair = false;
 	hrtimer_setup(&rq->hrtick_timer, hrtick, CLOCK_MONOTONIC,
 		      HRTIMER_MODE_REL_HARD | HRTIMER_MODE_LAZY_REARM);
 }
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d78467ec6ee1343050fcc2794dafb38ade3599e5..2d90a9a84175833bdb78f6f78d23b124105fcb82 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7681,6 +7681,22 @@ static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
 	hrtick_start(rq, (scale * delta) / 1024);
 }
 
+void __hrtick_rearm_fair(struct rq *rq, struct task_struct *p)
+{
+	rq->hrtick_rearm_fair = false;
+
+	if (!hrtick_enabled_fair(rq))
+		return;
+
+	if (hrtick_active(rq))
+		return;
+
+	if (p->sched_class != &fair_sched_class)
+		return;
+
+	hrtick_start_fair(rq, p);
+}
+
 /*
  * Called on enqueue to start the hrtick when h_nr_queued becomes more than 1.
  */
@@ -14858,8 +14874,19 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
 		entity_tick(cfs_rq, se, queued);
 	}
 
-	if (queued)
+	if (queued) {
+		/*
+		 * Fair hrtick is one-shot. If this hrtick-triggered
+		 * reschedule picks the same task again, set_next_task_fair()
+		 * will be skipped. Mark that path for a possible restart, but
+		 * avoid delayed-dequeue cases where queued entities are not all
+		 * runnable.
+		 */
+		rq->hrtick_rearm_fair = hrtick_enabled_fair(rq) &&
+					rq->cfs.h_nr_runnable > 1 &&
+					rq->cfs.h_nr_runnable == rq->cfs.h_nr_queued;
 		return;
+	}
 
 	if (static_branch_unlikely(&sched_numa_balancing))
 		task_tick_numa(rq, curr);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 56acf502ba260ab18bacd7a4c2efdec612d50125..faf63eea233981fbd7e0a13b652f0c37d292ef35 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1313,6 +1313,7 @@ struct rq {
 	ktime_t			hrtick_time;
 	ktime_t			hrtick_delay;
 	unsigned int		hrtick_sched;
+	bool			hrtick_rearm_fair;
 #endif
 
 #ifdef CONFIG_SCHEDSTATS
@@ -2745,6 +2746,18 @@ static inline void set_next_task(struct rq *rq, struct task_struct *next)
 	next->sched_class->set_next_task(rq, next, false);
 }
 
+#ifdef CONFIG_SCHED_HRTICK
+void __hrtick_rearm_fair(struct rq *rq, struct task_struct *p);
+
+static inline void hrtick_rearm_fair(struct rq *rq, struct task_struct *p)
+{
+	if (rq->hrtick_rearm_fair)
+		__hrtick_rearm_fair(rq, p);
+}
+#else
+static inline void hrtick_rearm_fair(struct rq *rq, struct task_struct *p) { }
+#endif
+
 static inline void
 __put_prev_set_next_dl_server(struct rq *rq,
 			      struct task_struct *prev,
@@ -2763,8 +2776,14 @@ static inline void put_prev_set_next_task(struct rq *rq,
 
 	__put_prev_set_next_dl_server(rq, prev, next);
 
-	if (next == prev)
+	if (next == prev) {
+		/*
+		 * Same-task repicks skip class callbacks. Restart fair hrtick
+		 * if the queued tick path marked it as needed.
+		 */
+		hrtick_rearm_fair(rq, next);
 		return;
+	}
 
 	prev->sched_class->put_prev_task(rq, prev, next);
 	next->sched_class->set_next_task(rq, next, true);

---
base-commit: 3aa1dcaa4f6f5ae08936491e08bd456f331f2d40
change-id: 20260813-sched-fair-hrtick-restart-ab9d3d47ef78

Best regards,
-- 
Shubhang Kaushik (Ampere) <sh@gentwo.org>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] sched/fair: Restart hrtick after same-task repicks
  2026-08-13 21:23 [PATCH] sched/fair: Restart hrtick after same-task repicks Shubhang Kaushik (Ampere)
@ 2026-08-26  1:13 ` Shubhang
  2026-08-26  4:00 ` Zhan Xusheng
  1 sibling, 0 replies; 3+ messages in thread
From: Shubhang @ 2026-08-26  1:13 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Shubhang Kaushik,
	Christoph Lameter
  Cc: linux-kernel

Hello everyone,

A gentle ping for this patch, sent on August 13.

A fair hrtick is one-shot. When it expires and schedule() selects the
current fair task again, the next == prev path skips
set_next_task_fair(), which normally starts the next fair hrtick.
Consequently, no new fair hrtick is armed after that same-task repick.

The patch starts a new one-shot fair hrtick only in this next == prev
path, and only when all queued fair entities are runnable
(h_nr_runnable == h_nr_queued).

Could you please comment on whether this is the appropriate point and
condition for starting the next fair hrtick?

Thanks,
Shubhang Kaushik

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] sched/fair: Restart hrtick after same-task repicks
  2026-08-13 21:23 [PATCH] sched/fair: Restart hrtick after same-task repicks Shubhang Kaushik (Ampere)
  2026-08-26  1:13 ` Shubhang
@ 2026-08-26  4:00 ` Zhan Xusheng
  1 sibling, 0 replies; 3+ messages in thread
From: Zhan Xusheng @ 2026-08-26  4:00 UTC (permalink / raw)
  To: sh
  Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, kprateek.nayak, shubhang, cl,
	linux-kernel, zhanxusheng

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

On Thu, 13 Aug 2026 14:23:48 -0700, Shubhang Kaushik (Ampere) wrote:
> +		rq->hrtick_rearm_fair = hrtick_enabled_fair(rq) &&
> +				rq->cfs.h_nr_runnable > 1 &&
> +				rq->cfs.h_nr_runnable == rq->cfs.h_nr_queued;

The last term switches the fix off whenever anything on the rq sits in
delayed dequeue.  set_delayed() decrements h_nr_runnable and leaves
h_nr_queued alone (kernel/sched/fair.c:6398), clear_delayed() puts it
back (6418), so the two differ exactly while a delay-dequeued entity is
present.  With DELAY_DEQUEUE that is routine, and it says nothing about
whether the running task still needs its slice bounded.

Your test cannot show that either way: two CPU-bound tasks pinned to one
CPU never sleep, so nothing is ever delay-dequeued there and the term is
true for the whole run.  Adding a third task that sleeps in a loop should
bring the missed hrtick back while the term is false.

If the intent is only to skip rqs whose other queued entities are not
competing for the CPU, h_nr_runnable > 1 already says that by itself.

> +static inline void hrtick_rearm_fair(struct rq *rq, struct task_struct *p)
> +{
> +	if (rq->hrtick_rearm_fair)
> +		__hrtick_rearm_fair(rq, p);
> +}

What does the rq field buy?  __hrtick_rearm_fair() already tests
hrtick_enabled_fair(), hrtick_active() and the class, and a same-task
repick that finds no hrtick armed wants one regardless of what triggered
the repick.  If there is a same-task repick that must not arm one, the
changelog is the place to name it.

Last one is only a question.  entity_tick() -> update_curr() ->
update_deadline() has already pushed se->deadline by a slice before
task_tick_fair() reaches the queued branch, so hrtick_start_fair() would
compute a valid delay if called right there, with no new field and no
change to put_prev_set_next_task().  The difference I can see is that the
tick callback runs with rq->hrtick_sched == 0, so hrtick_start() would
program the hrtimer immediately from inside its own callback instead of
leaving it to hrtick_schedule_exit().  Is that what moved you to the pick
side?

Thanks,
Zhan Xusheng

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-26  4:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 21:23 [PATCH] sched/fair: Restart hrtick after same-task repicks Shubhang Kaushik (Ampere)
2026-08-26  1:13 ` Shubhang
2026-08-26  4:00 ` Zhan Xusheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox