[PATCH 0/3] powerpc (powernv and pseries) cpuidle driver improvmeents

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/3] powerpc (powernv and pseries) cpuidle driver improvmeents

Nicholas Piggin-2
Hi,

These are a few small improvements that came from doing an
optimisation pass over powerpc cpu idle paths.

Michael reminded me to cc the cpuidle maintainers. I think he
will take the patches through the powerpc tree, but any suggestion
or ack or nack would be welcome.

Thanks,
Nick

Nicholas Piggin (3):
  cpuidle: powerpc: cpuidle set polling before enabling irqs
  cpuidle: powerpc: read mostly for common globals
  cpuidle: powerpc: no memory barrier after break from idle

 drivers/cpuidle/cpuidle-powernv.c | 25 +++++++++++++++++--------
 drivers/cpuidle/cpuidle-pseries.c | 22 +++++++++++++++-------
 2 files changed, 32 insertions(+), 15 deletions(-)

--
2.11.0

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/3] cpuidle: powerpc: cpuidle set polling before enabling irqs

Nicholas Piggin-2
local_irq_enable can cause interrupts to be taken which could
take significant amount of processing time. The idle process
should set its polling flag before this, so another process that
wakes it during this time will not have to send an IPI.

Expand the TIF_POLLING_NRFLAG coverage to as large as possible.

Reviewed-by: Gautham R. Shenoy <[hidden email]>
Signed-off-by: Nicholas Piggin <[hidden email]>
---
 drivers/cpuidle/cpuidle-powernv.c | 4 +++-
 drivers/cpuidle/cpuidle-pseries.c | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 45eaf06462ae..77bc50ad9f57 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -51,9 +51,10 @@ static int snooze_loop(struct cpuidle_device *dev,
 {
  u64 snooze_exit_time;
 
- local_irq_enable();
  set_thread_flag(TIF_POLLING_NRFLAG);
 
+ local_irq_enable();
+
  snooze_exit_time = get_tb() + snooze_timeout;
  ppc64_runlatch_off();
  HMT_very_low();
@@ -66,6 +67,7 @@ static int snooze_loop(struct cpuidle_device *dev,
  ppc64_runlatch_on();
  clear_thread_flag(TIF_POLLING_NRFLAG);
  smp_mb();
+
  return index;
 }
 
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 166ccd711ec9..7b12bb2ea70f 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -62,9 +62,10 @@ static int snooze_loop(struct cpuidle_device *dev,
  unsigned long in_purr;
  u64 snooze_exit_time;
 
+ set_thread_flag(TIF_POLLING_NRFLAG);
+
  idle_loop_prolog(&in_purr);
  local_irq_enable();
- set_thread_flag(TIF_POLLING_NRFLAG);
  snooze_exit_time = get_tb() + snooze_timeout;
 
  while (!need_resched()) {
--
2.11.0

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/3] cpuidle: powerpc: read mostly for common globals

Nicholas Piggin-2
In reply to this post by Nicholas Piggin-2
Ensure these don't get put into bouncing cachelines.

Reviewed-by: Vaidyanathan Srinivasan <[hidden email]>
Reviewed-by: Gautham R. Shenoy <[hidden email]>
Signed-off-by: Nicholas Piggin <[hidden email]>
---
 drivers/cpuidle/cpuidle-powernv.c | 10 +++++-----
 drivers/cpuidle/cpuidle-pseries.c |  8 ++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 77bc50ad9f57..abf2ffcd4a0a 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -32,18 +32,18 @@ static struct cpuidle_driver powernv_idle_driver = {
  .owner            = THIS_MODULE,
 };
 
-static int max_idle_state;
-static struct cpuidle_state *cpuidle_state_table;
+static int max_idle_state __read_mostly;
+static struct cpuidle_state *cpuidle_state_table __read_mostly;
 
 struct stop_psscr_table {
  u64 val;
  u64 mask;
 };
 
-static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX];
+static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX] __read_mostly;
 
-static u64 snooze_timeout;
-static bool snooze_timeout_en;
+static u64 snooze_timeout __read_mostly;
+static bool snooze_timeout_en __read_mostly;
 
 static int snooze_loop(struct cpuidle_device *dev,
  struct cpuidle_driver *drv,
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 7b12bb2ea70f..a404f352d284 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -25,10 +25,10 @@ struct cpuidle_driver pseries_idle_driver = {
  .owner            = THIS_MODULE,
 };
 
-static int max_idle_state;
-static struct cpuidle_state *cpuidle_state_table;
-static u64 snooze_timeout;
-static bool snooze_timeout_en;
+static int max_idle_state __read_mostly;
+static struct cpuidle_state *cpuidle_state_table __read_mostly;
+static u64 snooze_timeout __read_mostly;
+static bool snooze_timeout_en __read_mostly;
 
 static inline void idle_loop_prolog(unsigned long *in_purr)
 {
--
2.11.0

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 3/3] cpuidle: powerpc: no memory barrier after break from idle

Nicholas Piggin-2
In reply to this post by Nicholas Piggin-2
A memory barrier is not required after the task wakes up,
only if we clear the polling flag before waking. The case
where we have work to do is the important one, so optimise
for it.

Reviewed-by: Vaidyanathan Srinivasan <[hidden email]>
Signed-off-by: Nicholas Piggin <[hidden email]>
---
 drivers/cpuidle/cpuidle-powernv.c | 11 +++++++++--
 drivers/cpuidle/cpuidle-pseries.c | 11 +++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index abf2ffcd4a0a..722b81b03593 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -59,14 +59,21 @@ static int snooze_loop(struct cpuidle_device *dev,
  ppc64_runlatch_off();
  HMT_very_low();
  while (!need_resched()) {
- if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time)
+ 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);
+ smp_mb();
  break;
+ }
  }
 
  HMT_medium();
  ppc64_runlatch_on();
  clear_thread_flag(TIF_POLLING_NRFLAG);
- smp_mb();
 
  return index;
 }
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index a404f352d284..e9b3853d93ea 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -71,13 +71,20 @@ static int snooze_loop(struct cpuidle_device *dev,
  while (!need_resched()) {
  HMT_low();
  HMT_very_low();
- if (snooze_timeout_en && get_tb() > snooze_exit_time)
+ 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);
+ smp_mb();
  break;
+ }
  }
 
  HMT_medium();
  clear_thread_flag(TIF_POLLING_NRFLAG);
- smp_mb();
 
  idle_loop_epilog(in_purr);
 
--
2.11.0

Loading...