Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 22 Sep 2022 19:37:45 +0200 | Subject | Re: [RFC PATCH 1/2] PM: runtime: Add pm_runtime_try_put_sync() and pm_runtime_try_get_sync() |
| |
On Thu, Sep 22, 2022 at 5:44 PM Douglas Anderson <dianders@chromium.org> wrote: > > In some cases, a caller may wish to synchronously get or put the PM > Runtime state of a device but the caller may also be holding a > resource that the runtime suspend or runtime resume of the device > needs. Obviously this can lead to deadlock. > > A case in point is the clock framework. While running > clk_disable_unused() the clock framework holds the global clock > "prepare" lock. The clock framework then goes through and does PM > Runtime actions. It should be no surprise to anyone that some devices > need to prepare or unprepare clocks are part of their PM Runtime > actions. Things generally work OK because of the "recursive" nature of > the global clock "prepare" lock, but if we get unlucky and the PM > Runtime action is happening in another task then we can end up > deadlocking. > > Let's add a "try" version of the synchronous PM Runtime routines. > This version will return -EINPROGRESS rather than waiting. To > implement this we'll add a new flag: RPM_TRY. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > drivers/base/power/runtime.c | 7 +++++-- > include/linux/pm_runtime.h | 28 ++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > index 997be3ac20a7..67cc6a620b12 100644 > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -609,7 +609,7 @@ static int rpm_suspend(struct device *dev, int rpmflags) > if (dev->power.runtime_status == RPM_SUSPENDING) { > DEFINE_WAIT(wait); > > - if (rpmflags & (RPM_ASYNC | RPM_NOWAIT)) { > + if (rpmflags & (RPM_ASYNC | RPM_NOWAIT | RPM_TRY)) { > retval = -EINPROGRESS; > goto out; > } > @@ -791,7 +791,10 @@ static int rpm_resume(struct device *dev, int rpmflags) > || dev->power.runtime_status == RPM_SUSPENDING) { > DEFINE_WAIT(wait); > > - if (rpmflags & (RPM_ASYNC | RPM_NOWAIT)) { > + if (rpmflags & RPM_TRY) { > + retval = -EINPROGRESS;
Returning -EINPROGRESS from here may be misleading, because the device may not be resuming.
Besides, I'm not sure why a new flag is needed. What about using RPM_NOWAIT instead?
> + goto out; > + } else if (rpmflags & (RPM_ASYNC | RPM_NOWAIT)) { > if (dev->power.runtime_status == RPM_SUSPENDING) > dev->power.deferred_resume = true; > else > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h > index 0a41b2dcccad..c68baa63f0e7 100644 > --- a/include/linux/pm_runtime.h > +++ b/include/linux/pm_runtime.h > @@ -21,6 +21,8 @@ > #define RPM_GET_PUT 0x04 /* Increment/decrement the > usage_count */ > #define RPM_AUTO 0x08 /* Use autosuspend_delay */ > +#define RPM_TRY 0x10 /* Try to be synchronous but fail > + with an error if we can't. */ > > /* > * Use this for defining a set of PM operations to be used in all situations > @@ -425,6 +427,19 @@ static inline int pm_runtime_get_sync(struct device *dev) > return __pm_runtime_resume(dev, RPM_GET_PUT); > } > > +/** > + * pm_runtime_try_get_sync - Like pm_runtime_get_sync() but err if blocking > + * @dev: Target device. > + * > + * This function works just like pm_runtime_get_sync() except that if the > + * device in question is currently in the process of suspending or resuming > + * that it will return with -EINPROGRESS instead of blocking. > + */ > +static inline int pm_runtime_try_get_sync(struct device *dev) > +{ > + return __pm_runtime_resume(dev, RPM_GET_PUT | RPM_TRY); > +} > + > /** > * pm_runtime_resume_and_get - Bump up usage counter of a device and resume it. > * @dev: Target device. > @@ -489,6 +504,19 @@ static inline int pm_runtime_put_sync(struct device *dev) > return __pm_runtime_idle(dev, RPM_GET_PUT); > } > > +/** > + * pm_runtime_try_put_sync - Like pm_runtime_put_sync() but err if blocking > + * @dev: Target device. > + * > + * This function works just like pm_runtime_put_sync() except that if the > + * device in question is currently in the process of suspending that it will > + * return with -EINPROGRESS instead of blocking. > + */ > +static inline int pm_runtime_try_put_sync(struct device *dev) > +{ > + return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_TRY); > +} > + > /** > * pm_runtime_put_sync_suspend - Drop device usage counter and suspend if 0. > * @dev: Target device. > -- > 2.37.3.968.ga6b4b080e4-goog >
| |