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

[Vincent Guittot]

On Sat, 7 Nov 2026 at 12:36, Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

Reviewed-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>


> ---
>  kernel/sched/debug.c |    1
>  kernel/sched/fair.c  |   88 +++++++++++++++++++++++++++++++--------------------
>  kernel/sched/sched.h |    2 -
>  3 files changed, 56 insertions(+), 35 deletions(-)
>
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -1090,6 +1090,7 @@ void print_cfs_rq(struct seq_file *m, in
>         SEQ_printf(m, "  .%-30s: %d\n", "h_nr_runnable", cfs_rq->h_nr_runnable);
>         SEQ_printf(m, "  .%-30s: %d\n", "h_nr_queued", cfs_rq->h_nr_queued);
>         SEQ_printf(m, "  .%-30s: %d\n", "h_nr_idle", cfs_rq->h_nr_idle);
> +       SEQ_printf(m, "  .%-30s: %d\n", "idle", cfs_rq->idle);
>         SEQ_printf(m, "  .%-30s: %ld\n", "load", cfs_rq->load.weight);
>         SEQ_printf(m, "  .%-30s: %lu\n", "load_avg",
>                         cfs_rq->avg.load_avg);
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -445,8 +445,10 @@ static int cfs_rq_is_idle(struct cfs_rq
>
>  static int se_is_idle(struct sched_entity *se)
>  {
> -       if (entity_is_task(se))
> -               return task_has_idle_policy(task_of(se));
> +       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));
>  }
>
> @@ -15659,19 +15661,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;
>         }
>  }
>
> @@ -15780,6 +15783,43 @@ int sched_group_set_shares(struct task_g
>         return ret;
>  }
>
> +struct tg_set_idle_struct {
> +       long idle;
> +       int cpu;
> +};
> +
> +static int tg_set_idle_up(struct task_group *tg, void *data)
> +{
> +       struct tg_set_idle_struct *tsi = data;
> +       struct cfs_rq *grp_cfs_rq = tg_cfs_rq(tg, tsi->cpu);
> +       struct sched_entity *se = tg_se(tg, tsi->cpu);
> +       int old_idle = grp_cfs_rq->idle;
> +       int task_delta;
> +
> +       grp_cfs_rq->idle += tsi->idle;
> +
> +       if (!!old_idle == !!grp_cfs_rq->idle)
> +               return 0;
> +
> +       task_delta = grp_cfs_rq->h_nr_queued - grp_cfs_rq->h_nr_idle;
> +       if (!task_delta)
> +               return 0;
> +
> +       task_delta *= tsi->idle;
> +       for_each_sched_entity(se) {
> +               struct cfs_rq *cfs_rq = cfs_rq_of(se);
> +
> +               if (!se->on_rq)
> +                       break;
> +
> +               cfs_rq->h_nr_idle += task_delta;
> +
> +               if (cfs_rq_is_idle(cfs_rq))
> +                       break;
> +       }
> +       return 0;
> +}
> +
>  int sched_group_set_idle(struct task_group *tg, long idle)
>  {
>         int i;
> @@ -15798,34 +15838,14 @@ int sched_group_set_idle(struct task_gro
>
>         for_each_possible_cpu(i) {
>                 struct rq *rq = cpu_rq(i);
> -               struct sched_entity *se = tg_se(tg, i);
> -               struct cfs_rq *grp_cfs_rq = tg_cfs_rq(tg, i);
> -               bool was_idle = cfs_rq_is_idle(grp_cfs_rq);
> -               long idle_task_delta;
>
>                 guard(rq_lock_irqsave)(rq);
>
> -               grp_cfs_rq->idle = idle;
> -               if (WARN_ON_ONCE(was_idle == cfs_rq_is_idle(grp_cfs_rq)))
> -                       continue;
> -
> -               idle_task_delta = grp_cfs_rq->h_nr_queued -
> -                                 grp_cfs_rq->h_nr_idle;
> -               if (!cfs_rq_is_idle(grp_cfs_rq))
> -                       idle_task_delta *= -1;
> -
> -               for_each_sched_entity(se) {
> -                       struct cfs_rq *cfs_rq = cfs_rq_of(se);
> -
> -                       if (!se->on_rq)
> -                               break;
> -
> -                       cfs_rq->h_nr_idle += idle_task_delta;
> -
> -                       /* Already accounted at parent level and above. */
> -                       if (cfs_rq_is_idle(cfs_rq))
> -                               break;
> -               }
> +               struct tg_set_idle_struct tsi = {
> +                       .idle = idle ? 1 : -1,
> +                       .cpu = i,
> +               };
> +               walk_tg_tree_from(tg, tg_nop, tg_set_idle_up, &tsi);
>         }
>
>         /* Idle groups have minimum weight. */
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -751,7 +751,7 @@ struct cfs_rq {
>         struct list_head        leaf_cfs_rq_list;
>         struct task_group       *tg;    /* Group that "owns" this runqueue */
>
> -       /* Locally cached copy of our task_group's idle value */
> +       /* Sum of all ancestor tg->idle. */
>         int                     idle;
>
>  # ifdef CONFIG_CFS_BANDWIDTH