Re: [PATCH v2 4/4] sched/fair: Rework/fix task_h_load()

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Kayra,

On 9/29/2026 11:16 PM, Kayra Cizmeci wrote:
> Hi Peter,
> 
> (Fun Stuff):
> 
>> @@ -15731,6 +15766,8 @@ static int __sched_group_set_shares(stru
>>  			update_load_avg(cfs_rq, se, UPDATE_TG);
>>  			update_cfs_group(se);
>>  		}
>> +		for_each_sched_entity_bl(se, cfs_rq)
>> +			update_cfs_rq_h_load(group_cfs_rq(se), se, cfs_rq);
>>  		rq_unlock_irqrestore(rq, &rf);
>>  	}
>>
> 
> So, the code is this:
> 	#define for_each_sched_entity(se, cfs_rq)					\
> 		for (struct sched_entity *_BL = NULL;					\
> 		     (se) && ((cfs_rq) = cfs_rq_of(se), (cfs_rq)->backlink = _BL, true);\
> 		     (se) = (se)->parent, _BL = (se))
> 	
> 	#define for_each_sched_entity_bl(se, cfs_rq) \
> 		for (; ((se) = (cfs_rq)->backlink); (cfs_rq) = group_cfs_rq(se))
> 
> 
> (While writing this, a suitcase tried to assassinate me by falling from top of the closet,
> what follows after this part may be the symptoms of my brain-damage.)
> 
> Let's say we have a *thing* like this:
> 
>   +----+    +----+    +------+
>   |se_a| -> |rq_a| -> |task_a|
>   +----+    +----+    +------+
>      |
>      |
>      |
>      V
>   +----+    +----+     +------+
>   |se_b| -> |rq_b|  -> |task_b|
>   +----+    +----+     +------+

I'm having a super hard time understanding this hierarchy.

Is it like:

    root
    /  \
   A    B
   |    |
 task   task

or something like:

  root
   |
   A
   |
   B
   |
  task 

?

> 
> When we start as task_b, everything goes well. Both groups are updated.
> On task_a too, only se_a is updated. 

I'm assuming the hierarchy is like the latter then if traversal from
B updates A.

> 
> But when we start as se_a root's backlink is NULL so we don't update anything.
> While on se_b rq_a's backlink is NULL and update se_a but not ourselfes.

So for that specific section you've highlighted from Peter's patch,
in __sched_group_set_shares(), we first do a:

    for_each_sched_entity(se) {
        update_load_avg(cfs_rq_of(se), se, UPDATE_TG);
        update_cfs_group(se);
    }

That sets up backlink going until se->parent whose group_cfs_rq() is
the cfs_rq of tg_se(B) aka the cfs_rq just above the group whose shares
were altered.

Then we do:

    for_each_sched_entity_bl(se, cfs_rq)
        update_cfs_rq_h_load(group_cfs_rq(se), se, cfs_rq);

Which updates the h_load all the way from the root until the cfs_rq of
the cgroup we altered.

In case of:

  root
   |
   A
   |
   B*

  *shares of cgroup is updated

If we update shares of B (aka tg_se(B)), we update the h_load
until the cfs_rq_of(tg_se(B)) which is till tg_cfs_rq(A).

Now if you have:

   root
    |
    A
   / \
 *B   C
      | \
      D  E

Yes,d you'll still update h_load for only A and you can have stale
h_load for C, D, and E, and for all the tasks queued below them.

Since full propagation is expensive, we do those propagation lazily
when the task is picked, enqueued, or dequeued

Note: We cannot propagate this up further because we have not yet done an
update_load_avg() for the cfa_rq(s) in rest of the hierarchy. Next reweight
will see the correct h_load starting from A and propagate it further when
needed.

Was that the problem you were talking about or did I totally confuse this
with something else?
> I don't think this is that of a problem tho, and
> I could be missing something.

I don't even see the problem. Maybe I need glasses :-)

-- 
Thanks and Regards,
Prateek