Patchwork ACPI / PM: Hold acpi_scan_lock over system PM transitions

login
register
mail settings
Submitter Rafael J. Wysocki
Date Aug. 19, 2013, 11:47 p.m.
Message ID <2852781.TfChGuuEkJ@vostro.rjw.lan>
Download mbox | patch
Permalink /patch/268324/
State Not Applicable
Headers show

Comments

Rafael J. Wysocki - Aug. 19, 2013, 11:47 p.m.
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Bad things happen if ACPI hotplug events are handled during system
PM transitions, especially if devices are removed as a result.
To prevent those bad things from happening, acquire acpi_scan_lock
when a transition is started and release it when that transition
is complete or has been aborted.

This fixes resume lockup on my test-bed Acer Aspire S5 that happens
when Thunderbolt devices are disconnected from the machine while
suspended.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Applies on top of the linux-next branch of the linux-pm.git tree.

Thanks,
Rafael

---
 drivers/acpi/sleep.c |   39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg - Aug. 20, 2013, 11:08 a.m.
On Tue, Aug 20, 2013 at 01:47:57AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Bad things happen if ACPI hotplug events are handled during system
> PM transitions, especially if devices are removed as a result.
> To prevent those bad things from happening, acquire acpi_scan_lock
> when a transition is started and release it when that transition
> is complete or has been aborted.
> 
> This fixes resume lockup on my test-bed Acer Aspire S5 that happens
> when Thunderbolt devices are disconnected from the machine while
> suspended.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

I tested this on Intel DZ77RE-75K and after the patch is applied the
machine didn't hang anymore once resuming.

Feel free to add my tested-by.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki - Aug. 20, 2013, 12:24 p.m.
On Tuesday, August 20, 2013 02:08:33 PM Mika Westerberg wrote:
> On Tue, Aug 20, 2013 at 01:47:57AM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Bad things happen if ACPI hotplug events are handled during system
> > PM transitions, especially if devices are removed as a result.
> > To prevent those bad things from happening, acquire acpi_scan_lock
> > when a transition is started and release it when that transition
> > is complete or has been aborted.
> > 
> > This fixes resume lockup on my test-bed Acer Aspire S5 that happens
> > when Thunderbolt devices are disconnected from the machine while
> > suspended.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> I tested this on Intel DZ77RE-75K and after the patch is applied the
> machine didn't hang anymore once resuming.
> 
> Feel free to add my tested-by.

I will, thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toshi Kani - Aug. 20, 2013, 10:02 p.m.
On Tue, 2013-08-20 at 01:47 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Bad things happen if ACPI hotplug events are handled during system
> PM transitions, especially if devices are removed as a result.
> To prevent those bad things from happening, acquire acpi_scan_lock
> when a transition is started and release it when that transition
> is complete or has been aborted.
> 
> This fixes resume lockup on my test-bed Acer Aspire S5 that happens
> when Thunderbolt devices are disconnected from the machine while
> suspended.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The change looks good to me.

Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -420,10 +420,21 @@  static void acpi_pm_finish(void)
 }
 
 /**
- *	acpi_pm_end - Finish up suspend sequence.
+ * acpi_pm_start - Start system PM transition.
+ */
+static void acpi_pm_start(u32 acpi_state)
+{
+	acpi_target_sleep_state = acpi_state;
+	acpi_sleep_tts_switch(acpi_target_sleep_state);
+	acpi_scan_lock_acquire();
+}
+
+/**
+ * acpi_pm_end - Finish up system PM transition.
  */
 static void acpi_pm_end(void)
 {
+	acpi_scan_lock_release();
 	/*
 	 * This is necessary in case acpi_pm_finish() is not called during a
 	 * failing transition to a sleep state.
@@ -451,21 +462,19 @@  static u32 acpi_suspend_states[] = {
 static int acpi_suspend_begin(suspend_state_t pm_state)
 {
 	u32 acpi_state = acpi_suspend_states[pm_state];
-	int error = 0;
+	int error;
 
 	error = (nvs_nosave || nvs_nosave_s3) ? 0 : suspend_nvs_alloc();
 	if (error)
 		return error;
 
-	if (sleep_states[acpi_state]) {
-		acpi_target_sleep_state = acpi_state;
-		acpi_sleep_tts_switch(acpi_target_sleep_state);
-	} else {
-		printk(KERN_ERR "ACPI does not support this state: %d\n",
-			pm_state);
-		error = -ENOSYS;
+	if (!sleep_states[acpi_state]) {
+		pr_err("ACPI does not support sleep state S%u\n", acpi_state);
+		return -ENOSYS;
 	}
-	return error;
+
+	acpi_pm_start(acpi_state);
+	return 0;
 }
 
 /**
@@ -631,10 +640,8 @@  static int acpi_hibernation_begin(void)
 	int error;
 
 	error = nvs_nosave ? 0 : suspend_nvs_alloc();
-	if (!error) {
-		acpi_target_sleep_state = ACPI_STATE_S4;
-		acpi_sleep_tts_switch(acpi_target_sleep_state);
-	}
+	if (!error)
+		acpi_pm_start(ACPI_STATE_S4);
 
 	return error;
 }
@@ -713,8 +720,10 @@  static int acpi_hibernation_begin_old(vo
 	if (!error) {
 		if (!nvs_nosave)
 			error = suspend_nvs_alloc();
-		if (!error)
+		if (!error) {
 			acpi_target_sleep_state = ACPI_STATE_S4;
+			acpi_scan_lock_acquire();
+		}
 	}
 	return error;
 }