Re: [PATCH] sched/fair: Call update_util_est() after dequeue_entities()

[Tim Chen <tim.c.chen@linux.intel.com>]

On Tue, 2026-05-12 at 13:46 +0100, Qais Yousef wrote:
> update_util_est() reads task_util() at dequeue which is updated in
> dequeue_entities(). To read the accurate util_avg at dequeue, make sure
> to do the read after load_avg is updated in dequeue_entities().
> 
> util_est for a periodic task before
> 
>                                 periodic-3114 util_est.enqueued running
>    ┌───────────────────────────────────────────────────────────────────────────────────────────────┐
> 183┤                ▖▗  ▐▖         ▖ ▗▙   ▗   ▗▙▖▖       ▖▖   ▖       ▖▖        ▗  ▟  ▗▄▖          │
> 139┤               ▐▛█▜▙▞▀▄▄▞▚▄▟█▞▙█▄▟▀▚▄▄▞▚▄▄▟▀▀▛▄▝▄▄▄▙█▛▛█▛▜▛▄▄▀▄█▙▛▛▛▙▄▀▄▄▖▜▄▟█▟▀▜▟▄▜▀▄▄▟▙▖     │
>  95┤              ▐▀    ▘   ▝   ▝        ▝▘        ▘   ▘▘       ▝▘       ▝▘  ▝    ▝        ▀       │
>    │              ▛                                                                                │
>  51┤             ▐▘                                                                                │
>   7┤      ▖▗▗  ▗▄▐                                                                                 │
>    └┬─────────┬──────────┬─────────┬──────────┬─────────┬──────────┬─────────┬──────────┬─────────┬┘
>   0.00      0.65       1.30      1.96       2.61      3.26       3.91      4.57       5.22     5.87
> 
> and after
> 
>                                  periodic-2977 util_est.enqueued running
>      ┌─────────────────────────────────────────────────────────────────────────────────────────────┐
> 157.0┤               ▙▄ ▗▄  ▗▄▄▄ ▗▄  ▗▄▄▄▗▄▄  ▗▄▄▖ ▄   ▄▄▄   ▄  ▄▖▖  ▄▄▄▄▄▖▖▝▙▄▄▄▄▄▄▖ ▗▄           │
> 119.5┤             ▗▄▌▘▀▀ ▀▀▀ ▝▀▀▘▝▀▀▀ ▝▀▘ ▝▀▀▘ ▀▝▀▘▀▀▀▘▝▀▀▀▀▀▀▀▘▝▝▀▀ ▀   ▝▝▀  ▀   ▀▀▀▀            │
>  82.0┤             ▟                                                                               │
>      │             ▌                                                                               │
>  44.5┤             ▌                                                                               │
>   7.0┤      ▗   ▗▖ ▌                                                                               │
>      └┬─────────┬─────────┬──────────┬─────────┬─────────┬─────────┬──────────┬─────────┬─────────┬┘
>     0.00      0.65      1.30       1.95      2.60      3.25      3.90       4.56      5.21     5.86
> 
> Note how the signal is noisier and can peak to 183 vs 157 now.
> 
> Fixes: b55945c500c5 ("sched: Fix pick_next_task_fair() vs try_to_wake_up() race")
> Signed-off-by: Qais Yousef <qyousef@layalina.io>
> ---
> 
> This is split from [1] series where I stumbled upon this problem. AFAICS it
> needs backporting all the way to 6.12 LTS.
> 
> [1] https://lore.kernel.org/lkml/20260504020003.71306-1-qyousef@layalina.io/
> 
>  kernel/sched/fair.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 728965851842..96ba97e5f4ae 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7401,6 +7401,8 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
>   */
>  static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  {
> +	int ret;
> +
>  	if (task_is_throttled(p)) {
>  		dequeue_throttled_task(p, flags);
>  		return true;
> @@ -7409,8 +7411,9 @@ static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	if (!p->se.sched_delayed)
>  		util_est_dequeue(&rq->cfs, p);
>  
> +	ret = dequeue_entities(rq, &p->se, flags);
>  	util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);

I thought that util_est_update() was called intentionally before dequeue_entities
to update the utilization of task p up to this time right
before the dequeue.  Then dequeue_entities() is called later
with up to date task utilization estimate of p.

Perhaps util_est_update() should be moved before
util_est_dequeue() so the updated utilization of p
is subtracted from the rq utilization.

@@ -8002,10 +8002,10 @@ static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
                return true;
        }
 
+       util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);
        if (!p->se.sched_delayed)
                util_est_dequeue(&rq->cfs, p);
 
-       util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);
        if (dequeue_entities(rq, &p->se, flags) < 0)
                return false;


Tim

> -	if (dequeue_entities(rq, &p->se, flags) < 0)
> +	if (ret < 0)
>  		return false;
>  
>  	/*