| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0j+5ZJ3Za5pTiPe9NCRa3CV_KpaBX833vL=WH0tsTBF1g@mail.gmail.com>
Date: Thu, 22 Sep 2022 19:37:45 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Douglas Anderson <dianders@...omium.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Stephen Boyd <sboyd@...nel.org>, Pavel Machek <pavel@....cz>,
Linux PM <linux-pm@...r.kernel.org>,
Len Brown <len.brown@...el.com>,
linux-clk <linux-clk@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
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@...omium.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@...omium.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
>
Powered by blists - more mailing lists