Re: Do we need asynchronous pm_runtime_get()? (was: Re: bisected regression ...)

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Tuesday, July 31, 2012, Alan Stern wrote:
> [CC: list trimmed]
> 
> On Tue, 31 Jul 2012, Rafael J. Wysocki wrote:
> 
> > Now it occured to me that perhaps we don't need the current asynchronous
> > pm_runtime_get() at all.  The usefulness of it is quite questionable, because
> > either we want to resume the device immediately, for which pm_runtime_get_sync()
> > should be used, or we just want to bump up the usage counter, in which cases
> > pm_runtime_get_noresume() should always be sufficient.  I fail to see any
> > particularly compelling use case for pm_runtime_get() doing an asynchronous
> > resume at the moment, but perhaps that's just me.
> 
> There are indeed valid uses for pm_runtime_get().  We are forced to use
> it in non-sleepable contexts when we want to resume the device as
> quickly as possible.  Example: a driver receives an I/O request from an
> interrupt handler.

Is it actually suitable to be used in such contexts?  It doesn't give any
guarantee that the device will be active when it returns, so the caller can't
really rely on it.  The caller always has to check if the device has been
resumed before accessing it anyway, so it may as well do a _get_noresume(),
do the check and do pm_request_resume() if needed directly.

Now, as far as interrupt handlers are concerned, there are two cases: either
the device has to be active to generate an interrupt (like PCI), or the
interrupt is generated by something else on behalf of it.  We don't seem to
handle the first case very well right now, because in that case the interrupt
handler will always know that the device is active, so it should do a
_get_noresume() and then change the device's status to "active" without
calling any kind of "resume", but we don't provide a helper for that.  In the
second case calling pm_runtime_get() doesn't really help either.  I think it's
better to do _get_noresume(), check the status and if "suspended", set a flag
for the resume callback to do something specific before returning successfully,
then call pm_request_resume() and return.

> > However, I receive reports of people using pm_runtime_get() where they really
> > should use pm_runtime_get_sync(), so I wonder if we can simply rename
> > pm_runtime_get_sync() as pm_runtime_get() and drop the asynchronous version
> > altogether?
> 
> Well, IMO the naming should have been the other way around from the
> start.  That is, we should have made pm_runtime_get be the synchronous
> routine and pm_runtime_get_async be the asynchronous one.  But it's too
> late to change now.

I'm not sure it is too late.  If we first change all the instances of
pm_runtime_get() to pm_runtime_get_async() and then all of the instances of
pm_runtime_get_sync() to pm_runtime_get(), it should be technically possible.
Of course, it would be confusing, but that's a different matter. :-)

> And no, we can't get rid of the async version.

I'm still not sure of that.

Thanks,
Rafael