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

[Peter Zijlstra]

On Wed, Sep 30, 2026 at 01:20:43PM +0300, Kayra Cizmeci wrote:
> Hi Peter,
> 
> I checked everything *again* to make sure;
> 
> > -	if (!se) {
> > -		cfs_rq->h_load = cfs_rq_load_avg(cfs_rq);
> > -		cfs_rq->last_h_load_update = now;
> > -	}
> > +	/*
> > +	 * The above (forward) leaf_cfs_rq_list traversal will have done
> > +	 * update_cfs_rq_load_avg() in a bottom-up fashion. Now iterate the
> > +	 * list backwards, such that we're ensured to have visited every
> > +	 * parent of the current group to update h_load in a top-down fashion.
> > +	 */
> > +	list_for_each_entry_reverse(cfs_rq, &rq->leaf_cfs_rq_list, leaf_cfs_rq_list)
> > +		update_cfs_rq_h_load(cfs_rq, NULL, NULL);
> 
> 
> >  static unsigned long task_h_load(struct task_struct *p)
> >  {
> >  	struct cfs_rq *cfs_rq = task_cfs_rq(p);
> >  
> > -	update_cfs_rq_h_load(cfs_rq);
> > -	return div64_ul(p->se.avg.load_avg * cfs_rq->h_load,
> > +	return div64_ul(p->se.avg.load_avg * READ_ONCE(cfs_rq->h_load),
> >  			cfs_rq_load_avg(cfs_rq) + 1);
> >  }
> 
> Is updating the root less often intentional? 
> Or am I missing something and we update the root more than the old code?

You're referring to the removal of update_cfs_rq_h_load() here? That was
the whole purpose of the patch. The callers of task_h_load() do not (in
general) hold rq->lock, and thus update_cfs_rq_h_load() is unserialized
and broken.

> Also, there's a typo:
> 
> > + * Pelt uses apprixmate 'us' as ns/1024; and then uses time segments of 1024
> > + * 'us'. As a result each segment is in fact '1<<20' ns.
> 
> 'apprixmate' :-).

Some day I might learn to type :-)