linux-kernel - Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAPDyKFpNHL=tZYbQduDJqypz7rbwPwvW7sOymm-djbRSFgZ00g@mail.gmail.com>
Date:   Wed, 18 Oct 2017 14:34:10 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Linux PM <linux-pm@...r.kernel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linux Documentation <linux-doc@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kevin Hilman <khilman@...nel.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

[...]

>> Are there any major reasons why the appended patch (obviously untested) won't
>> work, then?
>
> OK, there is a reason, which is the optimizations bundled into
> pm_runtime_force_*, because (a) the device may be left in runtime suspend
> by them (in which case amba_pm_suspend_early() in my patch should not run)
> and (b) pm_runtime_force_resume() may decide to leave it suspended (in which
> case amba_pm_suspend_late() in my patch should not run).

Exactly.

>
> [BTW, the "leave the device suspended" optimization in pm_runtime_force_*
> is potentially problematic too, because it requires the children to do
> the right thing, which effectively means that their drivers need to use
> pm_runtime_force_* too, but what if they don't want to reuse their
> runtime PM callbacks for system-wide PM?]

Deployment of pm_runtime_force_suspend() should generally be done for
children devices first.

If some reason that isn't the case, it's expected that the call to
pm_runtime_set_suspended() invoked from pm_runtime_force_suspend(),
for the parent, should fail and thus abort system suspend.

>
> Honestly, I don't like the way this is designed.  IMO, it would be better
> to do the optimizations and all in the bus type middle-layer code instead
> of expecting drivers to use pm_runtime_force_* as their system-wide PM
> callbacks (and that expectation should at least be documented, which I'm
> not sure is the case now).  But whatever.
>
> It all should work the way it does now without pm_runtime_force_* if (a) the
> bus type's PM callbacks are changed like in the last patch and the drivers
> (b) point their system suspend callbacks to the runtime PM callback routines
> and (c) set DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED for the
> devices (if they need to do the PM in ->suspend and ->resume, they may set
> DPM_FLAG_AVOID_RPM too).
>
> And if you see a reason why that won't work, please let me know.

I will have look and try out the series by using my local "runtime PM
test driver".

I get back to you with an update on this.

Kind regards
Uffe