Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1718662 > unrolled thread

[PATCH 0/3] cpuidle: Rework the handling of the poll state

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] cpuidle: Rework the handling of the poll state "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-23 23:40 +0200
    [PATCH 2/3] cpuidle: Move polling state initialization code to separate file "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-23 23:40 +0200
    [PATCH 3/3] cpuidle: Make drivers initialize polling state "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-23 23:40 +0200
    Re: [PATCH 0/3] cpuidle: Rework the handling of the poll state Sudeep Holla <sudeep.holla@arm.com> - 2017-08-24 11:50 +0200

#1718662 — [PATCH 0/3] cpuidle: Rework the handling of the poll state

Hi,

On x86 the fist idle state is a polling one, but the way it is set up is far
from straightforward and then it is avoided by governors in rather somewhat
convoluted fashion.

Make this more clear by explicitly flagging that state as "polling" and
checking its flag where it needs to be avoided instead of using
arch-dependent numbering of idle states (patch [1/3]), move the
polling state code from driver.c to a separate C file (patch [2/3]) and
move the initialization of it from the core to the relevant cpuidle drivers -
ACPI and intel_idle (patch [3/3]).

Thanks,
Rafael

[toc] | [next] | [standalone]

#1718663 — [PATCH 2/3] cpuidle: Move polling state initialization code to separate file

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move the polling state initialization code to a separate file built
conditionally on CONFIG_ARCH_HAS_CPU_RELAX to get rid of the #ifdef
in driver.c.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/Makefile     |    1 +
 drivers/cpuidle/driver.c     |   31 -------------------------------
 drivers/cpuidle/poll_state.c |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/cpuidle.h      |    6 ++++++
 4 files changed, 43 insertions(+), 31 deletions(-)

Index: linux-pm/drivers/cpuidle/Makefile
===================================================================
--- linux-pm.orig/drivers/cpuidle/Makefile
+++ linux-pm/drivers/cpuidle/Makefile
@@ -5,6 +5,7 @@
 obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
 obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
 obj-$(CONFIG_DT_IDLE_STATES)		  += dt_idle_states.o
+obj-$(CONFIG_ARCH_HAS_CPU_RELAX)	  += poll_state.o
 
 ##################################################################################
 # ARM SoC drivers
Index: linux-pm/drivers/cpuidle/poll_state.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/cpuidle/poll_state.c
@@ -0,0 +1,36 @@
+/*
+ * poll_state.c - Polling idle state
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/sched.h>
+#include <linux/sched/idle.h>
+
+static int __cpuidle poll_idle(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv, int index)
+{
+	local_irq_enable();
+	if (!current_set_polling_and_test()) {
+		while (!need_resched())
+			cpu_relax();
+	}
+	current_clr_polling();
+
+	return index;
+}
+
+void poll_idle_init(struct cpuidle_driver *drv)
+{
+	struct cpuidle_state *state = &drv->states[0];
+
+	snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
+	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
+	state->exit_latency = 0;
+	state->target_residency = 0;
+	state->power_usage = -1;
+	state->enter = poll_idle;
+	state->disabled = false;
+	state->flags = CPUIDLE_FLAG_POLLING;
+}
Index: linux-pm/include/linux/cpuidle.h
===================================================================
--- linux-pm.orig/include/linux/cpuidle.h
+++ linux-pm/include/linux/cpuidle.h
@@ -225,6 +225,12 @@ static inline void cpuidle_coupled_paral
 }
 #endif
 
+#ifdef CONFIG_ARCH_HAS_CPU_RELAX
+void poll_idle_init(struct cpuidle_driver *drv);
+#else
+static void poll_idle_init(struct cpuidle_driver *drv) {}
+#endif
+
 /******************************
  * CPUIDLE GOVERNOR INTERFACE *
  ******************************/
Index: linux-pm/drivers/cpuidle/driver.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/driver.c
+++ linux-pm/drivers/cpuidle/driver.c
@@ -179,37 +179,6 @@ static void __cpuidle_driver_init(struct
 	}
 }
 
-#ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int __cpuidle poll_idle(struct cpuidle_device *dev,
-			       struct cpuidle_driver *drv, int index)
-{
-	local_irq_enable();
-	if (!current_set_polling_and_test()) {
-		while (!need_resched())
-			cpu_relax();
-	}
-	current_clr_polling();
-
-	return index;
-}
-
-static void poll_idle_init(struct cpuidle_driver *drv)
-{
-	struct cpuidle_state *state = &drv->states[0];
-
-	snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
-	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
-	state->exit_latency = 0;
-	state->target_residency = 0;
-	state->power_usage = -1;
-	state->enter = poll_idle;
-	state->disabled = false;
-	state->flags = CPUIDLE_FLAG_POLLING;
-}
-#else
-static void poll_idle_init(struct cpuidle_driver *drv) {}
-#endif /* !CONFIG_ARCH_HAS_CPU_RELAX */
-
 /**
  * __cpuidle_register_driver: register the driver
  * @drv: a valid pointer to a struct cpuidle_driver

[toc] | [prev] | [next] | [standalone]

#1718665 — [PATCH 3/3] cpuidle: Make drivers initialize polling state

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make the drivers that want to include the polling state into their
states table initialize it explicitly and drop the initialization of
it (which in fact is conditional, but that is not obvious from the
code) from the core.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |    9 ++++++++-
 drivers/cpuidle/driver.c      |    2 --
 drivers/cpuidle/poll_state.c  |    3 ++-
 drivers/idle/intel_idle.c     |    1 +
 include/linux/cpuidle.h       |    4 +---
 5 files changed, 12 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -842,7 +842,7 @@ static int acpi_processor_setup_cpuidle_
 
 static int acpi_processor_setup_cstates(struct acpi_processor *pr)
 {
-	int i, count = ACPI_IDLE_STATE_START;
+	int i, count;
 	struct acpi_processor_cx *cx;
 	struct cpuidle_state *state;
 	struct cpuidle_driver *drv = &acpi_idle_driver;
@@ -850,6 +850,13 @@ static int acpi_processor_setup_cstates(
 	if (max_cstate == 0)
 		max_cstate = 1;
 
+	if (IS_ENABLED(CONFIG_ARCH_HAS_CPU_RELAX)) {
+		cpuidle_poll_state_init(drv);
+		count = 1;
+	} else {
+		count = 0;
+	}
+
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		cx = &pr->power.states[i];
 
Index: linux-pm/drivers/cpuidle/driver.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/driver.c
+++ linux-pm/drivers/cpuidle/driver.c
@@ -216,8 +216,6 @@ static int __cpuidle_register_driver(str
 		on_each_cpu_mask(drv->cpumask, cpuidle_setup_broadcast_timer,
 				 (void *)1, 1);
 
-	poll_idle_init(drv);
-
 	return 0;
 }
 
Index: linux-pm/drivers/cpuidle/poll_state.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/poll_state.c
+++ linux-pm/drivers/cpuidle/poll_state.c
@@ -21,7 +21,7 @@ static int __cpuidle poll_idle(struct cp
 	return index;
 }
 
-void poll_idle_init(struct cpuidle_driver *drv)
+void cpuidle_poll_state_init(struct cpuidle_driver *drv)
 {
 	struct cpuidle_state *state = &drv->states[0];
 
@@ -34,3 +34,4 @@ void poll_idle_init(struct cpuidle_drive
 	state->disabled = false;
 	state->flags = CPUIDLE_FLAG_POLLING;
 }
+EXPORT_SYMBOL_GPL(cpuidle_poll_state_init);
Index: linux-pm/include/linux/cpuidle.h
===================================================================
--- linux-pm.orig/include/linux/cpuidle.h
+++ linux-pm/include/linux/cpuidle.h
@@ -226,9 +226,7 @@ static inline void cpuidle_coupled_paral
 #endif
 
 #ifdef CONFIG_ARCH_HAS_CPU_RELAX
-void poll_idle_init(struct cpuidle_driver *drv);
-#else
-static void poll_idle_init(struct cpuidle_driver *drv) {}
+void cpuidle_poll_state_init(struct cpuidle_driver *drv);
 #endif
 
 /******************************
Index: linux-pm/drivers/idle/intel_idle.c
===================================================================
--- linux-pm.orig/drivers/idle/intel_idle.c
+++ linux-pm/drivers/idle/intel_idle.c
@@ -1331,6 +1331,7 @@ static void __init intel_idle_cpuidle_dr
 
 	intel_idle_state_table_update();
 
+	cpuidle_poll_state_init(drv);
 	drv->state_count = 1;
 
 	for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {

[toc] | [prev] | [next] | [standalone]

#1719072

On 23/08/17 22:18, Rafael J. Wysocki wrote:
> Hi,
> 
> On x86 the fist idle state is a polling one, but the way it is set up is far
> from straightforward and then it is avoided by governors in rather somewhat
> convoluted fashion.
> 
> Make this more clear by explicitly flagging that state as "polling" and
> checking its flag where it needs to be avoided instead of using
> arch-dependent numbering of idle states (patch [1/3]), move the
> polling state code from driver.c to a separate C file (patch [2/3]) and
> move the initialization of it from the core to the relevant cpuidle drivers -
> ACPI and intel_idle (patch [3/3]).
> 

Tested this on ARM64 platform(both DT and ACPI/LPI) and everything
continues to work fine.
Tested-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web