cpuidle acpi driver: fix oops on AC<->DC

cpuidle and acpi driver interaction bug with the way cpuidle_register_driver() is called. Due to this bug, there will be oops on AC<->DC on some systems, where they support C-states in one DC and not in AC. The current code does ON BOOT: Look at CST and other C-state info to see whether more than C1 is supported. If it is, then acpi processor_idle does a cpuidle_register_driver() call, which internally enables the device. ON CST change notification (AC<->DC) and on suspend-resume: acpi driver temporarily disables device, updates the device with any new C-states, and reenables the device. The problem is is on boot, there are no C2, C3 states supported and we skip the register. Later on AC<->DC, we may get a CST notification and we try to reevaluate CST and enabled the device, without actually registering it. This causes breakage as we try to create /sys fs sub directory, without the parent directory which is created at register time. Thanks to Sanjeev for reporting the problem here. http://bugzilla.kernel.org/show_bug.cgi?id=10394 Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> Signed-off-by: Len Brown <len.brown@intel.com>

cpuidle acpi driver: fix oops on AC<->DC
cpuidle and acpi driver interaction bug with the way cpuidle_register_driver() is called. Due to this bug, there will be oops on AC<->DC on some systems, where they support C-states in one DC and not in AC. The current code does ON BOOT: Look at CST and other C-state info to see whether more than C1 is supported. If it is, then acpi processor_idle does a cpuidle_register_driver() call, which internally enables the device. ON CST change notification (AC<->DC) and on suspend-resume: acpi driver temporarily disables device, updates the device with any new C-states, and reenables the device. The problem is is on boot, there are no C2, C3 states supported and we skip the register. Later on AC<->DC, we may get a CST notification and we try to reevaluate CST and enabled the device, without actually registering it. This causes breakage as we try to create /sys fs sub directory, without the parent directory which is created at register time. Thanks to Sanjeev for reporting the problem here. http://bugzilla.kernel.org/show_bug.cgi?id=10394 Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> Signed-off-by: Len Brown <len.brown@intel.com>
Venkatesh Pallipadi · Len Brown
1 parent e1094bfa26
Showing 3 changed files with 43 additions and 11 deletions Side-by-side Diff
drivers/acpi/processor_idle.c
drivers/cpuidle/cpuidle.c
include/linux/cpuidle.h
@@ -1669,6 +1669,7 @@
 		return -EINVAL;
 	}
  
+	dev->cpu = pr->id;
 	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
 		dev->states[i].name[0] = '\0';
 		dev->states[i].desc[0] = '\0';
@@ -1738,7 +1739,7 @@
  
 int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 {
-	int ret;
+	int ret = 0;
  
 	if (boot_option_idle_override)
 		return 0;
@@ -1756,8 +1757,10 @@
 	cpuidle_pause_and_lock();
 	cpuidle_disable_device(&pr->power.dev);
 	acpi_processor_get_power_info(pr);
-	acpi_processor_setup_cpuidle(pr);
-	ret = cpuidle_enable_device(&pr->power.dev);
+	if (pr->flags.power) {
+		acpi_processor_setup_cpuidle(pr);
+		ret = cpuidle_enable_device(&pr->power.dev);
+	}
 	cpuidle_resume_and_unlock();
  
 	return ret;
@@ -1813,7 +1816,6 @@
 	if (pr->flags.power) {
 #ifdef CONFIG_CPU_IDLE
 		acpi_processor_setup_cpuidle(pr);
-		pr->power.dev.cpu = pr->id;
 		if (cpuidle_register_device(&pr->power.dev))
 			return -EIO;
 #endif
@@ -1850,8 +1852,7 @@
 		return 0;
  
 #ifdef CONFIG_CPU_IDLE
-	if (pr->flags.power)
-		cpuidle_unregister_device(&pr->power.dev);
+	cpuidle_unregister_device(&pr->power.dev);
 #endif
 	pr->flags.power_setup_done = 0;
  
@@ -38,6 +38,8 @@
 static void cpuidle_kick_cpus(void) {}
 #endif
  
+static int __cpuidle_register_device(struct cpuidle_device *dev);
+
 /**
  * cpuidle_idle_call - the main idle loop
  *
@@ -138,6 +140,12 @@
 	if (!dev->state_count)
 		return -EINVAL;
  
+	if (dev->registered == 0) {
+		ret = __cpuidle_register_device(dev);
+		if (ret)
+			return ret;
+	}
+
 	if ((ret = cpuidle_add_state_sysfs(dev)))
 		return ret;
  
  
  
@@ -232,10 +240,13 @@
 #endif /* CONFIG_ARCH_HAS_CPU_RELAX */
  
 /**
- * cpuidle_register_device - registers a CPU's idle PM feature
+ * __cpuidle_register_device - internal register function called before register
+ * and enable routines
  * @dev: the cpu
+ *
+ * cpuidle_lock mutex must be held before this is called
  */
-int cpuidle_register_device(struct cpuidle_device *dev)
+static int __cpuidle_register_device(struct cpuidle_device *dev)
 {
 	int ret;
 	struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
  
  
@@ -247,18 +258,34 @@
  
 	init_completion(&dev->kobj_unregister);
  
-	mutex_lock(&cpuidle_lock);
-
 	poll_idle_init(dev);
  
 	per_cpu(cpuidle_devices, dev->cpu) = dev;
 	list_add(&dev->device_list, &cpuidle_detected_devices);
 	if ((ret = cpuidle_add_sysfs(sys_dev))) {
-		mutex_unlock(&cpuidle_lock);
 		module_put(cpuidle_curr_driver->owner);
 		return ret;
 	}
  
+	dev->registered = 1;
+	return 0;
+}
+
+/**
+ * cpuidle_register_device - registers a CPU's idle PM feature
+ * @dev: the cpu
+ */
+int cpuidle_register_device(struct cpuidle_device *dev)
+{
+	int ret;
+
+	mutex_lock(&cpuidle_lock);
+
+	if ((ret = __cpuidle_register_device(dev))) {
+		mutex_unlock(&cpuidle_lock);
+		return ret;
+	}
+
 	cpuidle_enable_device(dev);
 	cpuidle_install_idle_handler();
  
@@ -277,6 +304,9 @@
 void cpuidle_unregister_device(struct cpuidle_device *dev)
 {
 	struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
+
+	if (dev->registered == 0)
+		return;
  
 	cpuidle_pause_and_lock();
  
@@ -82,6 +82,7 @@
 };
  
 struct cpuidle_device {
+	unsigned int		registered:1;
 	unsigned int		enabled:1;
 	unsigned int		cpu;
...	...	@@ -1669,6 +1669,7 @@
1669	1669	return -EINVAL;
1670	1670	}
1671	1671
	1672	+ dev->cpu = pr->id;
1672	1673	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
1673	1674	dev->states[i].name[0] = '\0';
1674	1675	dev->states[i].desc[0] = '\0';
...	...	@@ -1738,7 +1739,7 @@
1738	1739
1739	1740	int acpi_processor_cst_has_changed(struct acpi_processor *pr)
1740	1741	{
1741		- int ret;
	1742	+ int ret = 0;
1742	1743
1743	1744	if (boot_option_idle_override)
1744	1745	return 0;
...	...	@@ -1756,8 +1757,10 @@
1756	1757	cpuidle_pause_and_lock();
1757	1758	cpuidle_disable_device(&pr->power.dev);
1758	1759	acpi_processor_get_power_info(pr);
1759		- acpi_processor_setup_cpuidle(pr);
1760		- ret = cpuidle_enable_device(&pr->power.dev);
	1760	+ if (pr->flags.power) {
	1761	+ acpi_processor_setup_cpuidle(pr);
	1762	+ ret = cpuidle_enable_device(&pr->power.dev);
	1763	+ }
1761	1764	cpuidle_resume_and_unlock();
1762	1765
1763	1766	return ret;
...	...	@@ -1813,7 +1816,6 @@
1813	1816	if (pr->flags.power) {
1814	1817	#ifdef CONFIG_CPU_IDLE
1815	1818	acpi_processor_setup_cpuidle(pr);
1816		- pr->power.dev.cpu = pr->id;
1817	1819	if (cpuidle_register_device(&pr->power.dev))
1818	1820	return -EIO;
1819	1821	#endif
...	...	@@ -1850,8 +1852,7 @@
1850	1852	return 0;
1851	1853
1852	1854	#ifdef CONFIG_CPU_IDLE
1853		- if (pr->flags.power)
1854		- cpuidle_unregister_device(&pr->power.dev);
	1855	+ cpuidle_unregister_device(&pr->power.dev);
1855	1856	#endif
1856	1857	pr->flags.power_setup_done = 0;
1857	1858
...	...	@@ -38,6 +38,8 @@
38	38	static void cpuidle_kick_cpus(void) {}
39	39	#endif
40	40
	41	+static int __cpuidle_register_device(struct cpuidle_device *dev);
	42	+
41	43	/**
42	44	* cpuidle_idle_call - the main idle loop
43	45	*
...	...	@@ -138,6 +140,12 @@
138	140	if (!dev->state_count)
139	141	return -EINVAL;
140	142
	143	+ if (dev->registered == 0) {
	144	+ ret = __cpuidle_register_device(dev);
	145	+ if (ret)
	146	+ return ret;
	147	+ }
	148	+
141	149	if ((ret = cpuidle_add_state_sysfs(dev)))
142	150	return ret;
143	151
144	152
145	153
...	...	@@ -232,10 +240,13 @@
232	240	#endif /* CONFIG_ARCH_HAS_CPU_RELAX */
233	241
234	242	/**
235		- * cpuidle_register_device - registers a CPU's idle PM feature
	243	+ * __cpuidle_register_device - internal register function called before register
	244	+ * and enable routines
236	245	* @dev: the cpu
	246	+ *
	247	+ * cpuidle_lock mutex must be held before this is called
237	248	*/
238		-int cpuidle_register_device(struct cpuidle_device *dev)
	249	+static int __cpuidle_register_device(struct cpuidle_device *dev)
239	250	{
240	251	int ret;
241	252	struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
242	253
243	254
...	...	@@ -247,18 +258,34 @@
247	258
248	259	init_completion(&dev->kobj_unregister);
249	260
250		- mutex_lock(&cpuidle_lock);
251		-
252	261	poll_idle_init(dev);
253	262
254	263	per_cpu(cpuidle_devices, dev->cpu) = dev;
255	264	list_add(&dev->device_list, &cpuidle_detected_devices);
256	265	if ((ret = cpuidle_add_sysfs(sys_dev))) {
257		- mutex_unlock(&cpuidle_lock);
258	266	module_put(cpuidle_curr_driver->owner);
259	267	return ret;
260	268	}
261	269
	270	+ dev->registered = 1;
	271	+ return 0;
	272	+}
	273	+
	274	+/**
	275	+ * cpuidle_register_device - registers a CPU's idle PM feature
	276	+ * @dev: the cpu
	277	+ */
	278	+int cpuidle_register_device(struct cpuidle_device *dev)
	279	+{
	280	+ int ret;
	281	+
	282	+ mutex_lock(&cpuidle_lock);
	283	+
	284	+ if ((ret = __cpuidle_register_device(dev))) {
	285	+ mutex_unlock(&cpuidle_lock);
	286	+ return ret;
	287	+ }
	288	+
262	289	cpuidle_enable_device(dev);
263	290	cpuidle_install_idle_handler();
264	291
...	...	@@ -277,6 +304,9 @@
277	304	void cpuidle_unregister_device(struct cpuidle_device *dev)
278	305	{
279	306	struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
	307	+
	308	+ if (dev->registered == 0)
	309	+ return;
280	310
281	311	cpuidle_pause_and_lock();
282	312
...	...	@@ -82,6 +82,7 @@
82	82	};
83	83
84	84	struct cpuidle_device {
	85	+ unsigned int registered:1;
85	86	unsigned int enabled:1;
86	87	unsigned int cpu;
87	88