Messages in this thread Patch in this message | | | From | "Rafael J. Wysocki" <> | Date | Fri, 23 Apr 2021 16:08:23 +0200 | Subject | Re: [PATCH] PM: runtime: document common mistake with pm_runtime_get_sync() |
| |
On Thu, Apr 22, 2021 at 6:46 PM Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > pm_runtime_get_sync(), contradictory to intuition, does not drop the > runtime PM usage counter on errors which lead to several wrong usages in > drivers (missing the put). pm_runtime_resume_and_get() was added as a > better implementation so document the preference of using it, hoping it > will stop bad patterns. > > Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > --- > Documentation/power/runtime_pm.rst | 4 +++- > include/linux/pm_runtime.h | 3 +++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst > index 18ae21bf7f92..478f08942811 100644 > --- a/Documentation/power/runtime_pm.rst > +++ b/Documentation/power/runtime_pm.rst > @@ -378,7 +378,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h: > > `int pm_runtime_get_sync(struct device *dev);` > - increment the device's usage counter, run pm_runtime_resume(dev) and > - return its result > + return its result; > + be aware that it does not drop the device's usage counter on errors so > + usage of pm_runtime_resume_and_get(dev) usually results in cleaner code
Whether or not it results in cleaner code depends on what that code does.
If the code is
pm_runtime_get_sync(dev);
<Do something that will fail if the device is in a low-power state, but there is no way to handle the failure gracefully anyway>
pm_runtime_put(dev);
then having to use pm_runtime_resume_and_get() instead of the pm_runtime_get_sync() would be a nuisance.
However, if the code wants to check the return value, that is:
error = pm_runtime_resume_and_get(dev); if (error) return error;
<Do something that will crash and burn the system if the device is in a low-power state>
pm_runtime_put(dev);
then obviously pm_runtime_resume_and_get() should be your choice.
The rule of thumb seems to be whether or not the return value is going to be used.
> `int pm_runtime_get_if_in_use(struct device *dev);` > - return -EINVAL if 'power.disable_depth' is nonzero; otherwise, if the > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h > index 6c08a085367b..0dfd23d2cfc3 100644 > --- a/include/linux/pm_runtime.h > +++ b/include/linux/pm_runtime.h > @@ -380,6 +380,9 @@ static inline int pm_runtime_get(struct device *dev) > * The possible return values of this function are the same as for > * pm_runtime_resume() and the runtime PM usage counter of @dev remains > * incremented in all cases, even if it returns an error code. > + * Lack of decrementing the runtime PM usage counter on errors is a common > + * mistake, so consider using pm_runtime_resume_and_get() instead for a cleaner > + * code. > */ > static inline int pm_runtime_get_sync(struct device *dev) > { > -- > 2.25.1 >
| |