| Message ID | 20241114220921.2529905-2-saravanak@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | Optimize async device suspend/resume | expand |
On Thu, Nov 14, 2024 at 02:09:15PM -0800, Saravana Kannan wrote: > Some devices might have their is_suspended flag set to false. In these > cases, dpm_resume() should skip doing anything for those devices. > However, runtime PM enable and a few others steps are done before > checking for this flag. Fix it so that we do things in the right order. > > Signed-off-by: Saravana Kannan <saravanak@google.com> This looks like a nice generic fix as well, should it go to older kernels? thanks, greg k-h
On Fri, Nov 15, 2024 at 11:43 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Nov 14, 2024 at 02:09:15PM -0800, Saravana Kannan wrote: > > Some devices might have their is_suspended flag set to false. In these > > cases, dpm_resume() should skip doing anything for those devices. > > However, runtime PM enable and a few others steps are done before > > checking for this flag. Fix it so that we do things in the right order. > > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > This looks like a nice generic fix as well, should it go to older > kernels? Yeah, it's meant to be a generic fix. But I'll feel better about it if someone familiar with this code can give it a Reviewed-by. And as I look at it... I have a bug in there. I think it should be goto Complete and not Unlock! No idea how my devices didn't get freed after a few suspend aborts! I can send it out as a separate patch too if you want. And depending on when it lands, I can keep it or drop it from v2 of this series. Thanks, Saravana
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 4a67e83300e1..86e51b9fefab 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -913,6 +913,9 @@ static void device_resume(struct device *dev, pm_message_t state, bool async) if (dev->power.syscore) goto Complete; + if (!dev->power.is_suspended) + goto Unlock; + if (dev->power.direct_complete) { /* Match the pm_runtime_disable() in __device_suspend(). */ pm_runtime_enable(dev); @@ -931,9 +934,6 @@ static void device_resume(struct device *dev, pm_message_t state, bool async) */ dev->power.is_prepared = false; - if (!dev->power.is_suspended) - goto Unlock; - if (dev->pm_domain) { info = "power domain "; callback = pm_op(&dev->pm_domain->ops, state);
Some devices might have their is_suspended flag set to false. In these cases, dpm_resume() should skip doing anything for those devices. However, runtime PM enable and a few others steps are done before checking for this flag. Fix it so that we do things in the right order. Signed-off-by: Saravana Kannan <saravanak@google.com> --- drivers/base/power/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)