From 311aab73d273eb22be976055f6cab224f7279d5e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 8 Aug 2011 23:39:36 +0200 Subject: PM / Runtime: Add might_sleep() to runtime PM functions Some of the entry points to pm runtime are not safe to call in atomic context unless pm_runtime_irq_safe() has been called. Inspecting the code, it is not immediately obvious that the functions sleep at all, as they run inside a spin_lock_irqsave, but under some conditions they can drop the lock and turn on irqs. If a driver incorrectly calls the pm_runtime apis, it can cause sleeping and irq processing when it expects to stay in atomic context. Add might_sleep_if to the majority of the __pm_runtime_* entry points to enforce correct usage. Add pm_runtime_put_sync_autosuspend to the list of functions that can be called in atomic context. Signed-off-by: Colin Cross Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- Documentation/power/runtime_pm.txt | 1 + drivers/base/power/runtime.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt index 4ce5450ab6e8..62f37bc3866f 100644 --- a/Documentation/power/runtime_pm.txt +++ b/Documentation/power/runtime_pm.txt @@ -484,6 +484,7 @@ pm_runtime_resume() pm_runtime_get_sync() pm_runtime_put_sync() pm_runtime_put_sync_suspend() +pm_runtime_put_sync_autosuspend() 5. Runtime PM Initialization, Device Probing and Removal diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index acb3f83b8079..04e18abb50bb 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -732,13 +732,16 @@ EXPORT_SYMBOL_GPL(pm_schedule_suspend); * return immediately if it is larger than zero. Then carry out an idle * notification, either synchronous or asynchronous. * - * This routine may be called in atomic context if the RPM_ASYNC flag is set. + * This routine may be called in atomic context if the RPM_ASYNC flag is set, + * or if pm_runtime_irq_safe() has been called. */ int __pm_runtime_idle(struct device *dev, int rpmflags) { unsigned long flags; int retval; + might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe); + if (rpmflags & RPM_GET_PUT) { if (!atomic_dec_and_test(&dev->power.usage_count)) return 0; @@ -761,13 +764,16 @@ EXPORT_SYMBOL_GPL(__pm_runtime_idle); * return immediately if it is larger than zero. Then carry out a suspend, * either synchronous or asynchronous. * - * This routine may be called in atomic context if the RPM_ASYNC flag is set. + * This routine may be called in atomic context if the RPM_ASYNC flag is set, + * or if pm_runtime_irq_safe() has been called. */ int __pm_runtime_suspend(struct device *dev, int rpmflags) { unsigned long flags; int retval; + might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe); + if (rpmflags & RPM_GET_PUT) { if (!atomic_dec_and_test(&dev->power.usage_count)) return 0; @@ -789,13 +795,16 @@ EXPORT_SYMBOL_GPL(__pm_runtime_suspend); * If the RPM_GET_PUT flag is set, increment the device's usage count. Then * carry out a resume, either synchronous or asynchronous. * - * This routine may be called in atomic context if the RPM_ASYNC flag is set. + * This routine may be called in atomic context if the RPM_ASYNC flag is set, + * or if pm_runtime_irq_safe() has been called. */ int __pm_runtime_resume(struct device *dev, int rpmflags) { unsigned long flags; int retval; + might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe); + if (rpmflags & RPM_GET_PUT) atomic_inc(&dev->power.usage_count); -- cgit v1.2.3-55-g7522 From 5b1b0b812a7b1a5b968c5d06d90d1cb88621b941 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 19 Aug 2011 23:49:48 +0200 Subject: PM / Runtime: Add macro to test for runtime PM events This patch (as1482) adds a macro for testing whether or not a pm_message value represents an autosuspend or autoresume (i.e., a runtime PM) event. Encapsulating this notion seems preferable to open-coding the test all over the place. Signed-off-by: Alan Stern Acked-by: Greg Kroah-Hartman Signed-off-by: Rafael J. Wysocki --- Documentation/usb/power-management.txt | 8 ++++---- drivers/bluetooth/btusb.c | 2 +- drivers/hid/hid-picolcd.c | 2 +- drivers/hid/usbhid/hid-core.c | 7 +++---- drivers/net/usb/usbnet.c | 2 +- drivers/net/wimax/i2400m/usb.c | 4 ++-- drivers/usb/class/cdc-acm.c | 2 +- drivers/usb/class/cdc-wdm.c | 6 +++--- drivers/usb/core/driver.c | 9 ++++----- drivers/usb/core/hcd.c | 4 ++-- drivers/usb/core/hub.c | 10 +++++----- drivers/usb/serial/sierra.c | 2 +- drivers/usb/serial/usb_wwan.c | 2 +- include/linux/pm.h | 2 ++ sound/usb/card.c | 2 +- 15 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index c9ffa9ced7ee..e8662a5fbc5d 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -439,10 +439,10 @@ cause autosuspends to fail with -EBUSY if the driver needs to use the device. External suspend calls should never be allowed to fail in this way, -only autosuspend calls. The driver can tell them apart by checking -the PM_EVENT_AUTO bit in the message.event argument to the suspend -method; this bit will be set for internal PM events (autosuspend) and -clear for external PM events. +only autosuspend calls. The driver can tell them apart by applying +the PMSG_IS_AUTO() macro to the message argument to the suspend +method; it will return True for internal PM events (autosuspend) and +False for external PM events. Mutual exclusion diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 91d13a9e8c65..91b190c40497 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -1103,7 +1103,7 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message) return 0; spin_lock_irq(&data->txlock); - if (!((message.event & PM_EVENT_AUTO) && data->tx_in_flight)) { + if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) { set_bit(BTUSB_SUSPENDING, &data->flags); spin_unlock_irq(&data->txlock); } else { diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c index 9d8710f8bc79..1782693819f3 100644 --- a/drivers/hid/hid-picolcd.c +++ b/drivers/hid/hid-picolcd.c @@ -2409,7 +2409,7 @@ static int picolcd_raw_event(struct hid_device *hdev, #ifdef CONFIG_PM static int picolcd_suspend(struct hid_device *hdev, pm_message_t message) { - if (message.event & PM_EVENT_AUTO) + if (PMSG_IS_AUTO(message)) return 0; picolcd_suspend_backlight(hid_get_drvdata(hdev)); diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index ad978f5748d3..a9fa294ee7d3 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1332,7 +1332,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) struct usbhid_device *usbhid = hid->driver_data; int status; - if (message.event & PM_EVENT_AUTO) { + if (PMSG_IS_AUTO(message)) { spin_lock_irq(&usbhid->lock); /* Sync with error handler */ if (!test_bit(HID_RESET_PENDING, &usbhid->iofl) && !test_bit(HID_CLEAR_HALT, &usbhid->iofl) @@ -1367,7 +1367,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) return -EIO; } - if (!ignoreled && (message.event & PM_EVENT_AUTO)) { + if (!ignoreled && PMSG_IS_AUTO(message)) { spin_lock_irq(&usbhid->lock); if (test_bit(HID_LED_ON, &usbhid->iofl)) { spin_unlock_irq(&usbhid->lock); @@ -1380,8 +1380,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) hid_cancel_delayed_stuff(usbhid); hid_cease_io(usbhid); - if ((message.event & PM_EVENT_AUTO) && - test_bit(HID_KEYS_PRESSED, &usbhid->iofl)) { + if (PMSG_IS_AUTO(message) && test_bit(HID_KEYS_PRESSED, &usbhid->iofl)) { /* lost race against keypresses */ status = hid_start_in(hid); if (status < 0) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index ce395fe5de26..f1c435ba5284 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1470,7 +1470,7 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message) if (!dev->suspend_count++) { spin_lock_irq(&dev->txq.lock); /* don't autosuspend while transmitting */ - if (dev->txq.qlen && (message.event & PM_EVENT_AUTO)) { + if (dev->txq.qlen && PMSG_IS_AUTO(message)) { spin_unlock_irq(&dev->txq.lock); return -EBUSY; } else { diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c index 298f2b0b6311..9a644d052f1e 100644 --- a/drivers/net/wimax/i2400m/usb.c +++ b/drivers/net/wimax/i2400m/usb.c @@ -599,7 +599,7 @@ void i2400mu_disconnect(struct usb_interface *iface) * * As well, the device might refuse going to sleep for whichever * reason. In this case we just fail. For system suspend/hibernate, - * we *can't* fail. We check PM_EVENT_AUTO to see if the + * we *can't* fail. We check PMSG_IS_AUTO to see if the * suspend call comes from the USB stack or from the system and act * in consequence. * @@ -615,7 +615,7 @@ int i2400mu_suspend(struct usb_interface *iface, pm_message_t pm_msg) struct i2400m *i2400m = &i2400mu->i2400m; #ifdef CONFIG_PM - if (pm_msg.event & PM_EVENT_AUTO) + if (PMSG_IS_AUTO(pm_msg)) is_autosuspend = 1; #endif diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index dac7676ce21b..94e6c5c09dd8 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1305,7 +1305,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) struct acm *acm = usb_get_intfdata(intf); int cnt; - if (message.event & PM_EVENT_AUTO) { + if (PMSG_IS_AUTO(message)) { int b; spin_lock_irq(&acm->write_lock); diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 2b9ff518b509..42f180aca3fb 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -798,11 +798,11 @@ static int wdm_suspend(struct usb_interface *intf, pm_message_t message) dev_dbg(&desc->intf->dev, "wdm%d_suspend\n", intf->minor); /* if this is an autosuspend the caller does the locking */ - if (!(message.event & PM_EVENT_AUTO)) + if (!PMSG_IS_AUTO(message)) mutex_lock(&desc->lock); spin_lock_irq(&desc->iuspin); - if ((message.event & PM_EVENT_AUTO) && + if (PMSG_IS_AUTO(message) && (test_bit(WDM_IN_USE, &desc->flags) || test_bit(WDM_RESPONDING, &desc->flags))) { spin_unlock_irq(&desc->iuspin); @@ -815,7 +815,7 @@ static int wdm_suspend(struct usb_interface *intf, pm_message_t message) kill_urbs(desc); cancel_work_sync(&desc->rxwork); } - if (!(message.event & PM_EVENT_AUTO)) + if (!PMSG_IS_AUTO(message)) mutex_unlock(&desc->lock); return rv; diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 34e3da5aa72a..e03042883c68 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1046,8 +1046,7 @@ static int usb_resume_device(struct usb_device *udev, pm_message_t msg) /* Non-root devices on a full/low-speed bus must wait for their * companion high-speed root hub, in case a handoff is needed. */ - if (!(msg.event & PM_EVENT_AUTO) && udev->parent && - udev->bus->hs_companion) + if (!PMSG_IS_AUTO(msg) && udev->parent && udev->bus->hs_companion) device_pm_wait_for_dev(&udev->dev, &udev->bus->hs_companion->root_hub->dev); @@ -1075,7 +1074,7 @@ static int usb_suspend_interface(struct usb_device *udev, if (driver->suspend) { status = driver->suspend(intf, msg); - if (status && !(msg.event & PM_EVENT_AUTO)) + if (status && !PMSG_IS_AUTO(msg)) dev_err(&intf->dev, "%s error %d\n", "suspend", status); } else { @@ -1189,7 +1188,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) status = usb_suspend_interface(udev, intf, msg); /* Ignore errors during system sleep transitions */ - if (!(msg.event & PM_EVENT_AUTO)) + if (!PMSG_IS_AUTO(msg)) status = 0; if (status != 0) break; @@ -1199,7 +1198,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) status = usb_suspend_device(udev, msg); /* Again, ignore errors during system sleep transitions */ - if (!(msg.event & PM_EVENT_AUTO)) + if (!PMSG_IS_AUTO(msg)) status = 0; } diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 8669ba3fe794..da582f4e486b 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1960,7 +1960,7 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg) int old_state = hcd->state; dev_dbg(&rhdev->dev, "bus %s%s\n", - (msg.event & PM_EVENT_AUTO ? "auto-" : ""), "suspend"); + (PMSG_IS_AUTO(msg) ? "auto-" : ""), "suspend"); if (HCD_DEAD(hcd)) { dev_dbg(&rhdev->dev, "skipped %s of dead bus\n", "suspend"); return 0; @@ -1996,7 +1996,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg) int old_state = hcd->state; dev_dbg(&rhdev->dev, "usb %s%s\n", - (msg.event & PM_EVENT_AUTO ? "auto-" : ""), "resume"); + (PMSG_IS_AUTO(msg) ? "auto-" : ""), "resume"); if (HCD_DEAD(hcd)) { dev_dbg(&rhdev->dev, "skipped %s of dead bus\n", "resume"); return 0; diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index a428aa080a36..ee50e0bf84e8 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2342,7 +2342,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) dev_dbg(&udev->dev, "won't remote wakeup, status %d\n", status); /* bail if autosuspend is requested */ - if (msg.event & PM_EVENT_AUTO) + if (PMSG_IS_AUTO(msg)) return status; } } @@ -2367,12 +2367,12 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) USB_CTRL_SET_TIMEOUT); /* System sleep transitions should never fail */ - if (!(msg.event & PM_EVENT_AUTO)) + if (!PMSG_IS_AUTO(msg)) status = 0; } else { /* device has up to 10 msec to fully suspend */ dev_dbg(&udev->dev, "usb %ssuspend\n", - (msg.event & PM_EVENT_AUTO ? "auto-" : "")); + (PMSG_IS_AUTO(msg) ? "auto-" : "")); usb_set_device_state(udev, USB_STATE_SUSPENDED); msleep(10); } @@ -2523,7 +2523,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) } else { /* drive resume for at least 20 msec */ dev_dbg(&udev->dev, "usb %sresume\n", - (msg.event & PM_EVENT_AUTO ? "auto-" : "")); + (PMSG_IS_AUTO(msg) ? "auto-" : "")); msleep(25); /* Virtual root hubs can trigger on GET_PORT_STATUS to @@ -2625,7 +2625,7 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg) udev = hdev->children [port1-1]; if (udev && udev->can_submit) { dev_warn(&intf->dev, "port %d nyet suspended\n", port1); - if (msg.event & PM_EVENT_AUTO) + if (PMSG_IS_AUTO(msg)) return -EBUSY; } } diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c index d5d136a53b61..b18179bda0d8 100644 --- a/drivers/usb/serial/sierra.c +++ b/drivers/usb/serial/sierra.c @@ -1009,7 +1009,7 @@ static int sierra_suspend(struct usb_serial *serial, pm_message_t message) struct sierra_intf_private *intfdata; int b; - if (message.event & PM_EVENT_AUTO) { + if (PMSG_IS_AUTO(message)) { intfdata = serial->private; spin_lock_irq(&intfdata->susp_lock); b = intfdata->in_flight; diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index e4fad5e643d7..d555ca9567b8 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -651,7 +651,7 @@ int usb_wwan_suspend(struct usb_serial *serial, pm_message_t message) dbg("%s entered", __func__); - if (message.event & PM_EVENT_AUTO) { + if (PMSG_IS_AUTO(message)) { spin_lock_irq(&intfdata->susp_lock); b = intfdata->in_flight; spin_unlock_irq(&intfdata->susp_lock); diff --git a/include/linux/pm.h b/include/linux/pm.h index f7c84c9abd30..18de9f893497 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -366,6 +366,8 @@ extern struct dev_pm_ops generic_subsys_pm_ops; #define PMSG_AUTO_RESUME ((struct pm_message) \ { .event = PM_EVENT_AUTO_RESUME, }) +#define PMSG_IS_AUTO(msg) (((msg).event & PM_EVENT_AUTO) != 0) + /** * Device run-time power management status. * diff --git a/sound/usb/card.c b/sound/usb/card.c index 781d9e61adfb..d5754fa5e551 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -628,7 +628,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) if (chip == (void *)-1L) return 0; - if (!(message.event & PM_EVENT_AUTO)) { + if (!PMSG_IS_AUTO(message)) { snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot); if (!chip->num_suspended_intf++) { list_for_each(p, &chip->pcm_list) { -- cgit v1.2.3-55-g7522 From 2e6ba515f50ef7ddf35b2703d014d3216c9b8b24 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Wed, 21 Sep 2011 22:31:33 +0200 Subject: PM / Runtime: pm_runtime_idle() can be called in atomic context Add to pm_runtime_idle the list of functions that can be called in atomic context if pm_runtime_irq_safe() has been called for the device. Signed-off-by: Ming Lei Signed-off-by: Rafael J. Wysocki --- Documentation/power/runtime_pm.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt index 62f37bc3866f..1750740b17e8 100644 --- a/Documentation/power/runtime_pm.txt +++ b/Documentation/power/runtime_pm.txt @@ -478,6 +478,7 @@ pm_runtime_autosuspend_expiration() If pm_runtime_irq_safe() has been called for a device then the following helper functions may also be used in interrupt context: +pm_runtime_idle() pm_runtime_suspend() pm_runtime_autosuspend() pm_runtime_resume() -- cgit v1.2.3-55-g7522 From 30b1a7a32ca48fd8758f8ca44d60deebc0aa3d72 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Tue, 27 Sep 2011 21:54:22 +0200 Subject: USB: Add wakeup info to debugging messages This patch (as1487) improves the usbcore debugging output for port suspend and bus suspend, by stating whether or not remote wakeup is enabled. Signed-off-by: Alan Stern Acked-by: Greg Kroah-Hartman Signed-off-by: Rafael J. Wysocki --- drivers/usb/core/hcd.c | 9 +++++---- drivers/usb/core/hub.c | 7 +++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index da582f4e486b..877e0e27b90a 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1959,8 +1959,9 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg) int status; int old_state = hcd->state; - dev_dbg(&rhdev->dev, "bus %s%s\n", - (PMSG_IS_AUTO(msg) ? "auto-" : ""), "suspend"); + dev_dbg(&rhdev->dev, "bus %ssuspend, wakeup %d\n", + (PMSG_IS_AUTO(msg) ? "auto-" : ""), + rhdev->do_remote_wakeup); if (HCD_DEAD(hcd)) { dev_dbg(&rhdev->dev, "skipped %s of dead bus\n", "suspend"); return 0; @@ -1995,8 +1996,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg) int status; int old_state = hcd->state; - dev_dbg(&rhdev->dev, "usb %s%s\n", - (PMSG_IS_AUTO(msg) ? "auto-" : ""), "resume"); + dev_dbg(&rhdev->dev, "usb %sresume\n", + (PMSG_IS_AUTO(msg) ? "auto-" : "")); if (HCD_DEAD(hcd)) { dev_dbg(&rhdev->dev, "skipped %s of dead bus\n", "resume"); return 0; diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index ee50e0bf84e8..13bc83240adb 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2324,8 +2324,6 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) int port1 = udev->portnum; int status; - // dev_dbg(hub->intfdev, "suspend port %d\n", port1); - /* enable remote wakeup when appropriate; this lets the device * wake up the upstream hub (including maybe the root hub). * @@ -2371,8 +2369,9 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) status = 0; } else { /* device has up to 10 msec to fully suspend */ - dev_dbg(&udev->dev, "usb %ssuspend\n", - (PMSG_IS_AUTO(msg) ? "auto-" : "")); + dev_dbg(&udev->dev, "usb %ssuspend, wakeup %d\n", + (PMSG_IS_AUTO(msg) ? "auto-" : ""), + udev->do_remote_wakeup); usb_set_device_state(udev, USB_STATE_SUSPENDED); msleep(10); } -- cgit v1.2.3-55-g7522 From ad3c36a534bc7b945d7bffdda1c62e13bf93489a Mon Sep 17 00:00:00 2001 From: Rafael J. Wysocki Date: Tue, 27 Sep 2011 21:54:52 +0200 Subject: PM / Runtime: Don't run callbacks under lock for power.irq_safe set The rpm_suspend() and rpm_resume() routines execute subsystem or PM domain callbacks under power.lock if power.irq_safe is set for the given device. This is inconsistent with that rpm_idle() does after commit 02b2677 (PM / Runtime: Allow _put_sync() from interrupts-disabled context) and is problematic for subsystems and PM domains wanting to use power.lock for synchronization in their runtime PM callbacks. This change requires the code checking if the device's runtime PM status is RPM_SUSPENDING or RPM_RESUMING to be modified too, to take the power.irq_safe set case into account (that code wasn't reachable before with power.irq_safe set, because it's executed with the device's power.lock held). Signed-off-by: Rafael J. Wysocki Reviewed-by: Ming Lei Reviewed-by: Kevin Hilman --- drivers/base/power/runtime.c | 68 ++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 04e18abb50bb..aecb2a887ed7 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -154,6 +154,31 @@ static int rpm_check_suspend_allowed(struct device *dev) return retval; } +/** + * __rpm_callback - Run a given runtime PM callback for a given device. + * @cb: Runtime PM callback to run. + * @dev: Device to run the callback for. + */ +static int __rpm_callback(int (*cb)(struct device *), struct device *dev) + __releases(&dev->power.lock) __acquires(&dev->power.lock) +{ + int retval; + + if (dev->power.irq_safe) + spin_unlock(&dev->power.lock); + else + spin_unlock_irq(&dev->power.lock); + + retval = cb(dev); + + if (dev->power.irq_safe) + spin_lock(&dev->power.lock); + else + spin_lock_irq(&dev->power.lock); + + return retval; +} + /** * rpm_idle - Notify device bus type if the device can be suspended. * @dev: Device to notify the bus type about. @@ -225,19 +250,8 @@ static int rpm_idle(struct device *dev, int rpmflags) else callback = NULL; - if (callback) { - if (dev->power.irq_safe) - spin_unlock(&dev->power.lock); - else - spin_unlock_irq(&dev->power.lock); - - callback(dev); - - if (dev->power.irq_safe) - spin_lock(&dev->power.lock); - else - spin_lock_irq(&dev->power.lock); - } + if (callback) + __rpm_callback(callback, dev); dev->power.idle_notification = false; wake_up_all(&dev->power.wait_queue); @@ -252,22 +266,14 @@ static int rpm_idle(struct device *dev, int rpmflags) * @dev: Device to run the callback for. */ static int rpm_callback(int (*cb)(struct device *), struct device *dev) - __releases(&dev->power.lock) __acquires(&dev->power.lock) { int retval; if (!cb) return -ENOSYS; - if (dev->power.irq_safe) { - retval = cb(dev); - } else { - spin_unlock_irq(&dev->power.lock); - - retval = cb(dev); + retval = __rpm_callback(cb, dev); - spin_lock_irq(&dev->power.lock); - } dev->power.runtime_error = retval; return retval != -EACCES ? retval : -EIO; } @@ -347,6 +353,15 @@ static int rpm_suspend(struct device *dev, int rpmflags) goto out; } + if (dev->power.irq_safe) { + spin_unlock(&dev->power.lock); + + cpu_relax(); + + spin_lock(&dev->power.lock); + goto repeat; + } + /* Wait for the other suspend running in parallel with us. */ for (;;) { prepare_to_wait(&dev->power.wait_queue, &wait, @@ -496,6 +511,15 @@ static int rpm_resume(struct device *dev, int rpmflags) goto out; } + if (dev->power.irq_safe) { + spin_unlock(&dev->power.lock); + + cpu_relax(); + + spin_lock(&dev->power.lock); + goto repeat; + } + /* Wait for the operation carried out in parallel with us. */ for (;;) { prepare_to_wait(&dev->power.wait_queue, &wait, -- cgit v1.2.3-55-g7522 From 53b615ccca567ada1931eb04ad0614ac150c14a3 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Tue, 27 Sep 2011 22:53:27 +0200 Subject: PM / Runtime: Introduce trace points for tracing rpm_* functions This patch introduces 3 trace points to prepare for tracing rpm_idle/rpm_suspend/rpm_resume functions, so we can use these trace points to replace the current dev_dbg(). Signed-off-by: Ming Lei Acked-by: Steven Rostedt Signed-off-by: Rafael J. Wysocki --- include/trace/events/rpm.h | 99 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/trace/Makefile | 1 + kernel/trace/rpm-traces.c | 20 ++++++++++ 3 files changed, 120 insertions(+) create mode 100644 include/trace/events/rpm.h create mode 100644 kernel/trace/rpm-traces.c diff --git a/include/trace/events/rpm.h b/include/trace/events/rpm.h new file mode 100644 index 000000000000..d62c558bf64b --- /dev/null +++ b/include/trace/events/rpm.h @@ -0,0 +1,99 @@ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM rpm + +#if !defined(_TRACE_RUNTIME_POWER_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_RUNTIME_POWER_H + +#include +#include +#include + +/* + * The rpm_internal events are used for tracing some important + * runtime pm internal functions. + */ +DECLARE_EVENT_CLASS(rpm_internal, + + TP_PROTO(struct device *dev, int flags), + + TP_ARGS(dev, flags), + + TP_STRUCT__entry( + __string( name, dev_name(dev) ) + __field( int, flags ) + __field( int , usage_count ) + __field( int , disable_depth ) + __field( int , runtime_auto ) + __field( int , request_pending ) + __field( int , irq_safe ) + __field( int , child_count ) + ), + + TP_fast_assign( + __assign_str(name, dev_name(dev)); + __entry->flags = flags; + __entry->usage_count = atomic_read( + &dev->power.usage_count); + __entry->disable_depth = dev->power.disable_depth; + __entry->runtime_auto = dev->power.runtime_auto; + __entry->request_pending = dev->power.request_pending; + __entry->irq_safe = dev->power.irq_safe; + __entry->child_count = atomic_read( + &dev->power.child_count); + ), + + TP_printk("%s flags-%x cnt-%-2d dep-%-2d auto-%-1d p-%-1d" + " irq-%-1d child-%d", + __get_str(name), __entry->flags, + __entry->usage_count, + __entry->disable_depth, + __entry->runtime_auto, + __entry->request_pending, + __entry->irq_safe, + __entry->child_count + ) +); +DEFINE_EVENT(rpm_internal, rpm_suspend, + + TP_PROTO(struct device *dev, int flags), + + TP_ARGS(dev, flags) +); +DEFINE_EVENT(rpm_internal, rpm_resume, + + TP_PROTO(struct device *dev, int flags), + + TP_ARGS(dev, flags) +); +DEFINE_EVENT(rpm_internal, rpm_idle, + + TP_PROTO(struct device *dev, int flags), + + TP_ARGS(dev, flags) +); + +TRACE_EVENT(rpm_return_int, + TP_PROTO(struct device *dev, unsigned long ip, int ret), + TP_ARGS(dev, ip, ret), + + TP_STRUCT__entry( + __string( name, dev_name(dev)) + __field( unsigned long, ip ) + __field( int, ret ) + ), + + TP_fast_assign( + __assign_str(name, dev_name(dev)); + __entry->ip = ip; + __entry->ret = ret; + ), + + TP_printk("%pS:%s ret=%d", (void *)__entry->ip, __get_str(name), + __entry->ret) +); + +#endif /* _TRACE_RUNTIME_POWER_H */ + +/* This part must be outside protection */ +#include diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile index 761c510a06c5..56bdab5b3793 100644 --- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -53,6 +53,7 @@ endif obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o obj-$(CONFIG_TRACEPOINTS) += power-traces.o +obj-$(CONFIG_TRACEPOINTS) += rpm-traces.o ifeq ($(CONFIG_TRACING),y) obj-$(CONFIG_KGDB_KDB) += trace_kdb.o endif diff --git a/kernel/trace/rpm-traces.c b/kernel/trace/rpm-traces.c new file mode 100644 index 000000000000..4b3b5eaf94d1 --- /dev/null +++ b/kernel/trace/rpm-traces.c @@ -0,0 +1,20 @@ +/* + * Power trace points + * + * Copyright (C) 2009 Ming Lei + */ + +#include +#include +#include +#include +#include +#include + +#define CREATE_TRACE_POINTS +#include + +EXPORT_TRACEPOINT_SYMBOL_GPL(rpm_return_int); +EXPORT_TRACEPOINT_SYMBOL_GPL(rpm_idle); +EXPORT_TRACEPOINT_SYMBOL_GPL(rpm_suspend); +EXPORT_TRACEPOINT_SYMBOL_GPL(rpm_resume); -- cgit v1.2.3-55-g7522 From c3dc2f14622a06488f11452b6efd1e02c5a8548b Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Tue, 27 Sep 2011 22:54:41 +0200 Subject: PM / Runtime: Replace dev_dbg() with trace_rpm_*() This patch replaces dev_dbg with trace_rpm_* inside the three important functions: rpm_idle rpm_suspend rpm_resume Trace points have the below advantages compared with dev_dbg: - trace points include much runtime information(such as running cpu, current task, ...) - most of linux distributions may disable "verbose debug" driver debug compile switch, so it is very difficult to report/debug runtime pm related problems from distribution users without this kind of debug information. - for upstream kernel users, enableing the debug switch will produce many useless "rpm_resume" output, and it is very noise. - dev_dbg inside rpm_suspend/rpm_resume may have some effects on runtime pm behaviour of console devicer Signed-off-by: Ming Lei Acked-by: Steven Rostedt Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index aecb2a887ed7..7a6fb5e34a0e 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -9,6 +9,7 @@ #include #include +#include #include "power.h" static int rpm_resume(struct device *dev, int rpmflags); @@ -196,6 +197,7 @@ static int rpm_idle(struct device *dev, int rpmflags) int (*callback)(struct device *); int retval; + trace_rpm_idle(dev, rpmflags); retval = rpm_check_suspend_allowed(dev); if (retval < 0) ; /* Conditions are wrong. */ @@ -257,6 +259,7 @@ static int rpm_idle(struct device *dev, int rpmflags) wake_up_all(&dev->power.wait_queue); out: + trace_rpm_return_int(dev, _THIS_IP_, retval); return retval; } @@ -301,7 +304,7 @@ static int rpm_suspend(struct device *dev, int rpmflags) struct device *parent = NULL; int retval; - dev_dbg(dev, "%s flags 0x%x\n", __func__, rpmflags); + trace_rpm_suspend(dev, rpmflags); repeat: retval = rpm_check_suspend_allowed(dev); @@ -445,7 +448,7 @@ static int rpm_suspend(struct device *dev, int rpmflags) } out: - dev_dbg(dev, "%s returns %d\n", __func__, retval); + trace_rpm_return_int(dev, _THIS_IP_, retval); return retval; } @@ -474,7 +477,7 @@ static int rpm_resume(struct device *dev, int rpmflags) struct device *parent = NULL; int retval = 0; - dev_dbg(dev, "%s flags 0x%x\n", __func__, rpmflags); + trace_rpm_resume(dev, rpmflags); repeat: if (dev->power.runtime_error) @@ -639,7 +642,7 @@ static int rpm_resume(struct device *dev, int rpmflags) spin_lock_irq(&dev->power.lock); } - dev_dbg(dev, "%s returns %d\n", __func__, retval); + trace_rpm_return_int(dev, _THIS_IP_, retval); return retval; } -- cgit v1.2.3-55-g7522 From 2a5306cc5f383b0e7414c75e458111afd4a563a4 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 29 Sep 2011 22:07:23 +0200 Subject: PM / Tracing: build rpm-traces.c only if CONFIG_PM_RUNTIME is set Do not build kernel/trace/rpm-traces.c if CONFIG_PM_RUNTIME is not set, which avoids a build failure. [rjw: Added the changelog and modified the subject slightly.] Signed-off-by: Ming Lei Signed-off-by: Rafael J. Wysocki --- kernel/trace/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile index 56bdab5b3793..f49405f842f4 100644 --- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -53,7 +53,9 @@ endif obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o obj-$(CONFIG_TRACEPOINTS) += power-traces.o +ifeq ($(CONFIG_PM_RUNTIME),y) obj-$(CONFIG_TRACEPOINTS) += rpm-traces.o +endif ifeq ($(CONFIG_TRACING),y) obj-$(CONFIG_KGDB_KDB) += trace_kdb.o endif -- cgit v1.2.3-55-g7522