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

[Peter Zijlstra]

On Fri, Nov 06, 2026 at 11:57:03AM +0300, Kayra Cizmeci wrote:
> On Fri, Nov 06, 2026 at 09:31:06AM +0100, Peter Zijlstra wrote:
> > 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.
> 
> 
> Yeah, it is for group path. But we call se_is_idle for tasks in
> wakeup_preempt_fair(). Not groups, no?

Ah, that's what you meant. So we should do something like:

	if (entity_is_task(se))
		return task_has_idle_policy(task_of(se)) || cfs_rq_is_idle(cfs_rq_of(se));
	return cfs_rq_is_idle(group_cfs_rq(se));


no?