Re: [PATCH 1/2] cpuidle: Skip miss accounting for timer broadcast fallbacks

[Christian Loehle]

On 11/10/26 20:16, Rafael J. Wysocki (Intel) wrote:
> On Fri, Oct 30, 2026 at 9:36 AM Christian Loehle
> <christian.loehle@xxxxxxx> wrote:
>>
>> A failed tick_broadcast_enter() can force a shallower state than the
>> governor selected. Exclude successful broadcast fallbacks from above/below
>> miss accounting while retaining their usage, residency and governor
>> feedback.
> 
> Is this a theoretical problem, or can it be observed in practice?
> 
> In either case, it would be good to add some information on that here.

It's very much a real problem, see the cover letter, it's about 5% of
the accounted idle_misses on arm64.

> 
>> Fixes: 04dab58a39d4 ("cpuidle: Add 'above' and 'below' idle state metrics")
>> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
>> ---
>>  drivers/cpuidle/cpuidle.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 2d2f40a2cb81..faa29f58738e 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -222,6 +222,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>>
>>         struct cpuidle_state *target_state = &drv->states[index];
>>         bool broadcast = !!(target_state->flags & CPUIDLE_FLAG_TIMER_STOP);
>> +       bool broadcast_failed = false;
>>         ktime_t time_start, time_end;
>>
>>         instrumentation_begin();
>> @@ -232,6 +233,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>>          * CPU as a broadcast timer, this call may fail if it is not available.
>>          */
>>         if (broadcast && tick_broadcast_enter()) {
>> +               broadcast_failed = true;
>>                 index = find_deepest_state(drv, dev, target_state->exit_latency_ns,
>>                                            CPUIDLE_FLAG_TIMER_STOP, false);
>>
>> @@ -306,6 +308,13 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>>                 dev->states_usage[entered_state].time_ns += diff;
>>                 dev->states_usage[entered_state].usage++;
>>
>> +               /*
>> +                * The broadcast constraint forced a shallower state, do not
>> +                * account for it as a cpu_idle_miss.
>> +                */
>> +               if (broadcast_failed)
>> +                       goto out;
>> +
>>                 if (diff < drv->states[entered_state].target_residency_ns) {
>>                         for (i = entered_state - 1; i >= 0; i--) {
>>                                 if (dev->states_usage[i].disable)
>> @@ -338,6 +347,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>>                 dev->states_usage[index].rejected++;
>>         }
>>
>> +out:
>>         instrumentation_end();
>>
>>         return entered_state;
>> --
>> 2.34.1
>>