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

[Tim Chen <tim.c.chen@linux.intel.com>]

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@linaro.org>
> ---
>  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().

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