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

[Vincent Guittot]

On Fri, 6 Nov 2026 at 10:01, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> 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));

This is necessary for the 1st check in wakeup_preempt_sched_idle() to
prevent useless call to find_matching_se()

>
>
> no?
>