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
>