Re: [PATCH v15 12/16] cpuidle/poll_state: Wait for need-resched via tif_need_resched_relaxed_wait()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The timeout for the idle polling loop is erroneously extended because the generic wait macro calculates the timeout start time only after the first 200 spin iterations. Additionally, the commit relies on an architecture-specific optimization that does not actually exist on the target platform.
--

commit 9b952e08004e7de674a695400c7ab21ff69df4db
Author: Ankur Arora <ankur.a.arora@oracle.com>

cpuidle/poll_state: Wait for need-resched via tif_need_resched_relaxed_wait()

This commit replaces the manual polling loop in poll_idle() with a call to
tif_need_resched_relaxed_wait() to optimize idle power consumption.

> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index c7524e4c522a2..7443b3e971baf 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
[ ... ]
> @@ -21,15 +20,7 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>  		u64 limit;
>  
>  		limit = cpuidle_poll_time(drv, dev);
>  
> -		while (!need_resched()) {
> -			cpu_relax();
> -			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> -				continue;
> -
> -			loop_count = 0;
> -			if (local_clock_noinstr() - time_start > limit) {
> -				dev->poll_time_limit = true;
> -				break;
> -			}
> -		}
> +		dev->poll_time_limit = !tif_need_resched_relaxed_wait(limit);

[Severity: Medium]
Does this unintentionally extend the polling timeout?

When tif_need_resched_relaxed_wait() uses the generic
__smp_cond_load_relaxed_timeout() macro, it first spins for 200 iterations
before capturing the start time for the timeout:

include/asm-generic/barrier.h:__smp_cond_load_relaxed_timeout() {
    ...
    cpu_poll_relax(__PTR, VAL, (u64)__scl_timeout); \
    if (++__scl_count < __scl_spin)                 \
            continue;                               \
    __scl_time_now = (s64)(time_expr_ns);           \
    if (unlikely(__scl_time_end == 0)) {            \
            __scl_timeout = (s64)(timeout_ns);      \
            __scl_time_end = __scl_time_now + __scl_timeout; \
    ...
}

Since the timeout calculation is deferred until after the initial spin loop,
the total wait time becomes the requested limit plus the duration of those
first 200 iterations.

On platforms where 200 cpu_relax() iterations can take a significant amount
of time (e.g. ~14 microseconds on modern x86), could this effectively delay
entry into deeper C-states and cause a power regression for short polling
intervals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831202251.305046-1-ankur.a.arora@oracle.com?part=12