Re: [PATCH] cpuidle: teo: Do not return a disabled idle state

[Rafael J. Wysocki (Intel)]

On Thu, Aug 27, 2026 at 11:05 AM Xueqin Luo <luoxueqin@xxxxxxxxxx> wrote:
>
> If idle state 0 is disabled and none of the enabled idle states satisfies
> the PM QoS latency constraint, constraint_idx in teo_select() stays at its
> initial value of 0, because the loop that updates it starts at state 1,
> and so the candidate state index is capped at state 0, which is disabled,
> and that state is returned to the caller.
>
> The cpuidle core does not validate the returned state index against the
> per-state disable flags, so the CPU ends up entering the disabled state 0.
>
> Address this by falling back to the shallowest enabled idle state, found
> by the same loop that computes the candidate, whenever the state selected
> by the latency constraint check turns out to be disabled.
>
> Note that if none of the enabled idle states meets the latency constraint,
> choosing the shallowest enabled one is the least intrusive option, and it
> is also consistent with the behavior of the menu governor.
>
> Fixes: c410a9a142f1 ("cpuidle: teo: Change the main idle state selection logic")
> Signed-off-by: Xueqin Luo <luoxueqin@xxxxxxxxxx>
> ---
>  drivers/cpuidle/governors/teo.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
> index ac43b9b013b3..b4b14c86b2a9 100644
> --- a/drivers/cpuidle/governors/teo.c
> +++ b/drivers/cpuidle/governors/teo.c
> @@ -433,6 +433,14 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,

I would instead add the following conditional here:

if (idx0 < constraint_idx)
        constraint_idx = idx0;

>         if (idx > constraint_idx)
>                 idx = constraint_idx;
>
> +       /*
> +        * If the state selected above is disabled, which is only possible if
> +        * it is state 0 (all of the enabled states violate the latency
> +        * constraint in that case), fall back to the shallowest enabled one.
> +        */
> +       if (dev->states_usage[idx].disable)
> +               idx = idx0;
> +
>         /*
>          * If the tick has not been stopped and either the candidate state is
>          * state 0 or its target residency is low enough, there is basically
> --
> 2.43.0
>