Re: [PATCH v2] sched/idle: Fix avg_idle saturation by establishing symmetric idle entry hook

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Masahito,

On 4/23/2026 8:03 AM, Masahito S wrote:
> update_rq_avg_idle(), called from put_prev_task_idle(), computes
> rq->avg_idle as rq_clock() - rq->idle_stamp.  However, idle_stamp is
> only set by sched_balance_newidle() when a CPU enters CPU_NEWLY_IDLE
> through the fair class path.  When the idle task is preempted without
> sched_balance_newidle() having run (boot, hotplug, sched class
> transitions), idle_stamp remains 0, producing a delta equal to
> rq_clock() — a value in the billions of nanoseconds — which saturates
> avg_idle at 2 * max_idle_balance_cost.

But these are rare cases right?

Hotplug would anyways trigger a load balance when the CPU comes online
and the avg_idle will stabilize thereafter.

Boot happens once so it should be fine to saturate the counter at boot, and
idle task being preempted implies we are calling put_prev_task_idle(). For
idle task to be picked again it has to go though the pick for rest of the
scheduler classes which would do a newidle balance at fair right?

Maybe there is some case with sched-ext where this saturates but then the
counter only becomes relevant when the sched-ext scheduler is unloaded.

With PROXY_EXEC, we do switch to idle between schedule() to take the
current tasks off the CPU so this does have some merit.

> 
> This inflated avg_idle prevents sched_balance_newidle() from
> early-returning (fair.c: avg_idle < max_newidle_lb_cost check),
> making it overly aggressive.  The resulting excess newidle migrations
> override wake-time placement decisions made by select_idle_sibling(),
> degrading cache locality that careful placement (recent_used_cpu,
> select_idle_core, etc.) is designed to preserve.
> 
> Fix this by:
> 
> 1. Adding an idle_stamp validity guard to update_rq_avg_idle(), so
>    that a zero idle_stamp is never used as a timestamp.
> 
> 2. Setting idle_stamp in set_next_task_idle() when it has not already
>    been set by sched_balance_newidle().  This establishes a symmetric
>    idle entry/exit contract: set_next_task_idle() marks the start of
>    the idle period, put_prev_task_idle() measures and records it via
>    update_rq_avg_idle().
> 
> The entry hook preserves idle_stamp if sched_balance_newidle() has
> already set it, maintaining the existing semantic where balance-attempt
> duration is included in the idle measurement.
> 
> Signed-off-by: Masahito Suzuki <firelzrd@gmail.com>
> ---
> Changes in v2:
> - Added missing Signed-off-by tag (no functional changes).
>   Thanks to Eric Naim and Christian Loehle for pointing this out.
> 
>  kernel/sched/core.c | 3 +++
>  kernel/sched/idle.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 496dff740d..ec801f731c 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3633,6 +3633,9 @@ static inline void ttwu_do_wakeup(struct task_struct *p)
> 
>  void update_rq_avg_idle(struct rq *rq)
>  {
> +       if (!rq->idle_stamp)
> +               return;
> +

I think this makes sense since we can be forced into idle and we
don't want to account that.

>         u64 delta = rq_clock(rq) - rq->idle_stamp;
>         u64 max = 2*rq->max_idle_balance_cost;
> 
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index a83be0c834..9ceb7e6224 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -491,6 +491,9 @@ static void set_next_task_idle(struct rq *rq, struct task_struct *next, bool fir
>         schedstat_inc(rq->sched_goidle);
>         next->se.exec_start = rq_clock_task(rq);
> 
> +       if (!rq->idle_stamp)
> +               rq->idle_stamp = rq_clock(rq);
> +

I don't think this is required because we can switch the donor to
idle task for PROXY_EXEC and we don't want to account that as a
short idle time unless there is another case I'm missing.

>         /*
>          * rq is about to be idle, check if we need to update the
>          * lost_idle_time of clock_pelt
-- 
Thanks and Regards,
Prateek