[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <450effef-57e3-4e2d-ba22-d7bf802ac07a@arm.com>
Date: Wed, 21 Jan 2026 13:39:12 +0000
From: Christian Loehle <christian.loehle@....com>
To: "Rafael J. Wysocki" <rafael@...nel.org>,
Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Doug Smythies <dsmythies@...us.net>
Subject: Re: [PATCH v1] cpuidle: governors: menu: Always check timers with
tick stopped
On 1/20/26 15:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> After commit 5484e31bbbff ("cpuidle: menu: Skip tick_nohz_get_sleep_length()
> call in some cases"), if the return value of get_typical_interval()
> multiplied by NSEC_PER_USEC is not greater than RESIDENCY_THRESHOLD_NS,
> the menu governor will skip computing the time till the closest timer.
> If that happens when the tick has been stopped already, the selected
> idle state may be too deep due to the subsequent check comparing
> predicted_ns with TICK_NSEC and causing its value to be replaced with
> the expected time till the closest timer, which is KTIME_MAX in that
> case. That will cause the deepest enabled idle state to be selected,
> but the time till the closest timer very well may be shorter than the
> target residency of that state, in which case a shallower state should
> be used.
>
> Address this by making menu_select() always compute the time till the
> closest timer when the tick has been stopped.
>
> Also move the predicted_ns check mentioned above into the branch in
> which the time till the closest timer is determined because it only
> needs to be done in that case.
>
> Fixes: 5484e31bbbff ("cpuidle: menu: Skip tick_nohz_get_sleep_length() call in some cases")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/cpuidle/governors/menu.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -239,7 +239,7 @@ static int menu_select(struct cpuidle_dr
>
> /* Find the shortest expected idle interval. */
> predicted_ns = get_typical_interval(data) * NSEC_PER_USEC;
> - if (predicted_ns > RESIDENCY_THRESHOLD_NS) {
> + if (predicted_ns > RESIDENCY_THRESHOLD_NS || tick_nohz_tick_stopped()) {
> unsigned int timer_us;
>
> /* Determine the time till the closest timer. */
> @@ -259,6 +259,16 @@ static int menu_select(struct cpuidle_dr
> RESOLUTION * DECAY * NSEC_PER_USEC);
> /* Use the lowest expected idle interval to pick the idle state. */
> predicted_ns = min((u64)timer_us * NSEC_PER_USEC, predicted_ns);
> + /*
> + * If the tick is already stopped, the cost of possible short
> + * idle duration misprediction is much higher, because the CPU
> + * may be stuck in a shallow idle state for a long time as a
> + * result of it. In that case, say we might mispredict and use
> + * the known time till the closest timer event for the idle
> + * state selection.
> + */
> + if (tick_nohz_tick_stopped() && predicted_ns < TICK_NSEC)
> + predicted_ns = data->next_timer_ns;
> } else {
> /*
> * Because the next timer event is not going to be determined
> @@ -285,16 +295,6 @@ static int menu_select(struct cpuidle_dr
> }
>
> /*
> - * If the tick is already stopped, the cost of possible short idle
> - * duration misprediction is much higher, because the CPU may be stuck
> - * in a shallow idle state for a long time as a result of it. In that
> - * case, say we might mispredict and use the known time till the closest
> - * timer event for the idle state selection.
> - */
> - if (tick_nohz_tick_stopped() && predicted_ns < TICK_NSEC)
> - predicted_ns = data->next_timer_ns;
> -
> - /*
> * Find the idle state with the lowest power while satisfying
> * our constraints.
> */
>
Seems I forgot about this, sorry!
Reviewed-by: Christian Loehle <christian.loehle@....com>
Powered by blists - more mailing lists