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

[Peter Zijlstra]

On Thu, Nov 05, 2026 at 10:20:00PM +0300, Kayra Cizmeci wrote:
> On Thu, Nov 05, 2026 at 05:55:27PM +0100, Peter Zijlstra wrote:
> > Rather than keeping cfs_rq::idle as a copy of tg::idle, make it the sum of
> > all ancestor tg::idle. This makes it trivial to tell if the current cgroup
> > is marked idle somewhere along its ancestral path.
> > 
> > This makes se_id_idle() behave differently. Notably is will now be true
> > when there is a idle cgroup anywhere in the ancestor path. This
> > addresses an issue where preemption of cgroup::idle was overlooked when
> > flattening the runqueue.
> 
> How? The diff doesn't touches se_is_idle(). se_is_idle() just looks to
> the policy for tasks:
> 
> static int se_is_idle(struct sched_entity *se)
> {
> 	if (entity_is_task(se))
> 		return task_has_idle_policy(task_of(se));
> 	return cfs_rq_is_idle(group_cfs_rq(se));
> }

> Or am I getting something wrong?

cfs_rq_is_idle() := cfs_rq->idle > 0

and cfs_rq->idle is now getting incremented for the whole subtree. So if
any ancestor is idle, this is now true.


> > --- 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.