| Message ID | 760eb8c67a02e35938a4fbd3b31cbe9bb1414c5a.1474130360.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Sun, 2016-09-18 at 05:39 +0200, Lukas Wunner wrote: > We currently perform a mandatory runtime resume of all PCI devices on > ->shutdown. However it is pointless to wake devices only to immediately > power them down afterwards. (Or have the firmware reset them, in case > of a reboot.) > > It seems there are only two cases when a runtime resume is actually > necessary: If the driver has declared a ->shutdown callback or if kexec > is in progress. > > Constrain resume of a device to these cases and let it slumber > otherwise, thereby conserving energy and speeding up shutdown. What happens if you get a wakeup event while going down? Will it lead to a reboot when a shutdown has been requested? Regards Oliver -- 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
On Mon, Sep 19, 2016 at 11:12:23AM +0200, Oliver Neukum wrote: > On Sun, 2016-09-18 at 05:39 +0200, Lukas Wunner wrote: > > We currently perform a mandatory runtime resume of all PCI devices on > > ->shutdown. However it is pointless to wake devices only to immediately > > power them down afterwards. (Or have the firmware reset them, in case > > of a reboot.) > > > > It seems there are only two cases when a runtime resume is actually > > necessary: If the driver has declared a ->shutdown callback or if kexec > > is in progress. > > > > Constrain resume of a device to these cases and let it slumber > > otherwise, thereby conserving energy and speeding up shutdown. > > What happens if you get a wakeup event while going down? Hm, I should probably disable that with __pci_enable_wake(pci_dev, pci_dev->current_state, true, false); > Will it lead to a reboot when a shutdown has been requested? That would depend on the platform, my guess is no, but only someone with extensive real world experience with such corner cases can answer this. (Rafael?) Thanks, Lukas -- 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
On Monday, September 19, 2016 12:14:02 PM Lukas Wunner wrote: > On Mon, Sep 19, 2016 at 11:12:23AM +0200, Oliver Neukum wrote: > > On Sun, 2016-09-18 at 05:39 +0200, Lukas Wunner wrote: > > > We currently perform a mandatory runtime resume of all PCI devices on > > > ->shutdown. However it is pointless to wake devices only to immediately > > > power them down afterwards. (Or have the firmware reset them, in case > > > of a reboot.) > > > > > > It seems there are only two cases when a runtime resume is actually > > > necessary: If the driver has declared a ->shutdown callback or if kexec > > > is in progress. > > > > > > Constrain resume of a device to these cases and let it slumber > > > otherwise, thereby conserving energy and speeding up shutdown. > > > > What happens if you get a wakeup event while going down? > > Hm, I should probably disable that with > > __pci_enable_wake(pci_dev, pci_dev->current_state, true, false); > What if the user wants, say, WoL to power on the system? > > > Will it lead to a reboot when a shutdown has been requested? > > That would depend on the platform, my guess is no, but only someone > with extensive real world experience with such corner cases can answer > this. (Rafael?) That wakeup will be silently dropped on the floor as a rule AFAICS. But there's another theoretical issue here. On systems with ACPI some devices may need to be reconfigured for wakeup from S5 even though they have been configured for signaling wakeup while in S0. Those would still need to be resumed. Thanks, Rafael -- 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
On Sat, Sep 24, 2016 at 02:50:05AM +0200, Rafael J. Wysocki wrote: > On Monday, September 19, 2016 12:14:02 PM Lukas Wunner wrote: > > On Mon, Sep 19, 2016 at 11:12:23AM +0200, Oliver Neukum wrote: > > > On Sun, 2016-09-18 at 05:39 +0200, Lukas Wunner wrote: > > > > We currently perform a mandatory runtime resume of all PCI devices > > > > on ->shutdown. However it is pointless to wake devices only to > > > > immediately power them down afterwards. (Or have the firmware > > > > reset them, in case of a reboot.) > > > > > > > > It seems there are only two cases when a runtime resume is actually > > > > necessary: If the driver has declared a ->shutdown callback or if > > > > kexec is in progress. > > > > > > > > Constrain resume of a device to these cases and let it slumber > > > > otherwise, thereby conserving energy and speeding up shutdown. > > > > > > What happens if you get a wakeup event while going down? > > > > Hm, I should probably disable that with > > > > __pci_enable_wake(pci_dev, pci_dev->current_state, true, false); > > > > What if the user wants, say, WoL to power on the system? When the PCI device is runtime resumed, the function call above will be executed by pci_pm_runtime_resume(), so WoL will be disabled as well. There are a few drivers which re-enable WoL from their ->shutdown hook, but that isn't impacted by this patch because the fast path isn't taken if the driver has a ->shutdown hook. > But there's another theoretical issue here. > > On systems with ACPI some devices may need to be reconfigured for wakeup > from S5 even though they have been configured for signaling wakeup while > in S0. Those would still need to be resumed. Hm, why does the device have to be resumed for that? The ACPI 6.0 spec says for _DSW: "This control method can only access Operation Regions that are either always available while in a system working state or that are available when the Power Resources referenced by the _PRW object are all ON." Before _DSW is executed, all power resources are turned on, so whatever _DSW does, the device should be powered up and accessible. Does OSPM really have to runtime resume the device for _DSW? Another potential issue that crossed my mind is I/O virtualization: If a PCI device is passed through to a guest VM, the VM may enable PME, put the device in D3, etc. If the VM is shut down, can anything go wrong if the fast path is taken for runtime suspended devices? (To be fair, if the VM crashes and doesn't shut down passed through devices orderly, we'd be in the same situation.) PME would be disabled as described above, and leaving the device in D3hot should be fine as well. If the host or another guest VM wants to use the PCI device afterwards, it will resume it to D0 with pci_enable_device() in the driver's ->probe hook. Let me know if you find flaws in this line of thought. Thanks! Lukas -- 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
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index fd4b9c4..f21c620 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -459,6 +459,15 @@ static void pci_device_shutdown(struct device *dev) struct pci_dev *pci_dev = to_pci_dev(dev); struct pci_driver *drv = pci_dev->driver; + /* Fast path for suspended devices */ + if (pm_runtime_status_suspended(dev) && (!drv || !drv->shutdown) && + !kexec_in_progress) { + pm_runtime_disable(dev); + if (pm_runtime_status_suspended(dev)) + return; + pm_runtime_enable(dev); + } + pm_runtime_resume(dev); if (drv && drv->shutdown)
We currently perform a mandatory runtime resume of all PCI devices on ->shutdown. However it is pointless to wake devices only to immediately power them down afterwards. (Or have the firmware reset them, in case of a reboot.) It seems there are only two cases when a runtime resume is actually necessary: If the driver has declared a ->shutdown callback or if kexec is in progress. Constrain resume of a device to these cases and let it slumber otherwise, thereby conserving energy and speeding up shutdown. To prevent the device from being runtime resumed during the remainder of the shutdown process, disable runtime PM for it. Cc: Rafael J. Wysocki <rjw@rjwysocki.net> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- Changes since v1: * Disable runtime PM on the device to prevent it from being runtime resumed during the remainder of the shutdown process. drivers/pci/pci-driver.c | 9 +++++++++ 1 file changed, 9 insertions(+)