linux-kernel - Re: [linux-pm] [RFC] PCI: Runtime power management

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: <200908141905.32766.rjw@sisk.pl>
Date:	Fri, 14 Aug 2009 19:05:32 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Matthew Garrett <mjg59@...f.ucam.org>, linux-usb@...r.kernel.org,
	linux-pci@...r.kernel.org, Greg KH <gregkh@...e.de>,
	LKML <linux-kernel@...r.kernel.org>,
	"Linux-pm mailing list" <linux-pm@...ts.linux-foundation.org>
Subject: Re: [linux-pm] [RFC] PCI: Runtime power management

On Friday 14 August 2009, Alan Stern wrote:
> On Thu, 13 Aug 2009, Matthew Garrett wrote:
> 
> > On Thu, Aug 13, 2009 at 11:22:44AM -0400, Alan Stern wrote:
> >
> > > You have to call the HCD's pci_suspend method!  Not to mention calling
> > > synchronize_irq and all the other stuff in hcd_pci_suspend and
> > > hcd_pci_suspend_noirq.
> >
> > The bus level code does this, assuming that the driver-level code
> > doesn't return an error.
> 
> So it does; my mistake.
> 
> 
> On Fri, 14 Aug 2009, Matthew Garrett wrote:
> 
> > On Thu, Aug 13, 2009 at 10:47:01PM +0100, Matthew Garrett wrote:
> > 
> > > Ugh. I'd really prefer us to assume that drivers are able to cope unless 
> > > proven otherwise. Userspace policy makes sense where we don't have any 
> > > idea whether something will work or not, but I'd really expect that most 
> > > PCI drivers will either cope (in which case they'll have enabling code) 
> > > or won't (in which case they won't). Why would we want userspace to 
> > > influence this?
> > 
> > Though, thinking about it, you're right that setting this does override 
> > user policy. I think we need an additional flag to indicate that the 
> > device supports runtime wakeup and test that as well when doing 
> > device_may_wakeup().
> 
> You are suggesting separate flag sets for system-wide wakeup and
> runtime wakeup?  I don't disagree, but implementing them will be
> problematical.
> 
> That's because it's not always possible to change a device's wakeup 
> setting while it is suspended.  Thus if a device was runtime suspended 
> with wakeup enabled, and then we want to do a system sleep and change 
> the device's wakeup setting to disabled, we would have to wake the 
> device back up in order to do it.
> 
> 
> > > > This misses the point.  The whole idea of runtime_idle is to tell you 
> > > > that the device is idle and might be ready to be suspended.  If you're 
> > > > going to call pm_schedule_suspend anyway, there's no reason to invoke 
> > > > pm->runtime_idle.
> > > 
> > > My understanding of the API was that pm_device_put() invokes 
> > > runtime_idle if the refcount hits 0. The bus layer has no idea of the 
> > > refcount, and calling suspend directly from the driver would defeat the 
> > > point of the system-wide recounting.
> > 
> > From the API docs:
> > 
> > "The action performed by a bus type's ->runtime_idle() callback is 
> > totally dependent on the bus type 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."
> > 
> > Though perhaps the device level runtime_idle shouldn't be void - that 
> > way the bus can ask the driver whether its suspend conditions have been 
> > satisfied? Right now there doesn't seem to be any way for the bus to ask 
> > that.
> 
> If you want to get the device-level runtime_idle involved, you can make
> _it_ responsible for scheduling the suspend.  Then the bus-level code
> simply has to check whether everything is okay at the bus level, and if
> it is, call the device-level routine.
> 
> However changing the return type wouldn't hurt anything, and it would 
> allow the pm_schedule_suspend call to be centralized in the bus code.  
> You could ask Rafael about it, or just send him a patch.

Well, I'm not against that, but what should pm_runtime_idle() do with the
result returned by it?  Just pass it to the caller?

Rafael
--
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/