Re: [RFC PATCH 03/16] sched/fair: Use enqueue flags for DO_ATTACH in update_load_avg()

[Andrea Righi <arighi@nvidia.com>]

Hi Prateek,

On Wed, Aug 26, 2026 at 06:28:47AM +0000, K Prateek Nayak wrote:
> proxy_set_cpu() skips calling ->migrate_task_rq() callback for blocked
> donor migrations and can lead to incorrect PELT accounting.
> 
> enqueue_task() -> update_load_avg() depends on migrate_task_rq_fair() to
> clear the p->se.avg.last_update_time and use this as a gating condition
> to do DO_ATTACH which is skipped for blocked donors.
> 
> Use migrate instead of se->avg.last_update_time indicator for adding
> DO_ATTACH action. Add a defensive WARN_ON_ONCE() to catch any cases that
> are not covered by the MIGRATING | MIGRATED | INITIAL flags.
> 
> Fixes: b049b81bdff6 ("sched: Handle blocked-waiter migration (and return migration)")
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
>  kernel/sched/fair.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 244f1c68c041..ed397c259509 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5654,7 +5654,7 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
>  	decayed  = update_cfs_rq_load_avg(now, cfs_rq);
>  	decayed |= propagate_entity_load_avg(se);
>  
> -	if (!se->avg.last_update_time && (flags & DO_ATTACH)) {
> +	if (flags & DO_ATTACH) {
>  
>  		/*
>  		 * DO_ATTACH means we're here from enqueue_entity().

nit: the comment below still says:

   * !last_update_time means we've passed through
   * migrate_task_rq_fair() indicating we migrated.

We should probably update it, perhaps:

    /*
     * DO_ATTACH means the entity needs to be attached to this
     * cfs_rq's load-tracking sums, either on initial enqueue
     * or after migration.
     */

or something along these lines.

> @@ -6110,6 +6110,7 @@ static void
>  enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  {
>  	bool curr = cfs_rq->curr == se;
> +	int action = UPDATE_TG;
>  
>  	/*
>  	 * If we're the current task, we must renormalise before calling
> @@ -6120,6 +6121,11 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  
>  	update_curr(cfs_rq);
>  
> +	if (flags & (ENQUEUE_MIGRATING | ENQUEUE_MIGRATED | ENQUEUE_INITIAL))
> +		action |= DO_ATTACH;
> +	else
> +		WARN_ON_ONCE(!se->avg.last_update_time);
> +
>  	/*
>  	 * When enqueuing a sched_entity, we must:
>  	 *   - Update loads to have both entity and cfs_rq synced with now.
> @@ -6129,7 +6135,7 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  	 *     its group cfs_rq
>  	 *   - Add its new weight to cfs_rq->load.weight
>  	 */
> -	update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH);
> +	update_load_avg(cfs_rq, se, action);
>  	se_update_runnable(se);
>  	/*
>  	 * XXX update_load_avg() above will have attached us to the pelt sum;
> -- 
> 2.34.1
> 

Thanks,
-Andrea