Re: [PATCH 1/3] PM / sleep: New flag to speed up suspend-resume of suspended devices

From: Alan Stern
Date: Wed Feb 26 2014 - 11:49:16 EST


On Wed, 26 Feb 2014, Rafael J. Wysocki wrote:

> > I admit, there most likely _are_ devices that would get into trouble if
> > the question ever did arise.
>
> Well, I kind of put that to a test by posting these two patches:
>
> https://patchwork.kernel.org/patch/3705261/
> https://patchwork.kernel.org/patch/3705271/
>
> We'll see if they lead to any regressions, but I'm going to work on top of
> them going forward anyway.

And here I had the impression that you wanted to avoid any regressions
from those patches...

> Actually, on top of the two patches mentioned above (and for devices
> without power.ignore_children set) the question reduces to whether or not
> (i) the device itself is runtime-suspended when its .suspend() callback is
> running and (ii) its power state is such that it can remain suspended.
> If both (i) and (ii) are met, the device may be left suspended safely,
> because if any of its children had depended on it, they would have resumed
> it already.

Does this mean you changed your mind? In an earlier email, you wrote:

> > (b) It's okay for the device's parent to be in runtime suspend
> > when the device's ->suspend callbacks are invoked.
> >
> > I included this just to be thorough. In fact, I expect (b) to be true
> > for pretty much every device already.
>
> I don't quite understand this. What if the parent is a bridge and the
> child's ->suspend tries to access the child's registers? That surely won't
> work if the parent is in a low-power state at that point.

So the answer is that if the bridge is suspended, then the child must
be suspended too and hence the child's ->suspend should _expect_
problems if it tries to access the child's registers.

(By the way, during this discussion I have had a tendency to mix up two
related concepts:

The device's ->suspend routine expects the _parent_ not to be
suspended;

The device's ->suspend routine expects the _device_ not to be
suspended.

Obviously the second implies the first. But once the second has been
fixed, the first should never cause any trouble.)

> Still, I think that something like power.fast_suspend is needed to indicate
> that .suspend_late(), .suspend_noirq(), .resume_noirq() and .resume_early()
> should be skipped for it (in my opinion the core may very well skip them then)
> and so that .resume() knows how to handle the device.

I don't follow. Why would you skip these routines without also
skipping .suspend and .resume?

> I generally agree that whether or not a device may be left suspended during and
> after system resume and whether or not a device may be left suspended during
> system suspend are two different questions. However, when it *is* left
> suspended during system suspend, then that implies certain way of handling it
> during the subsequent system resume. After which it still may not be left
> suspended.

I would prefer to say: "However, when the system suspend callbacks
_are_ skipped, that implies the corresponding system resume callbacks
must also be skipped and hence the device must remain suspended". Is
this consistent with what you meant?

As I see it, the fast_suspend implementation could lead to regressions
in two ways:

The child's ->suspend doesn't expect the parent to be
suspended.

The child's ->resume doesn't expect the parent to be
suspended.

We agree now that the first won't be a problem, because it would imply
the child is suspended too.

However, the second may indeed be a problem. I don't know how you
intend to handle it. Apply the patch, like you did for ACPI and PCI
above, and then see what happens?

A simple solution is to use fast_suspend only for devices that have no
children. But that would not be optimal.

Another possibility is always to call pm_runtime_resume(dev->parent)
before invoking dev's ->resume callback. But that might not solve the
entire problem (it wouldn't help dev's ->resume_early callback, for
instance) and it also might be sub-optimal.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/