Re: [PATCH v2] cpuidle: Deny idle entry when CPU already have IPI interrupt pending

On 4/13/26 06:03, Maulik Shah (mkshah) wrote:
> 
> 
> On 4/6/2026 8:37 PM, Rafael J. Wysocki wrote:
>> On Fri, Apr 3, 2026 at 6:08 AM Maulik Shah <[email protected]> wrote:
>>>
>>> CPU can get IPI interrupt from another CPU while it is executing
>>> cpuidle_select() or about to execute same. The selection do not account
>>> for pending interrupts and may continue to enter selected idle state only
>>> to exit immediately.
>>>
>>> Example trace collected when there is cross CPU IPI.
>>>
>>>  [000] 154.892148: sched_waking: comm=sugov:4 pid=491 prio=-1 target_cpu=007
>>>  [000] 154.892148: ipi_raise: target_mask=00000000,00000080 (Function call interrupts)
>>>  [007] 154.892162: cpu_idle: state=2 cpu_id=7
>>>  [007] 154.892208: cpu_idle: state=4294967295 cpu_id=7
>>>  [007] 154.892211: irq_handler_entry: irq=2 name=IPI
>>>  [007] 154.892211: ipi_entry: (Function call interrupts)
>>>  [007] 154.892213: sched_wakeup: comm=sugov:4 pid=491 prio=-1 target_cpu=007
>>>  [007] 154.892214: ipi_exit: (Function call interrupts)
>>>
>>> This impacts performance and the above count increments.
>>>
>>> commit ccde6525183c ("smp: Introduce a helper function to check for pending
>>> IPIs") already introduced a helper function to check the pending IPIs and
>>> it is used in pmdomain governor to deny the cluster level idle state when
>>> there is a pending IPI on any of cluster CPUs.
>>>
>>> This however does not stop CPU to enter CPU level idle state. Make use of
>>> same at CPUidle to deny the idle entry when there is already IPI pending.
>>>
>>> With change observing glmark2 [1] off screen scores improving in the range
>>> of 25% to 30% on Qualcomm lemans-evk board which is arm64 based having two
>>> clusters each with 4 CPUs.
>>>
>>> [1] https://github.com/glmark2/glmark2
>>>
>>> Signed-off-by: Maulik Shah <[email protected]>
>>> ---
>>> Changes in v2:
>>> - Fix cpumask argument of cpus_peek_for_pending_ipi() to take single cpu
>>> - Link to v1: https://lore.kernel.org/r/[email protected]
>>> ---
>>>  drivers/cpuidle/cpuidle.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>>> index c7876e9e024f9076663063ad21cfc69343fdbbe7..c01e57df64ca5af8c28da3d971500b3f38306cdf 100644
>>> --- a/drivers/cpuidle/cpuidle.c
>>> +++ b/drivers/cpuidle/cpuidle.c
>>> @@ -224,6 +224,9 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>>>         bool broadcast = !!(target_state->flags & CPUIDLE_FLAG_TIMER_STOP);
>>>         ktime_t time_start, time_end;
>>>
>>> +       if (cpus_peek_for_pending_ipi(cpumask_of(dev->cpu)))
>>> +               return -EBUSY;
>>> +
>>
>> Why do you want to check it here and not in cpuidle_idle_call(), for example?
> 
> It can be moved in cpuidle_idle_call(), just before call_cpuidle() too.
> The intention is to check after cpuidle_select() is done.
> 
What do we do about the cpuidle stats in that case?
I'm thinking primiarly about last_residency_ns and rejected here.