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

[Vincent Guittot]

On Fri, 14 Aug 2026 at 13:22, Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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@xxxxxxxxxx>
> Date: Wed, 12 Aug 2026 14:50:39 +0200
>
> From: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Link: https://patch.msgid.link/20260812125039.1717249-1-vincent.guittot@xxxxxxxxxx
> ---
>  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) {