Re: [PATCH v3 7/7] sched/eevdf: Move to a single runqueue
From: Vincent Guittot
Date: Mon Oct 19 2026 - 13:55:27 EST
Le dimanche 18 oct. 2026 à 08:49:29 (+0200), Mike Galbraith a écrit :
> Greetings!
>
> On Fri, 2026-10-16 at 15:18 +0200, Jakub Okoński wrote:
> > Hi Peter,
> >
> > On 6/5/26 14:40, Peter Zijlstra wrote:
> >
> > > Change fair/cgroup to a single runqueue.
> > >
> > > [...]
> > >
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -9916,7 +9837,6 @@ static void wakeup_preempt_fair(struct r
> > > if (!sched_feat(WAKEUP_PREEMPTION))
> > > return;
> > >
> > > - find_matching_se(&se, &pse);
> > > WARN_ON_ONCE(!pse);
> > >
> > > cse_is_idle = se_is_idle(se);
> >
> > I think this changes how cpu.idle affects wakeup preemption.
>
> My box claims this to be unrelated to the jitter you're seeing, as it
> is present in 7.2.11, with and without the sched-flat set applied.
Yes, we don't take into account whether the task group is sched_idle
Jakub, Mike,
Could you try the patch below ? It solves the problem on my setup
---
kernel/sched/fair.c | 80 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 12 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0f2364479f9d..d17e88ad12a5 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -433,6 +433,38 @@ static inline struct sched_entity *parent_entity(const struct sched_entity *se)
return se->parent;
}
+static void
+find_matching_se(struct sched_entity **se, struct sched_entity **pse)
+{
+ int se_depth, pse_depth;
+
+ /*
+ * preemption test can be made between sibling entities who are in the
+ * same cfs_rq i.e who have a common parent. Walk up the hierarchy of
+ * both tasks until we find their ancestors who are siblings of common
+ * parent.
+ */
+
+ /* First walk up until both entities are at same depth */
+ se_depth = (*se)->depth;
+ pse_depth = (*pse)->depth;
+
+ while (se_depth > pse_depth) {
+ se_depth--;
+ *se = parent_entity(*se);
+ }
+
+ while (pse_depth > se_depth) {
+ pse_depth--;
+ *pse = parent_entity(*pse);
+ }
+
+ while (!is_same_group(*se, *pse)) {
+ *se = parent_entity(*se);
+ *pse = parent_entity(*pse);
+ }
+}
+
static int tg_is_idle(struct task_group *tg)
{
return tg->idle > 0;
@@ -476,6 +508,11 @@ static inline struct sched_entity *parent_entity(struct sched_entity *se)
return NULL;
}
+static inline void
+find_matching_se(struct sched_entity **se, struct sched_entity **pse)
+{
+}
+
static inline int tg_is_idle(struct task_group *tg)
{
return 0;
@@ -10129,7 +10166,34 @@ preempt_sync(struct rq *rq, int wake_flags,
}
/*
- * Preempt the current task with a newly woken task if needed:
+ * Preempt an idle entity in favor of a non-idle entity (and don't preempt
+ * in the inverse case).
+ */
+static bool wakeup_preempt_sched_idle(struct sched_entity *cse, struct sched_entity *pse)
+{
+ int cse_is_idle, pse_is_idle;
+
+ find_matching_se(&cse, &pse);
+ WARN_ON_ONCE(!pse);
+
+ cse_is_idle = se_is_idle(cse);
+ pse_is_idle = se_is_idle(pse);
+
+ /*
+ * Preempt an idle entity in favor of a non-idle entity (and don't preempt
+ * in the inverse case).
+ */
+ if (cse_is_idle && !pse_is_idle)
+ return PREEMPT_WAKEUP_RESCHED;
+
+ if (cse_is_idle != pse_is_idle)
+ return PREEMPT_WAKEUP_NONE;
+
+ return PREEMPT_WAKEUP_PICK;
+}
+
+/*
+ * Preempt the current task with a newly woken task if needed.
*/
static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_flags)
{
@@ -10174,22 +10238,14 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
if (!sched_feat(WAKEUP_PREEMPTION))
return;
- WARN_ON_ONCE(!pse);
-
- cse_is_idle = se_is_idle(se);
- pse_is_idle = se_is_idle(pse);
+ preempt_action = wakeup_preempt_sched_idle(se, pse);
- nse = se;
- /*
- * Preempt an idle entity in favor of a non-idle entity (and don't preempt
- * in the inverse case).
- */
- if (cse_is_idle && !pse_is_idle)
+ if (preempt_action == PREEMPT_WAKEUP_RESCHED)
goto preempt;
update_curr_fair(rq);
- if (cse_is_idle != pse_is_idle)
+ if (preempt_action == PREEMPT_WAKEUP_NONE)
goto update;
/*
--
2.53.0
>
> > With the single runqueue, find_matching_se() is gone and se_is_idle()
> > reduces to task_has_idle_policy(). Wouldn't this mean that SCHED_NORMAL
> > tasks in a cpu.idle cgroup are no longer treated as idle for
> > wakeup_preempt_fair()?
> >
> >
> > I tested my existing 7.2.9 desktop against 7.3-rc6 with the setup below.
> > cyclictest doesn't reproduce it because it just sleeps and wakes, no CPU
> > burn and therefore its accrued EEVDF lag keeps the latency low?
> >
> > cc -O2 -o wakelat wakelat.c
> > systemd-run --user --collect --unit=spin -p AllowedCPUs=7 -p CPUWeight=idle sh -c 'while :; do :; done'
> > systemd-run --user --collect --wait --pipe --same-dir -p AllowedCPUs=7 ./wakelat 1000 15 200 |
> > mlr --inidx --ifs space --opprint --barred --right label lat \
> > then stats1 -a count,p50,p90,p99,max -f lat then rename -r '^lat_,' \
> > then put -q 'emit mapsum({"kernel": system("uname -r")}, $*)'
> > systemctl --user stop spin
> >
> > wakelat wakes on a fixed 1 ms schedule, records how late each wakeup
> > was, then busy-loops for 200 micros (wall clock).
> >
> > +-----------+-------+-----+-----+------+------+
> > > kernel | count | p50 | p90 | p99 | max |
> > +-----------+-------+-----+-----+------+------+
> > > 7.2.9 | 14998 | 11 | 11 | 12 | 1869 |
> > > 7.3.0-rc6 | 13853 | 11 | 12 | 2029 | 2116 |
> > +-----------+-------+-----+-----+------+------+
> >
> > count is out of 15000 periods. rc6 had 1147 wakeups delayed by over 1 ms.
>
> Thanks a bunch for the proggy. I've been looking for a clear signal
> with various loads, and stuffing wakelat+awk into one of my scripts
> provided a perfectly clear signal, which I bisected and confirmed.
>
> Note: below is box pointing it's little silicon finger at the source of
> aforementioned jitter, not necessarily at breakage.
>
> Reverting the fingered commit removed the jitter from 7.2.11, and force
> reverting 68e37487810a + 333238da9a19 removed it from master as well.
>
> 333238da9a193ffc58792995f3e951e4cb87bfd2 is the first 'bad' commit
> commit 333238da9a193ffc58792995f3e951e4cb87bfd2
> Author: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> Date: Fri Aug 14 15:52:41 2026 +0200
>
> sched: Update time before requeueing delayed entities
>
> In order to compute the right lag, it is required to update time to 'now'.
> Without this, the delayed entity might appear younger than it really is and
> receive less compensation for having waited.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
>
> kernel/sched/fair.c | 2 ++
> 1 file changed, 2 insertions(+)
>