From: Rafael J. Wysocki <[email protected]>
Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
driver flags) inadvertently prevented the power.direct_complete flag
from being set for devices without PM callbacks and with disabled
runtime PM which also prevents power.direct_complete from being set
for their parents. That led to problems including a resume crash on
HP ZBook 14u.
Restore the previous behavior by causing power.direct_complete to be
set for those devices again, but do that in a more direct way to
avoid overlooking that case in the future.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
Reported-by: Thomas Martitz <[email protected]>
Tested-by: Thomas Martitz <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/main.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1920,10 +1920,8 @@ static int device_prepare(struct device
dev->power.wakeup_path = false;
- if (dev->power.no_pm_callbacks) {
- ret = 1; /* Let device go direct_complete */
+ if (dev->power.no_pm_callbacks)
goto unlock;
- }
if (dev->pm_domain)
callback = dev->pm_domain->ops.prepare;
@@ -1957,7 +1955,8 @@ unlock:
*/
spin_lock_irq(&dev->power.lock);
dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
- pm_runtime_suspended(dev) && ret > 0 &&
+ ((pm_runtime_suspended(dev) && ret > 0) ||
+ dev->power.no_pm_callbacks) &&
!dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
spin_unlock_irq(&dev->power.lock);
return 0;
Hello,
thanks for for your effort and the patch.
Is this eligible for stable?
Best regards
Am 22.05.2018 um 13:02 schrieb Rafael J. Wysocki:
> From: Rafael J. Wysocki <[email protected]>
>
> Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
> driver flags) inadvertently prevented the power.direct_complete flag
> from being set for devices without PM callbacks and with disabled
> runtime PM which also prevents power.direct_complete from being set
> for their parents. That led to problems including a resume crash on
> HP ZBook 14u.
>
> Restore the previous behavior by causing power.direct_complete to be
> set for those devices again, but do that in a more direct way to
> avoid overlooking that case in the future.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
> Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
> Reported-by: Thomas Martitz <[email protected]>
> Tested-by: Thomas Martitz <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/base/power/main.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/base/power/main.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/main.c
> +++ linux-pm/drivers/base/power/main.c
> @@ -1920,10 +1920,8 @@ static int device_prepare(struct device
>
> dev->power.wakeup_path = false;
>
> - if (dev->power.no_pm_callbacks) {
> - ret = 1; /* Let device go direct_complete */
> + if (dev->power.no_pm_callbacks)
> goto unlock;
> - }
>
> if (dev->pm_domain)
> callback = dev->pm_domain->ops.prepare;
> @@ -1957,7 +1955,8 @@ unlock:
> */
> spin_lock_irq(&dev->power.lock);
> dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
> - pm_runtime_suspended(dev) && ret > 0 &&
> + ((pm_runtime_suspended(dev) && ret > 0) ||
> + dev->power.no_pm_callbacks) &&
> !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
> spin_unlock_irq(&dev->power.lock);
> return 0;
>
On Tuesday, May 22, 2018 1:12:40 PM CEST Thomas Martitz wrote:
> Hello,
>
> thanks for for your effort and the patch.
>
> Is this eligible for stable?
Yes, it is.
Thanks,
Rafael
> Am 22.05.2018 um 13:02 schrieb Rafael J. Wysocki:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
> > driver flags) inadvertently prevented the power.direct_complete flag
> > from being set for devices without PM callbacks and with disabled
> > runtime PM which also prevents power.direct_complete from being set
> > for their parents. That led to problems including a resume crash on
> > HP ZBook 14u.
> >
> > Restore the previous behavior by causing power.direct_complete to be
> > set for those devices again, but do that in a more direct way to
> > avoid overlooking that case in the future.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
> > Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
> > Reported-by: Thomas Martitz <[email protected]>
> > Tested-by: Thomas Martitz <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/base/power/main.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > Index: linux-pm/drivers/base/power/main.c
> > ===================================================================
> > --- linux-pm.orig/drivers/base/power/main.c
> > +++ linux-pm/drivers/base/power/main.c
> > @@ -1920,10 +1920,8 @@ static int device_prepare(struct device
> >
> > dev->power.wakeup_path = false;
> >
> > - if (dev->power.no_pm_callbacks) {
> > - ret = 1; /* Let device go direct_complete */
> > + if (dev->power.no_pm_callbacks)
> > goto unlock;
> > - }
> >
> > if (dev->pm_domain)
> > callback = dev->pm_domain->ops.prepare;
> > @@ -1957,7 +1955,8 @@ unlock:
> > */
> > spin_lock_irq(&dev->power.lock);
> > dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
> > - pm_runtime_suspended(dev) && ret > 0 &&
> > + ((pm_runtime_suspended(dev) && ret > 0) ||
> > + dev->power.no_pm_callbacks) &&
> > !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
> > spin_unlock_irq(&dev->power.lock);
> > return 0;
> >
>
>
On 22 May 2018 at 13:02, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
> driver flags) inadvertently prevented the power.direct_complete flag
> from being set for devices without PM callbacks and with disabled
> runtime PM which also prevents power.direct_complete from being set
> for their parents. That led to problems including a resume crash on
> HP ZBook 14u.
>
> Restore the previous behavior by causing power.direct_complete to be
> set for those devices again, but do that in a more direct way to
> avoid overlooking that case in the future.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
> Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
> Reported-by: Thomas Martitz <[email protected]>
> Tested-by: Thomas Martitz <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
It seems like the resume path of HP ZBook 14u is kind of fragile, in
case it *requires* dev->power.direct_complete to be set for devices
like these. And that makes me wonder, that perhaps we should try to
address that issue as well, no?
In either way:
Reviewed-by: Ulf Hansson <[email protected]>
Kind regards
Uffe
> ---
> drivers/base/power/main.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/base/power/main.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/main.c
> +++ linux-pm/drivers/base/power/main.c
> @@ -1920,10 +1920,8 @@ static int device_prepare(struct device
>
> dev->power.wakeup_path = false;
>
> - if (dev->power.no_pm_callbacks) {
> - ret = 1; /* Let device go direct_complete */
> + if (dev->power.no_pm_callbacks)
> goto unlock;
> - }
>
> if (dev->pm_domain)
> callback = dev->pm_domain->ops.prepare;
> @@ -1957,7 +1955,8 @@ unlock:
> */
> spin_lock_irq(&dev->power.lock);
> dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
> - pm_runtime_suspended(dev) && ret > 0 &&
> + ((pm_runtime_suspended(dev) && ret > 0) ||
> + dev->power.no_pm_callbacks) &&
> !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
> spin_unlock_irq(&dev->power.lock);
> return 0;
>
On Tue, May 22, 2018 at 01:02:17PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
> driver flags) inadvertently prevented the power.direct_complete flag
> from being set for devices without PM callbacks and with disabled
> runtime PM which also prevents power.direct_complete from being set
> for their parents. That led to problems including a resume crash on
> HP ZBook 14u.
>
> Restore the previous behavior by causing power.direct_complete to be
> set for those devices again, but do that in a more direct way to
> avoid overlooking that case in the future.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
> Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
I stumbled over this the other day as well and tracked it down to the
above commit. In my case (child devices to serdev clients), this was
mostly benign although it did prevent the direct-complete optimisation
during suspend.
Never got around to reporting or fixing it myself, but your analysis and
fix matches my initial findings.
> Reported-by: Thomas Martitz <[email protected]>
> Tested-by: Thomas Martitz <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Johan Hovold <[email protected]>
As already suggested elsewhere in the thread, I think a stable is
warranted too.
Thanks,
Johan
On Tuesday, May 22, 2018 1:41:06 PM CEST Ulf Hansson wrote:
> On 22 May 2018 at 13:02, Rafael J. Wysocki <[email protected]> wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Commit 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE
> > driver flags) inadvertently prevented the power.direct_complete flag
> > from being set for devices without PM callbacks and with disabled
> > runtime PM which also prevents power.direct_complete from being set
> > for their parents. That led to problems including a resume crash on
> > HP ZBook 14u.
> >
> > Restore the previous behavior by causing power.direct_complete to be
> > set for those devices again, but do that in a more direct way to
> > avoid overlooking that case in the future.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199693
> > Fixes: 08810a4119aa (PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags)
> > Reported-by: Thomas Martitz <[email protected]>
> > Tested-by: Thomas Martitz <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> It seems like the resume path of HP ZBook 14u is kind of fragile,
Yes, it is.
> in case it *requires* dev->power.direct_complete to be set for devices
> like these. And that makes me wonder, that perhaps we should try to
> address that issue as well, no?
Yes, in principle.
But since the direct_complete handling needs to be fixed anyway, it doesn't
matter a lot in practice, because the resume issue on HP ZBook 14u will not
be reproducible anyway then. And since the dependency clearly is on a device
with no callbacks, I'm not worried too much about that.