[PATCH] sched/fair: Fix flat hierarchy

[PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

When a fair task is enqueued, we must update curr and more precisely
its vruntime before placing the enqueued task so avg vruntime will take
into account the last exec phase.

Example:
TA is an always running task in cgroup G0.
TB is a short running task (cyclictest) in cgroup G1.
The lag of TB always increases up the clamp limit because TB is placed
before TA(curr) is updated (since the last tick). When curr(TA) is
finally updated, its last exec phase provide positive lag to TB

Because TA and TB don't belong to the same group, enqueue_hierarchy()
will not update TA's entity when updating curr but only G0's entity at
root level.

The same applies when dequeuing.

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---

I overlooked the cgroup part when I reviewed the update_entity_lag() in
flat hierarchy.

 kernel/sched/fair.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index dcf860c59a14..649b4f7505a1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
 		util_est_enqueue(cfs_rq, p);
 
+	if (cfs_rq->curr)
+		update_curr(cfs_rq_of(cfs_rq->curr));
+
 	if (flags & ENQUEUE_DELAYED) {
 		requeue_delayed_entity(cfs_rq, se);
 		return;
@@ -8103,7 +8106,8 @@ static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags)
 
 	clear_buddies(cfs_rq, se);
 
-	update_curr(cfs_rq_of(se));
+	if (cfs_rq->curr)
+		update_curr(cfs_rq_of(cfs_rq->curr));
 	update_entity_lag(cfs_rq, se);
 
 	if (flags & DEQUEUE_DELAYED) {
-- 
2.43.0

Re: [PATCH] sched/fair: Fix flat hierarchy

[Peter Zijlstra]

On Wed, Aug 12, 2026 at 02:50:39PM +0200, Vincent Guittot wrote:
> When a fair task is enqueued, we must update curr and more precisely
> its vruntime before placing the enqueued task so avg vruntime will take
> into account the last exec phase.
> 
> Example:
> TA is an always running task in cgroup G0.
> TB is a short running task (cyclictest) in cgroup G1.
> The lag of TB always increases up the clamp limit because TB is placed
> before TA(curr) is updated (since the last tick). When curr(TA) is
> finally updated, its last exec phase provide positive lag to TB
> 
> Because TA and TB don't belong to the same group, enqueue_hierarchy()
> will not update TA's entity when updating curr but only G0's entity at
> root level.
> 
> The same applies when dequeuing.

This doesn't quite make sense to me; on the one hand you talk about
vruntime (which is only relevant for rq->cfs) on the other hand you talk
about non overlapping cgroup hierarchies.

Hmm, update_curr() looks at ->h_curr, which is the intermediate crud. So
even though it updates all the cgroup nonsense, it will not in fact
update the root group, because it never actually sees rq->cfs.curr.

Bah.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index dcf860c59a14..649b4f7505a1 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
>  		util_est_enqueue(cfs_rq, p);
>  
> +	if (cfs_rq->curr)
> +		update_curr(cfs_rq_of(cfs_rq->curr));
> +

Still, I think this wants to be in a different spot. It needs to be
below the whole initial if(curr) place_entity() thing. Perhaps stick
these into {en,de}queue_hierarchy() ?


>  	if (flags & ENQUEUE_DELAYED) {
>  		requeue_delayed_entity(cfs_rq, se);
>  		return;
> @@ -8103,7 +8106,8 @@ static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags)
>  
>  	clear_buddies(cfs_rq, se);
>  
> -	update_curr(cfs_rq_of(se));
> +	if (cfs_rq->curr)
> +		update_curr(cfs_rq_of(cfs_rq->curr));
>  	update_entity_lag(cfs_rq, se);
>  
>  	if (flags & DEQUEUE_DELAYED) {
> -- 
> 2.43.0
> 

Re: [PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

On Wed, 12 Aug 2026 at 16:03, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Aug 12, 2026 at 02:50:39PM +0200, Vincent Guittot wrote:
> > When a fair task is enqueued, we must update curr and more precisely
> > its vruntime before placing the enqueued task so avg vruntime will take
> > into account the last exec phase.
> >
> > Example:
> > TA is an always running task in cgroup G0.
> > TB is a short running task (cyclictest) in cgroup G1.
> > The lag of TB always increases up the clamp limit because TB is placed
> > before TA(curr) is updated (since the last tick). When curr(TA) is
> > finally updated, its last exec phase provide positive lag to TB
> >
> > Because TA and TB don't belong to the same group, enqueue_hierarchy()
> > will not update TA's entity when updating curr but only G0's entity at
> > root level.
> >
> > The same applies when dequeuing.
>
> This doesn't quite make sense to me; on the one hand you talk about
> vruntime (which is only relevant for rq->cfs) on the other hand you talk
> about non overlapping cgroup hierarchies.
>
> Hmm, update_curr() looks at ->h_curr, which is the intermediate crud. So
> even though it updates all the cgroup nonsense, it will not in fact
> update the root group, because it never actually sees rq->cfs.curr.

Exactly

>
> Bah.
>
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index dcf860c59a14..649b4f7505a1 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >       if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
> >               util_est_enqueue(cfs_rq, p);
> >
> > +     if (cfs_rq->curr)
> > +             update_curr(cfs_rq_of(cfs_rq->curr));
> > +
>
> Still, I think this wants to be in a different spot. It needs to be
> below the whole initial if(curr) place_entity() thing. Perhaps stick
> these into {en,de}queue_hierarchy() ?

But are we sure that cfs_rq->curr has been updated ? Otherwise it
means that we place cfs_rq->curr before having updated its vruntime so
avg_vruntime will not account the last running phase.

The same applies when we requeue a delayed entity.

>
>
> >       if (flags & ENQUEUE_DELAYED) {
> >               requeue_delayed_entity(cfs_rq, se);
> >               return;
> > @@ -8103,7 +8106,8 @@ static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags)
> >
> >       clear_buddies(cfs_rq, se);
> >
> > -     update_curr(cfs_rq_of(se));
> > +     if (cfs_rq->curr)
> > +             update_curr(cfs_rq_of(cfs_rq->curr));
> >       update_entity_lag(cfs_rq, se);
> >
> >       if (flags & DEQUEUE_DELAYED) {
> > --
> > 2.43.0
> >

Re: [PATCH] sched/fair: Fix flat hierarchy

[Peter Zijlstra]

On Wed, Aug 12, 2026 at 04:21:14PM +0200, Vincent Guittot wrote:
> On Wed, 12 Aug 2026 at 16:03, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Aug 12, 2026 at 02:50:39PM +0200, Vincent Guittot wrote:
> > > When a fair task is enqueued, we must update curr and more precisely
> > > its vruntime before placing the enqueued task so avg vruntime will take
> > > into account the last exec phase.
> > >
> > > Example:
> > > TA is an always running task in cgroup G0.
> > > TB is a short running task (cyclictest) in cgroup G1.
> > > The lag of TB always increases up the clamp limit because TB is placed
> > > before TA(curr) is updated (since the last tick). When curr(TA) is
> > > finally updated, its last exec phase provide positive lag to TB
> > >
> > > Because TA and TB don't belong to the same group, enqueue_hierarchy()
> > > will not update TA's entity when updating curr but only G0's entity at
> > > root level.
> > >
> > > The same applies when dequeuing.
> >
> > This doesn't quite make sense to me; on the one hand you talk about
> > vruntime (which is only relevant for rq->cfs) on the other hand you talk
> > about non overlapping cgroup hierarchies.
> >
> > Hmm, update_curr() looks at ->h_curr, which is the intermediate crud. So
> > even though it updates all the cgroup nonsense, it will not in fact
> > update the root group, because it never actually sees rq->cfs.curr.
> 
> Exactly
> 
> >
> > Bah.
> >
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index dcf860c59a14..649b4f7505a1 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > >       if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
> > >               util_est_enqueue(cfs_rq, p);
> > >
> > > +     if (cfs_rq->curr)
> > > +             update_curr(cfs_rq_of(cfs_rq->curr));
> > > +
> >
> > Still, I think this wants to be in a different spot. It needs to be
> > below the whole initial if(curr) place_entity() thing. Perhaps stick
> > these into {en,de}queue_hierarchy() ?
> 
> But are we sure that cfs_rq->curr has been updated ? Otherwise it
> means that we place cfs_rq->curr before having updated its vruntime so
> avg_vruntime will not account the last running phase.
> 
> The same applies when we requeue a delayed entity.

Well, it is the same place the update was previously, no? Also, that XXX
comment thing is about enqueueing self, with the thinking that you
should not shift time since dequeue, allowing 'atomic' dequeue+enqueue.

However, looking at things now, I can't actually see how this could
happen in the new code. Putting a trace_printk() in also doesn't seem to
help much.

So perhaps there is some dead code there to clean up as well. But for
consistencies sake I think placing it near {en,de}queue_hierarchy()
where the other update_curr() lived/lives makes most sense.

Hmm?

Re: [PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

On Thu, 13 Aug 2026 at 12:31, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Aug 12, 2026 at 04:21:14PM +0200, Vincent Guittot wrote:
> > On Wed, 12 Aug 2026 at 16:03, Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Wed, Aug 12, 2026 at 02:50:39PM +0200, Vincent Guittot wrote:
> > > > When a fair task is enqueued, we must update curr and more precisely
> > > > its vruntime before placing the enqueued task so avg vruntime will take
> > > > into account the last exec phase.
> > > >
> > > > Example:
> > > > TA is an always running task in cgroup G0.
> > > > TB is a short running task (cyclictest) in cgroup G1.
> > > > The lag of TB always increases up the clamp limit because TB is placed
> > > > before TA(curr) is updated (since the last tick). When curr(TA) is
> > > > finally updated, its last exec phase provide positive lag to TB
> > > >
> > > > Because TA and TB don't belong to the same group, enqueue_hierarchy()
> > > > will not update TA's entity when updating curr but only G0's entity at
> > > > root level.
> > > >
> > > > The same applies when dequeuing.
> > >
> > > This doesn't quite make sense to me; on the one hand you talk about
> > > vruntime (which is only relevant for rq->cfs) on the other hand you talk
> > > about non overlapping cgroup hierarchies.
> > >
> > > Hmm, update_curr() looks at ->h_curr, which is the intermediate crud. So
> > > even though it updates all the cgroup nonsense, it will not in fact
> > > update the root group, because it never actually sees rq->cfs.curr.
> >
> > Exactly
> >
> > >
> > > Bah.
> > >
> > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > index dcf860c59a14..649b4f7505a1 100644
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > > >       if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
> > > >               util_est_enqueue(cfs_rq, p);
> > > >
> > > > +     if (cfs_rq->curr)
> > > > +             update_curr(cfs_rq_of(cfs_rq->curr));
> > > > +
> > >
> > > Still, I think this wants to be in a different spot. It needs to be
> > > below the whole initial if(curr) place_entity() thing. Perhaps stick
> > > these into {en,de}queue_hierarchy() ?
> >
> > But are we sure that cfs_rq->curr has been updated ? Otherwise it
> > means that we place cfs_rq->curr before having updated its vruntime so
> > avg_vruntime will not account the last running phase.
> >
> > The same applies when we requeue a delayed entity.
>
> Well, it is the same place the update was previously, no? Also, that XXX
> comment thing is about enqueueing self, with the thinking that you
> should not shift time since dequeue, allowing 'atomic' dequeue+enqueue.

I was looking at which use case could trigger  such situation.

But doesn't the below need the update
ttwu_runnable
  update_rq_clock
  p->is_blocked is true and p->se.sched_delayed is true -->
enqueue_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_DELAYED);

>
> However, looking at things now, I can't actually see how this could
> happen in the new code. Putting a trace_printk() in also doesn't seem to
> help much.

I'm also running some tests w/ and w/o this patch and checking if
rq->cfs->curr->exec_start != rq_clock_task(rq) when we place an entity
or compute a lag.

>
> So perhaps there is some dead code there to clean up as well. But for
> consistencies sake I think placing it near {en,de}queue_hierarchy()
> where the other update_curr() lived/lives makes most sense.

I will run some test with the check above and the update_curr at the
beg of {en,de}queue_hierarchy()

>
> Hmm?

Re: [PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

On Thu, 13 Aug 2026 at 12:49, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Thu, 13 Aug 2026 at 12:31, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Aug 12, 2026 at 04:21:14PM +0200, Vincent Guittot wrote:
> > > On Wed, 12 Aug 2026 at 16:03, Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Wed, Aug 12, 2026 at 02:50:39PM +0200, Vincent Guittot wrote:
> > > > > When a fair task is enqueued, we must update curr and more precisely
> > > > > its vruntime before placing the enqueued task so avg vruntime will take
> > > > > into account the last exec phase.
> > > > >
> > > > > Example:
> > > > > TA is an always running task in cgroup G0.
> > > > > TB is a short running task (cyclictest) in cgroup G1.
> > > > > The lag of TB always increases up the clamp limit because TB is placed
> > > > > before TA(curr) is updated (since the last tick). When curr(TA) is
> > > > > finally updated, its last exec phase provide positive lag to TB
> > > > >
> > > > > Because TA and TB don't belong to the same group, enqueue_hierarchy()
> > > > > will not update TA's entity when updating curr but only G0's entity at
> > > > > root level.
> > > > >
> > > > > The same applies when dequeuing.
> > > >
> > > > This doesn't quite make sense to me; on the one hand you talk about
> > > > vruntime (which is only relevant for rq->cfs) on the other hand you talk
> > > > about non overlapping cgroup hierarchies.
> > > >
> > > > Hmm, update_curr() looks at ->h_curr, which is the intermediate crud. So
> > > > even though it updates all the cgroup nonsense, it will not in fact
> > > > update the root group, because it never actually sees rq->cfs.curr.
> > >
> > > Exactly
> > >
> > > >
> > > > Bah.
> > > >
> > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > > index dcf860c59a14..649b4f7505a1 100644
> > > > > --- a/kernel/sched/fair.c
> > > > > +++ b/kernel/sched/fair.c
> > > > > @@ -7983,6 +7983,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > > > >       if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
> > > > >               util_est_enqueue(cfs_rq, p);
> > > > >
> > > > > +     if (cfs_rq->curr)
> > > > > +             update_curr(cfs_rq_of(cfs_rq->curr));
> > > > > +
> > > >
> > > > Still, I think this wants to be in a different spot. It needs to be
> > > > below the whole initial if(curr) place_entity() thing. Perhaps stick
> > > > these into {en,de}queue_hierarchy() ?
> > >
> > > But are we sure that cfs_rq->curr has been updated ? Otherwise it
> > > means that we place cfs_rq->curr before having updated its vruntime so
> > > avg_vruntime will not account the last running phase.
> > >
> > > The same applies when we requeue a delayed entity.
> >
> > Well, it is the same place the update was previously, no? Also, that XXX
> > comment thing is about enqueueing self, with the thinking that you
> > should not shift time since dequeue, allowing 'atomic' dequeue+enqueue.
>
> I was looking at which use case could trigger  such situation.
>
> But doesn't the below need the update
> ttwu_runnable
>   update_rq_clock
>   p->is_blocked is true and p->se.sched_delayed is true -->
> enqueue_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_DELAYED);
>
> >
> > However, looking at things now, I can't actually see how this could
> > happen in the new code. Putting a trace_printk() in also doesn't seem to
> > help much.
>
> I'm also running some tests w/ and w/o this patch and checking if
> rq->cfs->curr->exec_start != rq_clock_task(rq) when we place an entity
> or compute a lag.
>
> >
> > So perhaps there is some dead code there to clean up as well. But for
> > consistencies sake I think placing it near {en,de}queue_hierarchy()
> > where the other update_curr() lived/lives makes most sense.
>
> I will run some test with the check above and the update_curr at the
> beg of {en,de}queue_hierarchy()

For the enqueue, the 1st requeue_delayed_entity() which is the main
path for delayed task, requires the update. Then another one before
reweight_eevdf(cfs_rq, se, weight, false); in the !curr case.

But I'm not sure we want to spread this in different places.

For the dequeue, update_entity_lag() requires to call the update 1st

I also wanted to use the helper below:

+/* Update curr's vruntime before placing entity or updating lag */
+static inline update_curr_eevdf(struct cfs_rq *cfs_rq)
+{
+       if (!cfs_rq->curr)
+               return;
+
+       update_curr(cfs_rq_of(cfs_rq->curr));
+}

 >
> >
> > Hmm?

Re: [PATCH] sched/fair: Fix flat hierarchy

[Peter Zijlstra]

On Fri, Aug 14, 2026 at 11:26:13AM +0200, Vincent Guittot wrote:

> For the enqueue, the 1st requeue_delayed_entity() which is the main
> path for delayed task, requires the update. Then another one before
> reweight_eevdf(cfs_rq, se, weight, false); in the !curr case.
> 
> But I'm not sure we want to spread this in different places.

Fair enough. I'll test the below and then push to sched/urgent.

---
Subject: sched/fair: Fix flat hierarchy
From: Vincent Guittot <vincent.guittot@linaro.org>
Date: Wed, 12 Aug 2026 14:50:39 +0200

From: Vincent Guittot <vincent.guittot@linaro.org>

When a fair task is enqueued, we must update curr and more precisely
its vruntime before placing the enqueued task so avg vruntime will take
into account the last exec phase.

Example:
TA is an always running task in cgroup G0.
TB is a short running task (cyclictest) in cgroup G1.
The lag of TB always increases up the clamp limit because TB is placed
before TA(curr) is updated (since the last tick). When curr(TA) is
finally updated, its last exec phase provide positive lag to TB

Because TA and TB don't belong to the same group, enqueue_hierarchy() will not
update TA's entity when updating curr but only G0's entity at root level.

The same applies when dequeuing.

This is because update_curr() uses ->h_curr, rather than ->curr, and therefore,
while it is invoked on the root cfs_rq, which contains all the eevdf bits, it
does not do the right thing.

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20260812125039.1717249-1-vincent.guittot@linaro.org
---
 kernel/sched/fair.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7958,6 +7958,15 @@ static unsigned long enqueue_hierarchy(s
 	return weight;
 }
 
+/* Update curr's vruntime before placing entity or updating lag */
+static inline void update_curr_eevdf(struct cfs_rq *cfs_rq)
+{
+	if (!cfs_rq->curr)
+		return;
+
+	update_curr(cfs_rq_of(cfs_rq->curr));
+}
+
 /*
  * The enqueue_task method is called before nr_running is
  * increased. Here we update the fair scheduling stats and
@@ -7985,6 +7994,8 @@ enqueue_task_fair(struct rq *rq, struct
 	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
 		util_est_enqueue(cfs_rq, p);
 
+	update_curr_eevdf(cfs_rq);
+
 	if (flags & ENQUEUE_DELAYED) {
 		requeue_delayed_entity(cfs_rq, se);
 		return;
@@ -8105,7 +8116,7 @@ static bool __dequeue_task(struct rq *rq
 
 	clear_buddies(cfs_rq, se);
 
-	update_curr(cfs_rq_of(se));
+	update_curr_eevdf(cfs_rq);
 	update_entity_lag(cfs_rq, se);
 
 	if (flags & DEQUEUE_DELAYED) {

Re: [PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

On Fri, 14 Aug 2026 at 13:22, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Aug 14, 2026 at 11:26:13AM +0200, Vincent Guittot wrote:
>
> > For the enqueue, the 1st requeue_delayed_entity() which is the main
> > path for delayed task, requires the update. Then another one before
> > reweight_eevdf(cfs_rq, se, weight, false); in the !curr case.
> >
> > But I'm not sure we want to spread this in different places.
>
> Fair enough. I'll test the below and then push to sched/urgent.

I tested this patch on sched/core.

That being said, 7.2 wants something like below for requeued_delayed_entity()

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index fd3edf72fb6e..3defda0e2f83 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7802,6 +7802,8 @@ requeue_delayed_entity(struct sched_entity *se)
        WARN_ON_ONCE(!se->sched_delayed);
        WARN_ON_ONCE(!se->on_rq);

+       update_curr(cfs_rq);
+
        if (update_entity_lag(cfs_rq, se)) {
                cfs_rq->nr_queued--;
                if (se != cfs_rq->curr)

So I would put the cahnge above in sched/urgent and the patch
discussed here in sched/core


>
> ---
> Subject: sched/fair: Fix flat hierarchy
> From: Vincent Guittot <vincent.guittot@linaro.org>
> Date: Wed, 12 Aug 2026 14:50:39 +0200
>
> From: Vincent Guittot <vincent.guittot@linaro.org>
>
> When a fair task is enqueued, we must update curr and more precisely
> its vruntime before placing the enqueued task so avg vruntime will take
> into account the last exec phase.
>
> Example:
> TA is an always running task in cgroup G0.
> TB is a short running task (cyclictest) in cgroup G1.
> The lag of TB always increases up the clamp limit because TB is placed
> before TA(curr) is updated (since the last tick). When curr(TA) is
> finally updated, its last exec phase provide positive lag to TB
>
> Because TA and TB don't belong to the same group, enqueue_hierarchy() will not
> update TA's entity when updating curr but only G0's entity at root level.
>
> The same applies when dequeuing.
>
> This is because update_curr() uses ->h_curr, rather than ->curr, and therefore,
> while it is invoked on the root cfs_rq, which contains all the eevdf bits, it
> does not do the right thing.
>
> Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://patch.msgid.link/20260812125039.1717249-1-vincent.guittot@linaro.org
> ---
>  kernel/sched/fair.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7958,6 +7958,15 @@ static unsigned long enqueue_hierarchy(s
>         return weight;
>  }
>
> +/* Update curr's vruntime before placing entity or updating lag */
> +static inline void update_curr_eevdf(struct cfs_rq *cfs_rq)
> +{
> +       if (!cfs_rq->curr)
> +               return;
> +
> +       update_curr(cfs_rq_of(cfs_rq->curr));
> +}
> +
>  /*
>   * The enqueue_task method is called before nr_running is
>   * increased. Here we update the fair scheduling stats and
> @@ -7985,6 +7994,8 @@ enqueue_task_fair(struct rq *rq, struct
>         if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
>                 util_est_enqueue(cfs_rq, p);
>
> +       update_curr_eevdf(cfs_rq);
> +
>         if (flags & ENQUEUE_DELAYED) {
>                 requeue_delayed_entity(cfs_rq, se);
>                 return;
> @@ -8105,7 +8116,7 @@ static bool __dequeue_task(struct rq *rq
>
>         clear_buddies(cfs_rq, se);
>
> -       update_curr(cfs_rq_of(se));
> +       update_curr_eevdf(cfs_rq);
>         update_entity_lag(cfs_rq, se);
>
>         if (flags & DEQUEUE_DELAYED) {

Re: [PATCH] sched/fair: Fix flat hierarchy

[Peter Zijlstra]

On Fri, Aug 14, 2026 at 02:51:20PM +0200, Vincent Guittot wrote:
> On Fri, 14 Aug 2026 at 13:22, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Aug 14, 2026 at 11:26:13AM +0200, Vincent Guittot wrote:
> >
> > > For the enqueue, the 1st requeue_delayed_entity() which is the main
> > > path for delayed task, requires the update. Then another one before
> > > reweight_eevdf(cfs_rq, se, weight, false); in the !curr case.
> > >
> > > But I'm not sure we want to spread this in different places.
> >
> > Fair enough. I'll test the below and then push to sched/urgent.
> 
> I tested this patch on sched/core.
> 
> That being said, 7.2 wants something like below for requeued_delayed_entity()
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index fd3edf72fb6e..3defda0e2f83 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7802,6 +7802,8 @@ requeue_delayed_entity(struct sched_entity *se)
>         WARN_ON_ONCE(!se->sched_delayed);
>         WARN_ON_ONCE(!se->on_rq);
> 
> +       update_curr(cfs_rq);
> +
>         if (update_entity_lag(cfs_rq, se)) {
>                 cfs_rq->nr_queued--;
>                 if (se != cfs_rq->curr)
> 
> So I would put the cahnge above in sched/urgent and the patch
> discussed here in sched/core

Right. I've pushed out sched/urgent and sched/core to queue.git. If you
could double check that, I'll push them out to tip later today.

For now, I'm going to go melt someplace outside for a bit.

Re: [PATCH] sched/fair: Fix flat hierarchy

[Vincent Guittot]

On Fri, 14 Aug 2026 at 16:16, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Aug 14, 2026 at 02:51:20PM +0200, Vincent Guittot wrote:
> > On Fri, 14 Aug 2026 at 13:22, Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Fri, Aug 14, 2026 at 11:26:13AM +0200, Vincent Guittot wrote:
> > >
> > > > For the enqueue, the 1st requeue_delayed_entity() which is the main
> > > > path for delayed task, requires the update. Then another one before
> > > > reweight_eevdf(cfs_rq, se, weight, false); in the !curr case.
> > > >
> > > > But I'm not sure we want to spread this in different places.
> > >
> > > Fair enough. I'll test the below and then push to sched/urgent.
> >
> > I tested this patch on sched/core.
> >
> > That being said, 7.2 wants something like below for requeued_delayed_entity()
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index fd3edf72fb6e..3defda0e2f83 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7802,6 +7802,8 @@ requeue_delayed_entity(struct sched_entity *se)
> >         WARN_ON_ONCE(!se->sched_delayed);
> >         WARN_ON_ONCE(!se->on_rq);
> >
> > +       update_curr(cfs_rq);
> > +
> >         if (update_entity_lag(cfs_rq, se)) {
> >                 cfs_rq->nr_queued--;
> >                 if (se != cfs_rq->curr)
> >
> > So I would put the cahnge above in sched/urgent and the patch
> > discussed here in sched/core
>
> Right. I've pushed out sched/urgent and sched/core to queue.git. If you
> could double check that, I'll push them out to tip later today.

Both look good to me. Thanks

>
> For now, I'm going to go melt someplace outside for a bit.

Enjoy :-)

[tip: sched/core] sched/fair: Fix flat hierarchy

[tip-bot2 for Vincent Guittot]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     68e37487810a3da43c48340fab7a55b3b6efdae3
Gitweb:        https://git.kernel.org/tip/68e37487810a3da43c48340fab7a55b3b6efdae3
Author:        Vincent Guittot <vincent.guittot@linaro.org>
AuthorDate:    Wed, 12 Aug 2026 14:50:39 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Aug 2026 16:13:02 +02:00

sched/fair: Fix flat hierarchy

When a fair task is enqueued, we must update curr and more precisely
its vruntime before placing the enqueued task so avg vruntime will take
into account the last exec phase.

Example:
TA is an always running task in cgroup G0.
TB is a short running task (cyclictest) in cgroup G1.
The lag of TB always increases up the clamp limit because TB is placed
before TA(curr) is updated (since the last tick). When curr(TA) is
finally updated, its last exec phase provide positive lag to TB

Because TA and TB don't belong to the same group, enqueue_hierarchy() will not
update TA's entity when updating curr but only G0's entity at root level.

The same applies when dequeuing.

This is because update_curr() uses ->h_curr, rather than ->curr, and therefore,
while it is invoked on the root cfs_rq, which contains all the eevdf bits, it
does not do the right thing.

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20260812125039.1717249-1-vincent.guittot@linaro.org
---
 kernel/sched/fair.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8a8247b..f79fcba 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7903,8 +7903,6 @@ requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	WARN_ON_ONCE(!se->sched_delayed);
 	WARN_ON_ONCE(!se->on_rq);
 
-	update_curr(cfs_rq);
-
 	if (update_entity_lag(cfs_rq, se)) {
 		cfs_rq->h_nr_queued--;
 		if (se != cfs_rq->curr)
@@ -7958,6 +7956,15 @@ static unsigned long enqueue_hierarchy(struct task_struct *p, int flags)
 	return weight;
 }
 
+/* Update curr's vruntime before placing entity or updating lag */
+static inline void update_curr_eevdf(struct cfs_rq *cfs_rq)
+{
+	if (!cfs_rq->curr)
+		return;
+
+	update_curr(cfs_rq_of(cfs_rq->curr));
+}
+
 /*
  * The enqueue_task method is called before nr_running is
  * increased. Here we update the fair scheduling stats and
@@ -7985,6 +7992,8 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
 		util_est_enqueue(cfs_rq, p);
 
+	update_curr_eevdf(cfs_rq);
+
 	if (flags & ENQUEUE_DELAYED) {
 		requeue_delayed_entity(cfs_rq, se);
 		return;
@@ -8105,7 +8114,7 @@ static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags)
 
 	clear_buddies(cfs_rq, se);
 
-	update_curr(cfs_rq_of(se));
+	update_curr_eevdf(cfs_rq);
 	update_entity_lag(cfs_rq, se);
 
 	if (flags & DEQUEUE_DELAYED) {