Message ID | 1432928056-25622-1-git-send-email-todd.e.brandt@linux.intel.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
On Fri, 29 May 2015, Todd Brandt wrote: > In theory, if a device has no pm_ops complete callback it shouldn't have > to be locked in order to skip it in the dpm_complete call. This causes > problems when a device without a complete callback has already begun > operation after its resume cb is called. It can end up holding up the > system resume as the pm subsystem tries to get a device lock just to > check for a callback that isn't there. > > This fixes an issue discovered on an Ivy Bridge laptop which has an > AlpsPS/2 GlidePoint touchpad connected to an i8042 serio bus. The resume > path ends up wasting a full second waiting for a device_lock on the psmouse > driver, only to then discover that it has no device_complete callback. The > alpa driver has already begun sending and receiving data after its resume > call was finished, which prevents the pm subsystem from getting the device > lock. Why not simply move the device_lock() and device_unlock() calls inside the "if (callback) {...}" block in device_complete()? Are you worried that the presence/absence of a callback might change while the device is unlocked? But that can happen with your patch too, and there the window is much larger. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 3d874ec..30eb16b 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -94,6 +94,7 @@ void device_pm_sleep_init(struct device *dev) dev->power.is_suspended = false; dev->power.is_noirq_suspended = false; dev->power.is_late_suspended = false; + dev->power.no_complete = false; init_completion(&dev->power.completion); complete_all(&dev->power.completion); dev->power.wakeup = NULL; @@ -897,6 +898,9 @@ static void device_complete(struct device *dev, pm_message_t state) if (dev->power.syscore) return; + if (dev->power.no_complete) + goto Complete; + device_lock(dev); if (dev->pm_domain) { @@ -927,6 +931,7 @@ static void device_complete(struct device *dev, pm_message_t state) device_unlock(dev); +Complete: pm_runtime_put(dev); } @@ -1591,6 +1596,16 @@ static int device_prepare(struct device *dev, pm_message_t state) trace_device_pm_callback_end(dev, ret); } + /* check for the existence of a complete callback while its locked */ + if ((dev->pm_domain && dev->pm_domain->ops.complete) || + (dev->type && dev->type->pm && dev->type->pm->complete) || + (dev->class && dev->class->pm && dev->class->pm->complete) || + (dev->bus && dev->bus->pm && dev->bus->pm->complete) || + (dev->driver && dev->driver->pm && dev->driver->pm->complete)) + dev->power.no_complete = false; + else + dev->power.no_complete = true; + device_unlock(dev); if (ret < 0) { diff --git a/include/linux/pm.h b/include/linux/pm.h index 2d29c64..1e334cd 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -553,6 +553,7 @@ struct dev_pm_info { bool ignore_children:1; bool early_init:1; /* Owned by the PM core */ bool direct_complete:1; /* Owned by the PM core */ + bool no_complete:1; /* Owned by the PM core */ spinlock_t lock; #ifdef CONFIG_PM_SLEEP struct list_head entry;
In theory, if a device has no pm_ops complete callback it shouldn't have to be locked in order to skip it in the dpm_complete call. This causes problems when a device without a complete callback has already begun operation after its resume cb is called. It can end up holding up the system resume as the pm subsystem tries to get a device lock just to check for a callback that isn't there. This fixes an issue discovered on an Ivy Bridge laptop which has an AlpsPS/2 GlidePoint touchpad connected to an i8042 serio bus. The resume path ends up wasting a full second waiting for a device_lock on the psmouse driver, only to then discover that it has no device_complete callback. The alpa driver has already begun sending and receiving data after its resume call was finished, which prevents the pm subsystem from getting the device lock. Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com> --- drivers/base/power/main.c | 15 +++++++++++++++ include/linux/pm.h | 1 + 2 files changed, 16 insertions(+)