Re: [linux-pm] runtime PM usage_count during driver_probe_device()?

[Kevin Hilman <khilman@ti.com>]

Alan Stern <stern@rowland.harvard.edu> writes:

> On Fri, 1 Jul 2011, Kevin Hilman wrote:
>
>> >> --- a/drivers/base/dd.c
>> >> +++ b/drivers/base/dd.c
>> >> @@ -329,13 +329,13 @@ static void __device_release_driver(struct device *dev)
>> >>  			blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>> >>  						     BUS_NOTIFY_UNBIND_DRIVER,
>> >>  						     dev);
>> >> -
>> >> -		pm_runtime_put_sync(dev);
>> >> -
>> >>  		if (dev->bus && dev->bus->remove)
>> >>  			dev->bus->remove(dev);
>> >>  		else if (drv->remove)
>> >>  			drv->remove(dev);
>> >> +
>> >> +		pm_runtime_put_sync(dev);
>> >> +
>> >>  		devres_release_all(dev);
>> >>  		dev->driver = NULL;
>> >>  		klist_remove(&dev->p->knode_driver);
>> >
>> > To be safer, the put_sync() call should be moved down here.  Or maybe 
>> > even after the blocking_notifier_call_chain() that occurs here.
>> 
>> I was actually thinking about the other direction: moving the get_sync
>> after the first notifier chain.  IOW, the get_sync/put_sync only
>> protects the ->remove() calls, not the notifiers.
>> 
>> The protection around the notifiers doesn't make sense to me, at least
>> in the context of driver runtime PM racing with the subsystem.
>> Especially since these notifiers are likely how the
>> subsystem/bus/pm_domain code getting notified that there may be a device
>> to manage in the first place.
>
> The get_sync part doesn't matter so much.  Moving it past the notifier 
> call would probably be okay -- unless one of the listeners on the 
> notifier chain expects the device to be active.  Changing the get_sync 
> to get_noresume would probably also be okay -- subject to a similar 
> reservation.

There are enough "probably"s in the above to make me a bit uncomfortable
making this change.  Maybe you can take this patch forward?

Kevin

> The problem with the put_sync isn't the notifier.  If you leave it
> where you've got it now, you'll end up invoking a callback at a time
> when the driver thinks it no longer controls the device but the
> driver-model core still thinks it does.  You certainly want to do the
>
> 	dev->driver = NULL;
>
> first.
>
> Alan Stern