[PATCH] cpuidle: menu: Remove incorrect unlikely() annotation

[PATCH] cpuidle: menu: Remove incorrect unlikely() annotation

[Breno Leitao]

The unlikely() annotation on the early-return condition in menu_select()
is incorrect on systems with only one idle state (e.g., ARM64 servers
with a single ACPI LPI state). Branch profiling shows 100% misprediction
on such systems since drv->state_count <= 1 is always true.

On platforms where only state0 is available, this path is the common
case, not an unlikely edge case. Remove the misleading annotation to
let the branch predictor learn the actual behavior.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/cpuidle/governors/menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 64d6f7a1c776..ef9c5a84643e 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 		data->bucket = BUCKETS - 1;
 	}
 
-	if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
+	if (drv->state_count <= 1 || latency_req == 0 ||
 	    ((data->next_timer_ns < drv->states[1].target_residency_ns ||
 	      latency_req < drv->states[1].exit_latency_ns) &&
 	     !dev->states_usage[0].disable)) {

---
base-commit: 34aa263125b6732375abcb908d73d98169154bb5
change-id: 20260105-annotated_idle-d6b614ecd207

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] cpuidle: menu: Remove incorrect unlikely() annotation

[Christian Loehle]

On 1/5/26 14:37, Breno Leitao wrote:
> The unlikely() annotation on the early-return condition in menu_select()
> is incorrect on systems with only one idle state (e.g., ARM64 servers
> with a single ACPI LPI state). Branch profiling shows 100% misprediction
> on such systems since drv->state_count <= 1 is always true.
> 
> On platforms where only state0 is available, this path is the common
> case, not an unlikely edge case. Remove the misleading annotation to
> let the branch predictor learn the actual behavior.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/cpuidle/governors/menu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index 64d6f7a1c776..ef9c5a84643e 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>  		data->bucket = BUCKETS - 1;
>  	}
>  
> -	if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
> +	if (drv->state_count <= 1 || latency_req == 0 ||
>  	    ((data->next_timer_ns < drv->states[1].target_residency_ns ||
>  	      latency_req < drv->states[1].exit_latency_ns) &&
>  	     !dev->states_usage[0].disable)) {
> 
> ---
> base-commit: 34aa263125b6732375abcb908d73d98169154bb5
> change-id: 20260105-annotated_idle-d6b614ecd207
> 
> Best regards,
> --  
> Breno Leitao <leitao@debian.org>
> 
> 

Fine with me per se, I don't think the unlikely() annotation makes a
difference for the 'good case' either, but if you run into this I'd be curious
if you can see a difference with menu (which should stop the tick on every idle enter
regardless) and teo (which should never stop the tick on state_count == 1).
Alternative you can also just change the menu branch to not stop the tick.
I'd like to know if we need something more sophisticated generally here.

Re: [PATCH] cpuidle: menu: Remove incorrect unlikely() annotation

[Breno Leitao]

On Mon, Jan 05, 2026 at 03:17:05PM +0000, Christian Loehle wrote:
> On 1/5/26 14:37, Breno Leitao wrote:
> > The unlikely() annotation on the early-return condition in menu_select()
> > is incorrect on systems with only one idle state (e.g., ARM64 servers
> > with a single ACPI LPI state). Branch profiling shows 100% misprediction
> > on such systems since drv->state_count <= 1 is always true.
> > 
> > On platforms where only state0 is available, this path is the common
> > case, not an unlikely edge case. Remove the misleading annotation to
> > let the branch predictor learn the actual behavior.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> >  drivers/cpuidle/governors/menu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> > index 64d6f7a1c776..ef9c5a84643e 100644
> > --- a/drivers/cpuidle/governors/menu.c
> > +++ b/drivers/cpuidle/governors/menu.c
> > @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> >  		data->bucket = BUCKETS - 1;
> >  	}
> >  
> > -	if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
> > +	if (drv->state_count <= 1 || latency_req == 0 ||
> >  	    ((data->next_timer_ns < drv->states[1].target_residency_ns ||
> >  	      latency_req < drv->states[1].exit_latency_ns) &&
> >  	     !dev->states_usage[0].disable)) {
> > 
> > ---
> > base-commit: 34aa263125b6732375abcb908d73d98169154bb5
> > change-id: 20260105-annotated_idle-d6b614ecd207
> > 
> > Best regards,
> > --  
> > Breno Leitao <leitao@debian.org>
> > 
> > 
> 
> Fine with me per se, I don't think the unlikely() annotation makes a
> difference for the 'good case' either, but if you run into this I'd be curious
> if you can see a difference with menu (which should stop the tick on every idle enter
> regardless) and teo (which should never stop the tick on state_count == 1).
> Alternative you can also just change the menu branch to not stop the tick.
> I'd like to know if we need something more sophisticated generally here.

Probably not. I am just running PROFILE_ANNOTATED_BRANCHES tests on some
production arm64 host and addressing those that are making wrong
assumptions.

Re: [PATCH] cpuidle: menu: Remove incorrect unlikely() annotation

[Rafael J. Wysocki]

On Mon, Jan 5, 2026 at 3:38 PM Breno Leitao <leitao@debian.org> wrote:
>
> The unlikely() annotation on the early-return condition in menu_select()
> is incorrect on systems with only one idle state (e.g., ARM64 servers
> with a single ACPI LPI state). Branch profiling shows 100% misprediction
> on such systems since drv->state_count <= 1 is always true.
>
> On platforms where only state0 is available, this path is the common
> case, not an unlikely edge case. Remove the misleading annotation to
> let the branch predictor learn the actual behavior.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/cpuidle/governors/menu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index 64d6f7a1c776..ef9c5a84643e 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>                 data->bucket = BUCKETS - 1;
>         }
>
> -       if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
> +       if (drv->state_count <= 1 || latency_req == 0 ||
>             ((data->next_timer_ns < drv->states[1].target_residency_ns ||
>               latency_req < drv->states[1].exit_latency_ns) &&
>              !dev->states_usage[0].disable)) {
>
> ---

Applied as 6.20 material, thanks!