Re: [PATCH 11/18 v2] sched/fair: Support not wakeup case in select_idle_sibling

[Vincent Guittot]

On Wed, 7 Oct 2026 at 19:55, Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> wrote:
>
> On Fri, 2026-10-02 at 17:44 +0200, Vincent Guittot wrote:
> > With push callback mecanism, select_task_rq_fair can be called for a task
> > that is already enqueued. Task into account this case when choosing idle
> > CPU.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 6ed3e5bb7fd6..38eb80dbd1a4 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7974,10 +7974,13 @@ static int choose_sched_idle_rq(struct rq *rq, struct task_struct *p)
> >       return sched_idle_rq(rq) && !task_has_idle_policy(p);
> >  }
> >
> > +static int idle_cpu_without(int cpu, struct task_struct *p);
> > +
> >  static int choose_idle_cpu(int cpu, struct task_struct *p)
> >  {
> >       return available_idle_cpu(cpu) ||
> > -            choose_sched_idle_rq(cpu_rq(cpu), p);
> > +            choose_sched_idle_rq(cpu_rq(cpu), p) ||
> > +            idle_cpu_without(cpu, p);
> >  }
> >
> >  static void
>
>
> Hi Vincent,
>
> One concern I have is that many CPUs will try to push
> their tasks onto the same idle CPU and causes over stacking.
>
> idle_cpu_without() doesn't check nr_running. Its comment says:
>
>          * rq->nr_running can't be used but an updated version without the
>          * impact of p on cpu must be used instead. The updated nr_running
>          * be computed and tested before calling idle_cpu_without().


Yes, I missed this part and will fix it

I'm going to merge the check of check nr_running in idle_cpu_without()



>
> and update_sg_wakeup_stats() does this:
>
>         if (!nr_running && idle_cpu_without(i, p))
>
> choose_idle_cpu() calls it without that test. So a CPU that runs its
> idle task but already has a queued task is now idle for every caller:
> the target, prev and recent_used_cpu checks, __select_idle_cpu(),
> select_idle_smt() and select_idle_capacity(). This affects wakeups
> too, not only pushes.
>
> For a wakeup to an idle CPU, the wakelist usually sets ttwu_pending,
> which idle_cpu_without() checks. A push enqueues the task directly, so
> after a push the destination stays idle for everyone until it
> schedules. If it wakes up from a deep idle state, this can take a
> while, and other CPUs can push or wake their tasks onto it in the
> meantime.
>
> The fixup below removes p from nr_running when p is queued on the CPU,
> and calls idle_cpu_without() only when nothing else is queued. The
> tick case that this patch is for still works: p is the only task, so
> its CPU is still idle without it.
>
> It is build tested only.
>
> Thanks,
> Tim
>
> ---
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3eb8a0170902..cb00f24a58fa 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7978,9 +7978,16 @@ static int idle_cpu_without(int cpu, struct task_struct *p);
>
>  static int choose_idle_cpu(int cpu, struct task_struct *p)
>  {
> +       struct rq *rq = cpu_rq(cpu);
> +       unsigned int nr_running = rq->nr_running;
> +
> +       /* idle_cpu_without() needs nr_running without p */
> +       if (task_cpu(p) == cpu && task_on_rq_queued(p))
> +               nr_running--;
> +
>         return available_idle_cpu(cpu) ||
> -              choose_sched_idle_rq(cpu_rq(cpu), p) ||
> -              idle_cpu_without(cpu, p);
> +              choose_sched_idle_rq(rq, p) ||
> +              (!nr_running && idle_cpu_without(cpu, p));
>  }
>
>  static void
>
>