From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: use of pm_runtime_disable() from driver probe? Date: Wed, 11 Jul 2012 19:36:45 +0200 Message-ID: <201207111936.45639.rjw@sisk.pl> References: Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Alan Stern , Kevin Hilman Cc: linux-pm@lists.linux-foundation.org List-Id: linux-pm@vger.kernel.org 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);