linux-kernel - Re: [PATCH 3/3] PM / Domains: Add support for PM domain on/off notifiers for genpd

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: <CAPDyKFpyNXgPveqUEha58S977k5=bX3P9h8raExEqRS-pZvvBQ@mail.gmail.com>
Date:   Tue, 22 Sep 2020 09:45:06 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Lina Iyer <ilina@...eaurora.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Lukasz Luba <lukasz.luba@....com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Benjamin Gaignard <benjamin.gaignard@...com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] PM / Domains: Add support for PM domain on/off
 notifiers for genpd

On Mon, 21 Sep 2020 at 18:11, Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Wed, Aug 26, 2020 at 8:38 AM Ulf Hansson <ulf.hansson@...aro.org> wrote:
> >
> > On Tue, 25 Aug 2020 at 18:16, Lina Iyer <ilina@...eaurora.org> wrote:
> > >
> > > On Wed, Aug 19 2020 at 04:41 -0600, Ulf Hansson wrote:
> > > >A device may have specific HW constraints that must be obeyed to, before
> > > >its corresponding PM domain (genpd) can be powered off - and vice verse at
> > > >power on. These constraints can't be managed through the regular runtime PM
> > > >based deployment for a device, because the access pattern for it, isn't
> > > >always request based. In other words, using the runtime PM callbacks to
> > > >deal with the constraints doesn't work for these cases.
> > > >
> > > >For these reasons, let's instead add a PM domain power on/off notification
> > > >mechanism to genpd. To add/remove a notifier for a device, the device must
> > > >already have been attached to the genpd, which also means that it needs to
> > > >be a part of the PM domain topology.
> > > >
> > > >To add/remove a notifier, let's introduce two genpd specific functions:
> > > > - dev_pm_genpd_add|remove_notifier()
> > > >
> > > >Note that, to further clarify when genpd power on/off notifiers may be
> > > >used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT
> > > >notifiers. In the long run, the genpd power on/off notifiers should be able
> > > >to replace them, but that requires additional genpd based platform support
> > > >for the current users.
> > > >
> > > >Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> > > >---
> > > > drivers/base/power/domain.c | 130 ++++++++++++++++++++++++++++++++++--
> > > > include/linux/pm_domain.h   |  15 +++++
> > > > 2 files changed, 141 insertions(+), 4 deletions(-)
> > > >
> > > >diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > > >index 4b787e1ff188..9cb85a5e8342 100644
> > > >--- a/drivers/base/power/domain.c
> > > >+++ b/drivers/base/power/domain.c
> > > >@@ -545,13 +545,21 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> > > >       if (!genpd->gov)
> > > >               genpd->state_idx = 0;
> > > >
> > > >+      /* Notify consumers that we are about to power off. */
> > > >+      ret = raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_OFF,
> > > >+                                    NULL);
> > > >+      if (ret)
> > > >+              return ret;
> > > >+
> > > >       /* Don't power off, if a child domain is waiting to power on. */
> > > >-      if (atomic_read(&genpd->sd_count) > 0)
> > > >-              return -EBUSY;
> > > >+      if (atomic_read(&genpd->sd_count) > 0) {
> > > >+              ret = -EBUSY;
> > > >+              goto busy;
> > > >+      }
> > > >
> > > >       ret = _genpd_power_off(genpd, true);
> > > >       if (ret)
> > > >-              return ret;
> > > >+              goto busy;
> > > >
> > > >       genpd->status = GENPD_STATE_OFF;
> > > >       genpd_update_accounting(genpd);
> > > >@@ -564,6 +572,9 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> > > >       }
> > > >
> > > >       return 0;
> > > >+busy:
> > > >+      raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL);
> > > It would be helpful to abstract these notification related calls into
> > > functions of their own. So, for CPU PM domains, it would help to add
> > > RCU_NONIDLE() around the notifiers, allowing the callbacks to add trace
> > > functions.
> >
> > Thanks for the suggestion! It makes perfect sense to me - and would
> > also be consistent with how CPU PM notifiers are managed,
>
> So I thought that you wanted to send a v2, but I cannot find it.

Yes, I am about to post it. I just wanted to give people more time to comment.

Kind regards
Uffe