linux-kernel - Re: [PATCH RFC] PM / Runtime: Rework the "runtime idle" helper routine

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]

Message-ID: <5291416.b4M4MTrzPC@vostro.rjw.lan>
Date:	Thu, 30 May 2013 21:55:03 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Linux PM list <linux-pm@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Linux PCI <linux-pci@...r.kernel.org>,
	Kevin Hilman <khilman@...aro.org>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH RFC] PM / Runtime: Rework the "runtime idle" helper routine

On Thursday, May 30, 2013 01:08:08 PM Alan Stern wrote:
> On Thu, 30 May 2013, Rafael J. Wysocki wrote:
> 
> > > Since you're making this change, wouldn't it be a good idea to adopt
> > > Mika's original suggestion and turn on the RPM_AUTO bit in rpmflags
> > > when the use_autosuspend flag is set?
> > 
> > I'm not actually sure.  It can be done, but I'd prefer to do that as a separate
> > change in any case.
> 
> That makes sense.
> 
> > > What about cases where the runtime-idle callback does
> > > rpm_schedule_suspend or rpm_request_suspend?  You'd have to make sure
> > > that it returns -EBUSY in such cases.  Did you audit for this?
> > 
> > As far as I could.
> > 
> > I'm not worried about the subsystems modified by this patch, because the
> > functionality there won't change (except for PCI, that is).
> 
> Right.  The subsystems that _aren't_ modified are the ones to worry 
> about -- like the USB callback.  They are the ones where the behavior 
> might change.

Right.

> > > > Index: linux-pm/Documentation/power/runtime_pm.txt
> > > > ===================================================================
> > > > --- linux-pm.orig/Documentation/power/runtime_pm.txt
> > > > +++ linux-pm/Documentation/power/runtime_pm.txt
> > > > @@ -660,11 +660,6 @@ Subsystems may wish to conserve code spa
> > > >  management callbacks provided by the PM core, defined in
> > > >  driver/base/power/generic_ops.c:
> > > >  
> > > > -  int pm_generic_runtime_idle(struct device *dev);
> > > > -    - invoke the ->runtime_idle() callback provided by the driver of this
> > > > -      device, if defined, and call pm_runtime_suspend() for this device if the
> > > > -      return value is 0 or the callback is not defined
> > > > -
> > > 
> > > The documentation for the runtime-idle callback needs to be updated too.
> > 
> > Well, I actually couldn't find the part of it that would need to be updated. :-)
> 
> How about this?

Looks good! :-)

May I add your sign-off to it?

Rafael


> Index: usb-3.10/Documentation/power/runtime_pm.txt
> ===================================================================
> --- usb-3.10.orig/Documentation/power/runtime_pm.txt
> +++ usb-3.10/Documentation/power/runtime_pm.txt
> @@ -144,8 +144,12 @@ The action performed by the idle callbac
>  (or driver) in question, but the expected and recommended action is to check
>  if the device can be suspended (i.e. if all of the conditions necessary for
>  suspending the device are satisfied) and to queue up a suspend request for the
> -device in that case.  The value returned by this callback is ignored by the PM
> -core.
> +device in that case.  If there is no idle callback, or if the callback returns
> +0, then the PM core will attempt to carry out a runtime suspend of the device;
> +in essence, it will call pm_runtime_suspend() directly.  To prevent this (for
> +example, if the callback routine has started a delayed suspend), the routine
> +should return a non-zero value.  Negative error return codes are ignored by the
> +PM core.
>  
>  The helper functions provided by the PM core, described in Section 4, guarantee
>  that the following constraints are met with respect to runtime PM callbacks for
> @@ -301,9 +305,10 @@ drivers/base/power/runtime.c and include
>        removing the device from device hierarchy
>  
>    int pm_runtime_idle(struct device *dev);
> -    - execute the subsystem-level idle callback for the device; returns 0 on
> -      success or error code on failure, where -EINPROGRESS means that
> -      ->runtime_idle() is already being executed
> +    - execute the subsystem-level idle callback for the device; returns an
> +      error code on failure, where -EINPROGRESS means that ->runtime_idle() is
> +      already being executed; if there is no callback or the callback returns 0
> +      then run pm_runtime_suspend(dev) and return its result
>  
>    int pm_runtime_suspend(struct device *dev);
>      - execute the subsystem-level suspend callback for the device; returns 0 on
> 
> 
> Alan Stern
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/