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

Groups > linux.kernel > #1366276 > unrolled thread

[PATCH 00/10] intel_idle: Fix hot plug handling.

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/10] intel_idle: Fix hot plug handling. Richard Cochran <rcochran@linutronix.de> - 2016-03-29 15:20 +0200
    [PATCH 09/10] intel_idle: Propagate hot plug errors. Richard Cochran <rcochran@linutronix.de> - 2016-03-29 15:20 +0200
    [PATCH 02/10] intel_idle: Fix a helper function's return value. Richard Cochran <rcochran@linutronix.de> - 2016-03-29 15:20 +0200
    [PATCH 01/10] intel_idle: remove useless return from void function. Richard Cochran <rcochran@linutronix.de> - 2016-03-29 15:20 +0200
    [PATCH 04/10] intel_idle: Fix deallocation order on the driver exit path. Richard Cochran <rcochran@linutronix.de> - 2016-03-29 15:20 +0200

#1366276 — [PATCH 00/10] intel_idle: Fix hot plug handling.

This driver has one serious and one mild bug in its hot plug handling.

First, whenever a new CPU goes on line, if the call to
cpuidle_register_driver() should fail (say, due to lack of memory),
then the driver frees its per-CPU region.  On the *next* CPU_ONLINE
event, the driver will happily use the region again and even free it
again if the failure repeats.

Second, for each new on line CPU, a device is registered with the
cpuidle layer.  However, when a CPU goes down, its device is never
unregistered, even if the module exits.

Although this driver may not (yet?) be a built as a module, still this
patch series cleans up the exit path in order to make the resource
allocations clear.


Richard Cochran (10):
  intel_idle: remove useless return from void function.
  intel_idle: Fix a helper function's return value.
  intel_idle: Remove redundant initialization calls.
  intel_idle: Fix deallocation order on the driver exit path.
  intel_idle: Fix dangling registration on error path.
  intel_idle: Avoid a double free of the per-CPU data.
  intel_idle: Setup the timer broadcast only on successful driver load.
  intel_idle: Don't overreact to a cpuidle registration failure.
  intel_idle: Propagate hot plug errors.
  intel_idle: Clean up all registered devices on exit.

 drivers/idle/intel_idle.c | 61 ++++++++++++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 30 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1366278 — [PATCH 09/10] intel_idle: Propagate hot plug errors.

If a cpuidle registration error occurs during the hot plug notifier
callback, we should really inform the hot plug machinery instead of
just ignoring the error.  This patch changes the callback to properly
return on error.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Richard Cochran <rcochran@linutronix.de>
---
 drivers/idle/intel_idle.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 4418cfa..8420ba1 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -818,8 +818,11 @@ static int cpu_hotplug_notify(struct notifier_block *n,
 		 * driver in this case
 		 */
 		dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
-		if (!dev->registered)
-			intel_idle_cpu_init(hotcpu);
+		if (dev->registered)
+			break;
+
+		if (intel_idle_cpu_init(hotcpu))
+			return NOTIFY_BAD;
 
 		break;
 	}
-- 
2.1.4

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

#1366279 — [PATCH 02/10] intel_idle: Fix a helper function's return value.

The function, intel_idle_cpuidle_driver_init, delivers no error codes
at all.  This patch changes the function to return 'void' instead of
returning zero.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Richard Cochran <rcochran@linutronix.de>
---
 drivers/idle/intel_idle.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 9d5ed32..dfa9055 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1111,7 +1111,7 @@ static void intel_idle_state_table_update(void)
  * intel_idle_cpuidle_driver_init()
  * allocate, initialize cpuidle_states
  */
-static int __init intel_idle_cpuidle_driver_init(void)
+static void __init intel_idle_cpuidle_driver_init(void)
 {
 	int cstate;
 	struct cpuidle_driver *drv = &intel_idle_driver;
@@ -1173,8 +1173,6 @@ static int __init intel_idle_cpuidle_driver_init(void)
 
 	if (icpu->disable_promotion_to_c1e)	/* each-cpu is redundant */
 		on_each_cpu(c1e_promotion_disable, NULL, 1);
-
-	return 0;
 }
 
 
-- 
2.1.4

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

#1366280 — [PATCH 01/10] intel_idle: remove useless return from void function.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Richard Cochran <rcochran@linutronix.de>
---
 drivers/idle/intel_idle.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index ba947df..9d5ed32 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1260,8 +1260,6 @@ static void __exit intel_idle_exit(void)
 	__unregister_cpu_notifier(&cpu_hotplug_notifier);
 
 	cpu_notifier_register_done();
-
-	return;
 }
 
 module_init(intel_idle_init);
-- 
2.1.4

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

#1366281 — [PATCH 04/10] intel_idle: Fix deallocation order on the driver exit path.

In the module_exit() method, this driver first frees its per-CPU
pointer, then unregisters a callback making use of the pointer.
Furthermore, the function, intel_idle_cpuidle_devices_uninit, is racy
against CPU hot plugging as it calls for_each_online_cpu().

This patch corrects the issues by unregistering first on the exit path
while holding the hot plug lock.

Cc: Len Brown <lenb@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Richard Cochran <rcochran@linutronix.de>
---
 drivers/idle/intel_idle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index cb85c4c..4a1de3d 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1242,16 +1242,16 @@ static int __init intel_idle_init(void)
 
 static void __exit intel_idle_exit(void)
 {
-	intel_idle_cpuidle_devices_uninit();
-	cpuidle_unregister_driver(&intel_idle_driver);
-
 	cpu_notifier_register_begin();
 
 	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
 		on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
 	__unregister_cpu_notifier(&cpu_hotplug_notifier);
+	intel_idle_cpuidle_devices_uninit();
 
 	cpu_notifier_register_done();
+
+	cpuidle_unregister_driver(&intel_idle_driver);
 }
 
 module_init(intel_idle_init);
-- 
2.1.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web