[PATCH v1] cpuidle: governors: menu: Select polling state in some more cases

Rafael J. Wysocki posted 1 patch 3 months, 2 weeks ago
drivers/cpuidle/governors/menu.c |    7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[PATCH v1] cpuidle: governors: menu: Select polling state in some more cases
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

A throughput regression of 11% introduced by commit 779b1a1cb13a ("cpuidle:
governors: menu: Avoid selecting states with too much latency") has been
reported and it is related to the case when the menu governor checks if
selecting a proper idle state instead of a polling one makes sense.

In particular, it is questionable to do so if the exit latency of the
idle state in question exceeds the predicted idle duration, so add a
check for that, which is sufficient to make the reported regression go
away, and update the related code comment accordingly.

Fixes: 779b1a1cb13a ("cpuidle: governors: menu: Avoid selecting states with too much latency")
Closes: https://lore.kernel.org/linux-pm/004501dc43c9$ec8aa930$c59ffb90$@telus.net/
Reported-by: Doug Smythies <dsmythies@telus.net>
Tested-by: Doug Smythies <dsmythies@telus.net>
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/menu.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -321,10 +321,13 @@ static int menu_select(struct cpuidle_dr
 
 		/*
 		 * Use a physical idle state, not busy polling, unless a timer
-		 * is going to trigger soon enough.
+		 * is going to trigger soon enough or the exit latency of the
+		 * idle state in question is greater than the predicted idle
+		 * duration.
 		 */
 		if ((drv->states[idx].flags & CPUIDLE_FLAG_POLLING) &&
-		    s->target_residency_ns <= data->next_timer_ns) {
+		    s->target_residency_ns <= data->next_timer_ns &&
+		    s->exit_latency_ns <= predicted_ns) {
 			predicted_ns = s->target_residency_ns;
 			idx = i;
 			break;
Re: [PATCH v1] cpuidle: governors: menu: Select polling state in some more cases
Posted by Christian Loehle 3 months, 2 weeks ago
On 10/23/25 18:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> A throughput regression of 11% introduced by commit 779b1a1cb13a ("cpuidle:
> governors: menu: Avoid selecting states with too much latency") has been
> reported and it is related to the case when the menu governor checks if
> selecting a proper idle state instead of a polling one makes sense.
> 
> In particular, it is questionable to do so if the exit latency of the
> idle state in question exceeds the predicted idle duration, so add a
> check for that, which is sufficient to make the reported regression go
> away, and update the related code comment accordingly.
> 
> Fixes: 779b1a1cb13a ("cpuidle: governors: menu: Avoid selecting states with too much latency")
> Closes: https://lore.kernel.org/linux-pm/004501dc43c9$ec8aa930$c59ffb90$@telus.net/
> Reported-by: Doug Smythies <dsmythies@telus.net>
> Tested-by: Doug Smythies <dsmythies@telus.net>
> Cc: All applicable <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpuidle/governors/menu.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -321,10 +321,13 @@ static int menu_select(struct cpuidle_dr
>  
>  		/*
>  		 * Use a physical idle state, not busy polling, unless a timer
> -		 * is going to trigger soon enough.
> +		 * is going to trigger soon enough or the exit latency of the
> +		 * idle state in question is greater than the predicted idle
> +		 * duration.
>  		 */
>  		if ((drv->states[idx].flags & CPUIDLE_FLAG_POLLING) &&
> -		    s->target_residency_ns <= data->next_timer_ns) {
> +		    s->target_residency_ns <= data->next_timer_ns &&
> +		    s->exit_latency_ns <= predicted_ns) {
>  			predicted_ns = s->target_residency_ns;
>  			idx = i;
>  			break;
> 
> 
> 

Makes sense to me, sorry I don't have a good testing platform with a
polling state.

Reviewed-by: Christian Loehle <christian.loehle@arm.com>