[PATCH v7 7/7] cpuidle/poll_state: Poll via smp_cond_load_relaxed_timeout()

Ankur Arora posted 7 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v7 7/7] cpuidle/poll_state: Poll via smp_cond_load_relaxed_timeout()
Posted by Ankur Arora 3 months, 3 weeks ago
The inner loop in poll_idle() polls over the thread_info flags,
waiting to see if the thread has TIF_NEED_RESCHED set. The loop
exits once the condition is met, or if the poll time limit has
been exceeded.

To minimize the number of instructions executed in each iteration,
the time check is done only intermittently (once every
POLL_IDLE_RELAX_COUNT iterations). In addition, each loop iteration
executes cpu_relax() which on certain platforms provides a hint to
the pipeline that the loop busy-waits, allowing the processor to
reduce power consumption.

This is close to what smp_cond_load_relaxed_timeout() provides. So,
restructure the loop and fold the loop condition and the timeout check
in smp_cond_load_relaxed_timeout().

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 drivers/cpuidle/poll_state.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index 9b6d90a72601..dc7f4b424fec 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -8,35 +8,22 @@
 #include <linux/sched/clock.h>
 #include <linux/sched/idle.h>
 
-#define POLL_IDLE_RELAX_COUNT	200
-
 static int __cpuidle poll_idle(struct cpuidle_device *dev,
 			       struct cpuidle_driver *drv, int index)
 {
-	u64 time_start;
-
-	time_start = local_clock_noinstr();
+	u64 time_end;
+	u32 flags = 0;
 
 	dev->poll_time_limit = false;
 
+	time_end = local_clock_noinstr() + cpuidle_poll_time(drv, dev);
+
 	raw_local_irq_enable();
 	if (!current_set_polling_and_test()) {
-		unsigned int loop_count = 0;
-		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;
-			}
-		}
+		flags = smp_cond_load_relaxed_timeout(&current_thread_info()->flags,
+						      (VAL & _TIF_NEED_RESCHED),
+						      (local_clock_noinstr() >= time_end));
+		dev->poll_time_limit = !(flags & _TIF_NEED_RESCHED);
 	}
 	raw_local_irq_disable();
 
-- 
2.43.5
Re: [PATCH v7 7/7] cpuidle/poll_state: Poll via smp_cond_load_relaxed_timeout()
Posted by Ankur Arora 3 months, 2 weeks ago
Hi Rafael, Daniel,

Could you take a look at the proposed smp_cond_load_relaxed_timeout()
interface and how it's used here and see if that looks fine to you?


Thanks
Ankur

Ankur Arora <ankur.a.arora@oracle.com> writes:

> The inner loop in poll_idle() polls over the thread_info flags,
> waiting to see if the thread has TIF_NEED_RESCHED set. The loop
> exits once the condition is met, or if the poll time limit has
> been exceeded.
>
> To minimize the number of instructions executed in each iteration,
> the time check is done only intermittently (once every
> POLL_IDLE_RELAX_COUNT iterations). In addition, each loop iteration
> executes cpu_relax() which on certain platforms provides a hint to
> the pipeline that the loop busy-waits, allowing the processor to
> reduce power consumption.
>
> This is close to what smp_cond_load_relaxed_timeout() provides. So,
> restructure the loop and fold the loop condition and the timeout check
> in smp_cond_load_relaxed_timeout().
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> ---
>  drivers/cpuidle/poll_state.c | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index 9b6d90a72601..dc7f4b424fec 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -8,35 +8,22 @@
>  #include <linux/sched/clock.h>
>  #include <linux/sched/idle.h>
>
> -#define POLL_IDLE_RELAX_COUNT	200
> -
>  static int __cpuidle poll_idle(struct cpuidle_device *dev,
>  			       struct cpuidle_driver *drv, int index)
>  {
> -	u64 time_start;
> -
> -	time_start = local_clock_noinstr();
> +	u64 time_end;
> +	u32 flags = 0;
>
>  	dev->poll_time_limit = false;
>
> +	time_end = local_clock_noinstr() + cpuidle_poll_time(drv, dev);
> +
>  	raw_local_irq_enable();
>  	if (!current_set_polling_and_test()) {
> -		unsigned int loop_count = 0;
> -		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;
> -			}
> -		}
> +		flags = smp_cond_load_relaxed_timeout(&current_thread_info()->flags,
> +						      (VAL & _TIF_NEED_RESCHED),
> +						      (local_clock_noinstr() >= time_end));
> +		dev->poll_time_limit = !(flags & _TIF_NEED_RESCHED);
>  	}
>  	raw_local_irq_disable();


--
ankur
Re: [PATCH v7 7/7] cpuidle/poll_state: Poll via smp_cond_load_relaxed_timeout()
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
On Mon, Oct 27, 2025 at 9:07 PM Ankur Arora <ankur.a.arora@oracle.com> wrote:
>
>
> Hi Rafael, Daniel,
>
> Could you take a look at the proposed smp_cond_load_relaxed_timeout()
> interface and how it's used here and see if that looks fine to you?

Well, can you please resend this with a CC to linux-pm?

I believe others on that list may be interested in it too.

Thanks!
Re: [PATCH v7 7/7] cpuidle/poll_state: Poll via smp_cond_load_relaxed_timeout()
Posted by Ankur Arora 3 months, 1 week ago
Rafael J. Wysocki <rafael@kernel.org> writes:

> On Mon, Oct 27, 2025 at 9:07 PM Ankur Arora <ankur.a.arora@oracle.com> wrote:
>>
>>
>> Hi Rafael, Daniel,
>>
>> Could you take a look at the proposed smp_cond_load_relaxed_timeout()
>> interface and how it's used here and see if that looks fine to you?
>
> Well, can you please resend this with a CC to linux-pm?

Done.

Thanks
--
ankur