linux-kernel - Re: [PATCH v1] PM: sleep: core: Synchronize runtime PM status of parents and children

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: <CAJZ5v0gBCQW2wwdB+4SyBXtqiis2k1Z2L8SOVKwcVbNxPHqvYA@mail.gmail.com>
Date: Fri, 7 Feb 2025 19:14:27 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Johan Hovold <johan@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Jon Hunter <jonathanh@...dia.com>, 
	"Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, Alan Stern <stern@...land.harvard.edu>, 
	Bjorn Helgaas <helgaas@...nel.org>, Linux PCI <linux-pci@...r.kernel.org>, 
	Ulf Hansson <ulf.hansson@...aro.org>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, Kevin Xie <kevin.xie@...rfivetech.com>, 
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v1] PM: sleep: core: Synchronize runtime PM status of
 parents and children

On Fri, Feb 7, 2025 at 5:26 PM Johan Hovold <johan@...nel.org> wrote:
>
> On Fri, Feb 07, 2025 at 05:06:32PM +0100, Johan Hovold wrote:
> > On Fri, Feb 07, 2025 at 04:41:18PM +0100, Rafael J. Wysocki wrote:
> > > On Fri, Feb 7, 2025 at 3:45 PM Johan Hovold <johan@...nel.org> wrote:
> > > > On Fri, Feb 07, 2025 at 02:50:29PM +0100, Johan Hovold wrote:
> >
> > > > Ok, so the driver data is never set and runtime PM is never enabled for
> > > > this simple bus device, which uses pm_runtime_force_suspend() for system
> > > > sleep.
> > >
> > > This is kind of confusing.  Why use pm_runtime_force_suspend() if
> > > runtime PM is never enabled and cannot really work?
> >
> > It's only done for some buses that this driver handles. The driver is
> > buggy; I'm preparing a fix for it regardless of the correctness of the
> > commit that now triggered this.
>
> Hmm. The driver implementation is highly odd, but actually works as long
> as the runtime PM status is left at 'suspended' (as
> pm_runtime_force_resume() won't enable runtime PM unless it was enabled
> before suspend).
>
> So we'd strictly only need something like the below if we are going to
> keep the set_active propagation.

I think you are right.

> diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
> index 5dea31769f9a..d8e029e7e53f 100644
> --- a/drivers/bus/simple-pm-bus.c
> +++ b/drivers/bus/simple-pm-bus.c
> @@ -109,9 +109,29 @@ static int simple_pm_bus_runtime_resume(struct device *dev)
>         return 0;
>  }
>
> +static int simple_pm_bus_suspend(struct device *dev)
> +{
> +       struct simple_pm_bus *bus = dev_get_drvdata(dev);
> +
> +       if (!bus)
> +               return 0;
> +
> +       return pm_runtime_force_suspend(dev);
> +}
> +
> +static int simple_pm_bus_resume(struct device *dev)
> +{
> +       struct simple_pm_bus *bus = dev_get_drvdata(dev);
> +
> +       if (!bus)
> +               return 0;
> +
> +       return pm_runtime_force_resume(dev);
> +}
> +
>  static const struct dev_pm_ops simple_pm_bus_pm_ops = {
>         RUNTIME_PM_OPS(simple_pm_bus_runtime_suspend, simple_pm_bus_runtime_resume, NULL)
> -       NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> +       NOIRQ_SYSTEM_SLEEP_PM_OPS(simple_pm_bus_suspend, simple_pm_bus_resume)
>  };
>
>  #define ONLY_BUS       ((void *) 1) /* Match if the device is only a bus. */

In the meantime, I've cut the attached (untested) patch that should
take care of the pm_runtime_force_suspend() issue altogether.

It changes the code to only set the device's runtime PM status to
"active" if runtime PM is going to be enabled for it by the first
pm_runtime_enable() call, which never happens for devices where
runtime PM has never been enabled (because it is disabled for them
once again in device_suspend_late()) and for devices subject to
pm_runtime_force_suspend() during system suspend (because it disables
runtime PM for them once again).

View attachment "pm-runtime-cond-set-active.patch" of type "text/x-patch" (4946 bytes)