Re: [PATCH 2/3] sched/fair: Make cfs_rq::idle the sum of ancestor tg::idle

[Kayra Cizmeci <kayracizmeci@gmail.com>]

On Fri, Nov 06, 2026 at 09:53:20AM +0100, Peter Zijlstra wrote:
> On Fri, Nov 06, 2026 at 09:31:06AM +0100, Peter Zijlstra wrote:
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -15594,8 +15594,10 @@ static void set_next_task_fair(struct rq
> > > >  		hrtick_start_fair(rq, p);
> > > >  }
> > > >  
> > > > -void init_cfs_rq(struct cfs_rq *cfs_rq)
> > > > +void init_cfs_rq(struct cfs_rq *cfs_rq, struct cfs_rq *parent)
> > > >  {
> > > > +	if (parent)
> > > > +		cfs_rq->idle = parent->idle;
> > > 
> > > AFAICT this creates a race:
> > 
> > > Or am I missing something?
> > 
> > Nah, you're right, this needs rq->lock.
> 
> Something like so..
> 
> ---
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8970,7 +8970,7 @@ void __init sched_init(void)
>  		rq->nr_running = 0;
>  		rq->calc_load_active = 0;
>  		rq->calc_load_update = jiffies + LOAD_FREQ;
> -		init_cfs_rq(&rq->cfs, NULL);
> +		init_cfs_rq(&rq->cfs);
>  		init_rt_rq(&rq->rt);
>  		init_dl_rq(&rq->dl);
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -15594,10 +15594,8 @@ static void set_next_task_fair(struct rq
>  		hrtick_start_fair(rq, p);
>  }
>  
> -void init_cfs_rq(struct cfs_rq *cfs_rq, struct cfs_rq *parent)
> +void init_cfs_rq(struct cfs_rq *cfs_rq)
>  {
> -	if (parent)
> -		cfs_rq->idle = parent->idle;
>  	cfs_rq->tasks_timeline = RB_ROOT_CACHED;
>  	cfs_rq->zero_vruntime = (u64)(-(1LL << 20));
>  	raw_spin_lock_init(&cfs_rq->removed.lock);
> @@ -15648,7 +15646,7 @@ int alloc_fair_sched_group(struct task_g
>  			goto err;
>  
>  		se = tg_se(tg, i);
> -		init_cfs_rq(cfs_rq, tg_cfs_rq(parent, i));
> +		init_cfs_rq(cfs_rq);
>  		init_tg_cfs_entry(tg, cfs_rq, se, i, tg_se(parent, i));
>  		init_entity_runnable_average(se);
>  	}
> @@ -15661,19 +15659,20 @@ int alloc_fair_sched_group(struct task_g
>  
>  void online_fair_sched_group(struct task_group *tg)
>  {
> -	struct sched_entity *se;
> -	struct rq_flags rf;
> -	struct rq *rq;
>  	int i;
>  
>  	for_each_possible_cpu(i) {
> -		rq = cpu_rq(i);
> -		se = tg_se(tg, i);
> -		rq_lock_irq(rq, &rf);
> +		struct cfs_rq *cfs_rq = tg_cfs_rq(tg, i);
> +		struct sched_entity *se = tg_se(tg, i);
> +		struct rq *rq = cpu_rq(i);
> +
> +		guard(rq_lock_irq)(rq);
> +
>  		update_rq_clock(rq);
>  		attach_entity_cfs_rq(se);
>  		sync_throttle(tg, i);
> -		rq_unlock_irq(rq, &rf);
> +
> +		cfs_rq->idle = tg_cfs_rq(tg->parent, i)->idle;
>  	}
>  }
>  


Yeah this should do the job.


Thanks!

Kayra