diff mbox series

[7/7] cpuidle: Handle TIF_NR_POLLING on behalf of software polling idle states

Message ID 20231124223226.24249-8-frederic@kernel.org
State New
Headers show
Series cpuidle: Handle TIF_NR_POLLING on behalf of polling idle states | expand

Commit Message

Frederic Weisbecker Nov. 24, 2023, 10:32 p.m. UTC
Software polling idle states set again TIF_NR_POLLING and clear it upon
exit. This involves error prone duplicated code and wasted cycles
performing atomic operations, sometimes RmW fully ordered.

To avoid this, benefit instead from the same generic TIF_NR_POLLING
handling that is currently in use for hardware polling states.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/cpuidle/cpuidle-powernv.c | 10 ----------
 drivers/cpuidle/cpuidle-pseries.c | 11 -----------
 drivers/cpuidle/cpuidle.c         |  4 ++--
 drivers/cpuidle/poll_state.c      | 30 ++++++++++++------------------
 4 files changed, 14 insertions(+), 41 deletions(-)

Comments

Rafael J. Wysocki Dec. 12, 2023, 1:27 p.m. UTC | #1
On Fri, Nov 24, 2023 at 11:33 PM Frederic Weisbecker
<frederic@kernel.org> wrote:
>
> Software polling idle states set again TIF_NR_POLLING and clear it upon
> exit. This involves error prone duplicated code and wasted cycles
> performing atomic operations, sometimes RmW fully ordered.
>
> To avoid this, benefit instead from the same generic TIF_NR_POLLING
> handling that is currently in use for hardware polling states.
>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  drivers/cpuidle/cpuidle-powernv.c | 10 ----------
>  drivers/cpuidle/cpuidle-pseries.c | 11 -----------
>  drivers/cpuidle/cpuidle.c         |  4 ++--
>  drivers/cpuidle/poll_state.c      | 30 ++++++++++++------------------
>  4 files changed, 14 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 675b8eb81ebd..b88bbf7ead41 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -71,8 +71,6 @@ static int snooze_loop(struct cpuidle_device *dev,
>  {
>         u64 snooze_exit_time;
>
> -       set_thread_flag(TIF_POLLING_NRFLAG);
> -
>         local_irq_enable();
>
>         snooze_exit_time = get_tb() + get_snooze_timeout(dev, drv, index);
> @@ -81,21 +79,13 @@ static int snooze_loop(struct cpuidle_device *dev,
>         HMT_very_low();
>         while (!need_resched()) {
>                 if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) {
> -                       /*
> -                        * Task has not woken up but we are exiting the polling
> -                        * loop anyway. Require a barrier after polling is
> -                        * cleared to order subsequent test of need_resched().
> -                        */
> -                       clear_thread_flag(TIF_POLLING_NRFLAG);
>                         dev->poll_time_limit = true;
> -                       smp_mb();
>                         break;
>                 }
>         }
>
>         HMT_medium();
>         ppc64_runlatch_on();
> -       clear_thread_flag(TIF_POLLING_NRFLAG);
>
>         local_irq_disable();
>
> diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> index 4e08c9a39172..0ae76512b740 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -39,8 +39,6 @@ int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>  {
>         u64 snooze_exit_time;
>
> -       set_thread_flag(TIF_POLLING_NRFLAG);
> -
>         pseries_idle_prolog();
>         raw_local_irq_enable();
>         snooze_exit_time = get_tb() + snooze_timeout;
> @@ -50,21 +48,12 @@ int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>                 HMT_low();
>                 HMT_very_low();
>                 if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) {
> -                       /*
> -                        * Task has not woken up but we are exiting the polling
> -                        * loop anyway. Require a barrier after polling is
> -                        * cleared to order subsequent test of need_resched().
> -                        */
>                         dev->poll_time_limit = true;
> -                       clear_thread_flag(TIF_POLLING_NRFLAG);
> -                       smp_mb();
>                         break;
>                 }
>         }
>
>         HMT_medium();
> -       clear_thread_flag(TIF_POLLING_NRFLAG);
> -
>         raw_local_irq_disable();
>
>         pseries_idle_epilog();
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 49078cc83f4a..9eb811b5d8b6 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -236,8 +236,8 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
>                 broadcast = false;
>         }
>
> -       polling = target_state->flags & CPUIDLE_FLAG_POLLING_HARD;
> -
> +       polling = (target_state->flags & (CPUIDLE_FLAG_POLLING_SOFT |
> +                                         CPUIDLE_FLAG_POLLING_HARD));

The outer parens are not needed on the right-hand side, or apply !! to it.

>         /*
>          * If the target state doesn't poll on need_resched(), this is
>          * the last check after which further TIF_NEED_RESCHED remote setting
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index a2fe173de117..3bfa251b344a 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -13,35 +13,29 @@
>  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_start = local_clock_noinstr();
> +       unsigned int loop_count = 0;
> +       u64 limit;
>
>         dev->poll_time_limit = false;
>
>         raw_local_irq_enable();
> -       if (!current_set_polling_and_test()) {
> -               unsigned int loop_count = 0;
> -               u64 limit;
>
> -               limit = cpuidle_poll_time(drv, dev);
> +       limit = cpuidle_poll_time(drv, dev);
>
> -               while (!need_resched()) {
> -                       cpu_relax();
> -                       if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> -                               continue;
> +       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;
> -                       }
> +               loop_count = 0;
> +               if (local_clock_noinstr() - time_start > limit) {
> +                       dev->poll_time_limit = true;
> +                       break;
>                 }
>         }
>         raw_local_irq_disable();
>
> -       current_clr_polling();
> -
>         return index;
>  }
>
> --
diff mbox series

Patch

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 675b8eb81ebd..b88bbf7ead41 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -71,8 +71,6 @@  static int snooze_loop(struct cpuidle_device *dev,
 {
 	u64 snooze_exit_time;
 
-	set_thread_flag(TIF_POLLING_NRFLAG);
-
 	local_irq_enable();
 
 	snooze_exit_time = get_tb() + get_snooze_timeout(dev, drv, index);
@@ -81,21 +79,13 @@  static int snooze_loop(struct cpuidle_device *dev,
 	HMT_very_low();
 	while (!need_resched()) {
 		if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) {
-			/*
-			 * Task has not woken up but we are exiting the polling
-			 * loop anyway. Require a barrier after polling is
-			 * cleared to order subsequent test of need_resched().
-			 */
-			clear_thread_flag(TIF_POLLING_NRFLAG);
 			dev->poll_time_limit = true;
-			smp_mb();
 			break;
 		}
 	}
 
 	HMT_medium();
 	ppc64_runlatch_on();
-	clear_thread_flag(TIF_POLLING_NRFLAG);
 
 	local_irq_disable();
 
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 4e08c9a39172..0ae76512b740 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -39,8 +39,6 @@  int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv,
 {
 	u64 snooze_exit_time;
 
-	set_thread_flag(TIF_POLLING_NRFLAG);
-
 	pseries_idle_prolog();
 	raw_local_irq_enable();
 	snooze_exit_time = get_tb() + snooze_timeout;
@@ -50,21 +48,12 @@  int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv,
 		HMT_low();
 		HMT_very_low();
 		if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) {
-			/*
-			 * Task has not woken up but we are exiting the polling
-			 * loop anyway. Require a barrier after polling is
-			 * cleared to order subsequent test of need_resched().
-			 */
 			dev->poll_time_limit = true;
-			clear_thread_flag(TIF_POLLING_NRFLAG);
-			smp_mb();
 			break;
 		}
 	}
 
 	HMT_medium();
-	clear_thread_flag(TIF_POLLING_NRFLAG);
-
 	raw_local_irq_disable();
 
 	pseries_idle_epilog();
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 49078cc83f4a..9eb811b5d8b6 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -236,8 +236,8 @@  noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
 		broadcast = false;
 	}
 
-	polling = target_state->flags & CPUIDLE_FLAG_POLLING_HARD;
-
+	polling = (target_state->flags & (CPUIDLE_FLAG_POLLING_SOFT |
+					  CPUIDLE_FLAG_POLLING_HARD));
 	/*
 	 * If the target state doesn't poll on need_resched(), this is
 	 * the last check after which further TIF_NEED_RESCHED remote setting
diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index a2fe173de117..3bfa251b344a 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -13,35 +13,29 @@ 
 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_start = local_clock_noinstr();
+	unsigned int loop_count = 0;
+	u64 limit;
 
 	dev->poll_time_limit = false;
 
 	raw_local_irq_enable();
-	if (!current_set_polling_and_test()) {
-		unsigned int loop_count = 0;
-		u64 limit;
 
-		limit = cpuidle_poll_time(drv, dev);
+	limit = cpuidle_poll_time(drv, dev);
 
-		while (!need_resched()) {
-			cpu_relax();
-			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
-				continue;
+	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;
-			}
+		loop_count = 0;
+		if (local_clock_noinstr() - time_start > limit) {
+			dev->poll_time_limit = true;
+			break;
 		}
 	}
 	raw_local_irq_disable();
 
-	current_clr_polling();
-
 	return index;
 }