Re: use of pm_runtime_disable() from driver probe?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday, July 11, 2012, Alan Stern wrote:
> On Wed, 11 Jul 2012, Rafael J. Wysocki wrote:
> 
> > > This opens up the possibility of calling probe while a runtime resume
> > > or suspend is in progress.  (On the other hand, the existing code
> > > doesn't prevent a concurrent runtime resume.)  Maybe it would be best
> > > to leave the pm_runtime_barrier().
> > 
> > That wouldn't close the race, though, because the suspend/resume may still
> > be started right after _barrier has returned.
> 
> True, but see below.
> 
> > The race is only possible if runtime PM is enabled by a subsystem or PM domain
> > code before the first eligible driver is registered and if that code is not
> > careful enough to get ready for driver registration.  I'm not sure how likely
> > it is to happen in practice.
> 
> It's not just the first eligible driver.  Drivers can be bound and 
> unbound dynamically, and suspends/resume operations can sit on the wait 
> queue or wait until a timer expires.  We don't want an old request 
> suddenly to take effect in the middle of a probe.
> 
> The barrier will get rid of any old requests.  New ones would have to 
> be added after the probe starts, which as you say, is unlikely.

OK, I'll keep the barrier, then.

Kevin, can you please double check the patch below?

Rafael

---
 drivers/base/dd.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux/drivers/base/dd.c
===================================================================
--- linux.orig/drivers/base/dd.c
+++ linux/drivers/base/dd.c
@@ -356,10 +356,9 @@ int driver_probe_device(struct device_dr
 	pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
 		 drv->bus->name, __func__, dev_name(dev), drv->name);
 
-	pm_runtime_get_noresume(dev);
 	pm_runtime_barrier(dev);
 	ret = really_probe(dev, drv);
-	pm_runtime_put_sync(dev);
+	pm_runtime_idle(dev);
 
 	return ret;
 }
@@ -406,9 +405,8 @@ int device_attach(struct device *dev)
 			ret = 0;
 		}
 	} else {
-		pm_runtime_get_noresume(dev);
 		ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
-		pm_runtime_put_sync(dev);
+		pm_runtime_idle(dev);
 	}
 out_unlock:
 	device_unlock(dev);


[Index of Archives]     [Linux ACPI]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [CPU Freq]     [Kernel Newbies]     [Fedora Kernel]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux