linux-kernel - Re: [PATCH] PM: runtime: Use pmruntime sync variant to put suppliers

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0htFwyDhJS3spKf73v=PX5dTCT2+Piotf4=NyGe+9g6Tw@mail.gmail.com>
Date:   Thu, 5 Nov 2020 18:42:20 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH] PM: runtime: Use pmruntime sync variant to put suppliers

On Thu, Oct 8, 2020 at 3:08 AM Stanimir Varbanov
<stanimir.varbanov@...aro.org> wrote:
>
> Hi Rafael,
>
> On 10/7/20 5:37 PM, Rafael J. Wysocki wrote:
> > On Wed, Oct 7, 2020 at 2:20 AM Stanimir Varbanov
> > <stanimir.varbanov@...aro.org> wrote:
> >>
> >> Calling pm_runtime_put_sync over a device with suppliers with device
> >> link flags PM_RUNTIME | RPM_ACTIVE it is observed that the supplier
> >> is not put (turned off) at the end, but instead put asynchronously.
> >
> > Yes, that's by design.
> >
> >> In some case This could lead to issues for the callers which expects
> >> that the pmruntime sync variants should also put the suppliers
> >> synchronously.
> >
> > Why would anyone expect that to happen?
>
> It is logical to me that when I call pm_runtime_put_sync the device and
> its suppliers are put synchronously. If I want to put device and its
> suppliers asynchronously I'd use pm_runtime_put. Is that wrong assumption?

The handling of suppliers is analogous to the handling of parents and
the parents are suspended asynchronously when a child suspends.

The difference between _put() and _put_sync() only applies to the
device passed in as the argument.

> >> Also the opposite rpm_get_suppliers is already using pmruntime _sync
> >> variant of the API.
> >
> > Yes, it does, because that is necessary.
> >
> >> Correct this by changing pmruntime_put to pmruntime_put_sync in
> >> rpm_put_suppliers.
> >
> > It is not a correction, but a change in behavior without good enough
> > rationale, as it stands.
>
> In my driver case I want to deal with a recovery of a crash in remote
> processor (the remote processor is used to control and program hardware
> blocks and also to communicate with host processor through shared
> memory). To restart the remote processor I have to disable clocks and
> turn off few power domains (one of the power domains is made a supplier
> of my main device) in order to complete the cold-boot.

PM-runtime doesn't guarantee you the behavior that you'd like to see here.

> The problem I'm facing with this design is that when I call
> runtime_put_sync (to disable device's clocks and turn off power domain)
> the clocks are disabled (part of pmruntime_suspend callback) but the
> pmdomain (the device supplier) is not turned synchronously. I workaround
> this by checking the supplier device via pm_runtime_active() until it
> becomes inactive and the continue with rest of the steps.

This is not a use case for PM-runtime at all.

PM-runtime is all about going low-power opportunistically, whereas you
want to enforce power down.

> From my point of view this check for supplier activity should be part of
> pmruntime API.

But the API is not what you should be using for this purpose in the first place.