linux-kernel - Re: [PATCH 2/2] PM / runtime: Rework pm_runtime_force

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]

Date:   Fri, 12 Jan 2018 14:59:38 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Linux PM <linux-pm@...r.kernel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH 2/2] PM / runtime: Rework pm_runtime_force_suspend/resume()

On 12 January 2018 at 14:12, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> One of the limitations of pm_runtime_force_suspend/resume() is that
> if a parent driver wants to use these functions, all of its child
> drivers generally have to do that too because of the parent usage
> counter manipulations necessary to get the correct state of the parent
> during system-wide transitions to the working state (system resume).
> However, that limitation turns out to be artificial, so remove it.

According to my comment on the other thread, this stands true in case
the child is managed by runtime PM as well.

Otherwise this looks good to me.

>
> Namely, pm_runtime_force_suspend() only needs to update the children
> counter of its parent (if there's is a parent) when the device can
> stay in suspend after the subsequent system resume transition, as
> that counter is correct already otherwise.  Now, if the parent's
> children counter is not updated, it is not necessary to increment
> the parent's usage counter in that case any more, as long as the
> children counters of devices are checked along with their usage
> counters in order to decide whether or not the devices may be left
> in suspend after the subsequent system resume transition.
>
> Accordingly, modify pm_runtime_force_suspend() to only call
> pm_runtime_set_suspended() for devices whose usage and children
> counters are at the "no references" level (the runtime PM status
> of the device needs to be updated to "suspended" anyway in case
> this function is called once again for the same device during the
> transition under way), drop the parent usage counter incrementation
> from it and update pm_runtime_force_resume() to compensate for these
> changes.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
>  drivers/base/power/runtime.c |   74 +++++++++++++++++++------------------------
>  1 file changed, 34 insertions(+), 40 deletions(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -1613,17 +1613,28 @@ void pm_runtime_drop_link(struct device
>         spin_unlock_irq(&dev->power.lock);
>  }
>
> +static bool pm_runtime_need_not_resume(struct device *dev)
> +{
> +       return atomic_read(&dev->power.usage_count) <= 1 &&
> +               atomic_read(&dev->power.child_count) == 0;

How about adding an additional patch on top taking into account the
ignore_children flag and folding that into the series, kind of as you
also suggested?

My point is, we might as well take the opportunity to fix this right
away, don't you think?

[...]

Kind regards
Uffe