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

From: Kayra Cizmeci

Date: Fri Nov 06 2026 - 04:04:07 EST


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