Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions

[Ohad Ben-Cohen <ohad@wizery.com>]

On Sat, Dec 11, 2010 at 12:59 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>>> > A different, bolder solution, will always call pm_runtime_idle instead
>>> > of pm_request_idle (see below): when a device is runtime suspended, it
>>> > can't be too bad to immediately send idle notification to its parent,
>>> > too. I'm aware this might not always be desirable, but I'm bringing
>>> > this up even just for the sake of discussion:
>>> >
>>> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
>>> > index 02c652b..9719811 100644
>>> > --- a/drivers/base/power/runtime.c
>>> > +++ b/drivers/base/power/runtime.c
>>> > @@ -407,7 +407,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
>>> >         if (parent && !parent->power.ignore_children) {
>>> >                 spin_unlock_irq(&dev->power.lock);
>>> >
>>> > -               pm_request_idle(parent);
>>> > +               pm_runtime_idle(parent);
>>> >
>>> >                 spin_lock_irq(&dev->power.lock);
>>> >         }
>>>
>>> I acutally think we can do that.  If the parent cannot be suspended,
>>> pm_runtime_idle() will just return, which is fine.  Right now I don't see
>>> any big drawback of this.
>>>
>>> Alan, what do you think?
>>
>> I don't like this change quite so much.  It can give rise to unbounded
>> nested suspend sequences: we suspend the device, then call the parent's
>> runtime_idle routine which suspends the parent, then we call the
>> grandparent's runtime_idle routine, etc.
>
> Yes, that's why I thought this might not be desirable, but I wasn't
> quite sure why would it be wrong; if a context is willing to
> synchronously suspend a device (either a driver using put_sync() or
> the PM worker), what do we gain by deferring the idling of the parent
> and not synchronously suspending as much ancestors as possible in the
> same context ?

By allowing nested suspend sequences we even squeeze some more power
by suspending devices earlier, and saving some CPU cycles (due to the
avoided context switches), so it sounds like a win-win...

But of course I may be overlooking a real problem this may pose.. I
haven't spotted any, but it does feel like a significant behavior
change, hence the cautious approach :)

Thanks,
Ohad.

>