Re: [PATCH v3] sched/core: Skip rq->avg_idle update without a valid idle_stamp

Vincent Guittot <[email protected]>
Newsgroups gmane.linux.kernel
Message-ID <CAKfTPtBywWojCTedRR17_cLjbD0kkHWwjs9=dWL9Zh792-dPxQ@mail.gmail.com>
On Fri, 7 Aug 2026 at 22:47, Shubhang Kaushik (Ampere) <[email protected]> wrote:
>
> Commit 4b603f1551a73 ("sched: Update rq->avg_idle when a task is moved
> to an idle CPU") moved rq->avg_idle accounting out of the wakeup path and
> into put_prev_task_idle(), so that the idle interval is consumed whenever
> the idle task is switched out.
>
> The wakeup-side accounting that it replaced only updated rq->avg_idle
> when rq->idle_stamp was non-zero. The new helper lost that validity
> check and unconditionally computes:
>
>         rq_clock(rq) - rq->idle_stamp
>
> If rq->idle_stamp is zero, this uses rq_clock(rq) as the sample. That is
> not a valid idle duration and can immediately drive rq->avg_idle to its
> clamp.
>
> This can happen when sched_balance_newidle() returns before setting
> rq->idle_stamp, for example when this_rq->ttwu_pending is set. In that
> case the rq can switch to the idle task with idle_stamp still zero and
> leave idle again when the pending wakeup is processed.
>
> Other paths can also switch to the idle task without setting
> rq->idle_stamp via newidle_balance(), for example find_proxy_task() or
> force-idling.
>
> Restore the idle_stamp validity check in update_rq_avg_idle() and skip
> the rq->avg_idle update when there is no measured idle interval.
>
> Fixes: 4b603f1551a73 ("sched: Update rq->avg_idle when a task is moved to an idle CPU")
> Reviewed-by: K Prateek Nayak <[email protected]>
> Acked-by: John Stultz <[email protected]>
> Signed-off-by: Shubhang Kaushik (Ampere) <[email protected]>

Reviewed-by: Vincent Guittot <[email protected]>

> ---
> Temporary tracing under hackbench load confirmed that
> update_rq_avg_idle() can be reached with rq->idle_stamp == 0.
> Hackbench showed no material regression versus v7.2-rc5 mainline.
>
> Related discussion:
>   https://lore.kernel.org/r/[email protected]
>
> This is a narrower variant of the earlier proposal. It keeps the
> rq->idle_stamp guard in update_rq_avg_idle(), but intentionally does not
> stamp idle entry from set_next_task_idle(), preserving the existing
> newidle accounting model and avoiding force-idle/proxy-exec accounting
> concerns.
> ---
> Changes in v3:
>   - Describe the sched_balance_newidle()/ttwu_pending path as an
>     example of entering idle without a valid rq->idle_stamp.
>   - Drop unlikely() from the idle_stamp check.
>   - Add Acked-by from John Stultz.
>
> Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
>   - Add Reviewed-by from Prateek.
>   - Mention find_proxy_task() and force-idling as examples of paths that
>     can switch to the idle task without a valid rq->idle_stamp.
>   - Cc John Stultz.
>
> Link to v1: https://lore.kernel.org/r/[email protected]
> ---
>  kernel/sched/core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 96226707c2f6135341aa779b8262f113e103d8ad..68fa724cd668fb6c4cead329d05fa95e2f1ea5db 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3732,11 +3732,17 @@ static inline void ttwu_do_wakeup(struct task_struct *p)
>
>  void update_rq_avg_idle(struct rq *rq)
>  {
> -       u64 delta = rq_clock(rq) - rq->idle_stamp;
> -       u64 max = 2*rq->max_idle_balance_cost;
> +       u64 idle_stamp = rq->idle_stamp;
> +       u64 delta, max;
> +
> +       if (!idle_stamp)
> +               return;
> +
> +       delta = rq_clock(rq) - idle_stamp;
>
>         update_avg(&rq->avg_idle, delta);
>
> +       max = 2 * rq->max_idle_balance_cost;
>         if (rq->avg_idle > max)
>                 rq->avg_idle = max;
>         rq->idle_stamp = 0;
>
> ---
> base-commit: 3f008280327ba5ad132965abab0c7846283cef0c
> change-id: 20260728-master-55cd7cc13290
>
> Best regards,
> --
> Shubhang Kaushik (Ampere) <[email protected]>
>
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.