linux-kernel - Re: [PATCH v2 08/10] PM core: Fix restoring devices with links during system PM transition

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1509709.za4jzbeXig@vostro.rjw.lan>
Date:   Wed, 07 Sep 2016 01:24:40 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Marek Szyprowski <m.szyprowski@...sung.com>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org,
        linux-samsung-soc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Joerg Roedel <joro@...tes.org>,
        Inki Dae <inki.dae@...sung.com>, Kukjin Kim <kgene@...nel.org>,
        Krzysztof Kozlowski <k.kozlowski@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v2 08/10] PM core: Fix restoring devices with links during system PM transition

On Friday, June 17, 2016 08:26:58 AM Marek Szyprowski wrote:
> When devices are being runtime resumed during the system PM transition to
> suspend state, the link suppliers might be already resumed and
> have runtime pm disabled. This is normal case. This patch adds special
> support for such case. Simple call to pm_runtime_get_sync returns error
> when device has runtime PM disabled, what results in incorrect runtime PM
> state during system wide PM transition.

The problem is real, but the proposed solution is questionable.

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@...sung.com>
> ---
>  drivers/base/power/runtime.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 0ea00d442e0f..d00b079fad88 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -261,6 +261,26 @@ static int rpm_check_suspend_allowed(struct device *dev)
>  }
>  
>  /**
> + * __pm_runtime_force_resume - force resume of the device
> + * @dev: Device to resume
> + *
> + * This function works like __pm_runtime_resume(dev, RPM_GET_PUT), but
> + * also handles devices with runtime PM disabled. This allows to properly
> + * control all devices during preparation for system PM transition.
> + */
> +static int __pm_runtime_force_resume(struct device *dev)
> +{
> +	if (!dev->power.disable_depth)
> +		return pm_runtime_get_sync(dev);
> +
> +	if (dev->power.runtime_status != RPM_ACTIVE ||
> +	    atomic_inc_not_zero(&dev->power.usage_count) == 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/**
>   * __rpm_callback - Run a given runtime PM callback for a given device.
>   * @cb: Runtime PM callback to run.
>   * @dev: Device to run the callback for.
> @@ -291,7 +311,7 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
>  				if ((link->flags & DEVICE_LINK_PM_RUNTIME)
>  				    && link->status != DEVICE_LINK_SUPPLIER_UNBIND
>  				    && !link->rpm_active) {
> -					retval = pm_runtime_get_sync(link->supplier);
> +					retval = __pm_runtime_force_resume(link->supplier);
>  					if (retval < 0) {
>  						pm_runtime_put_noidle(link->supplier);
>  						goto fail;

The reason why power.disable_depth is different from 0 may not be system suspend,
in which case it should fail even if the status happens to be RPM_ACTIVE.

Thanks,
Rafael