Re: Khubd calls disconnect() after driver module unloaded

On Tue, 13 Nov 2007, David Woodhouse wrote:

> On Tue, 2007-11-13 at 19:16 -0500, David Woodhouse wrote:
> > It seems that driver_detach() doesn't find any devices to detach _from_
> > (there were printks there for when it called a remove function, _and_
> > for when it didn't because the driver didn't match).
> > 
> > drv->klist_devices.k_list is empty already.
> 
> ... because we removed the device from the driver's list _before_
> calling the driver's ->remove() function. And although we're holding a
> refcount on the driver, that doesn't really help because the driver
> module is already in the process of being unloaded.
> 
> This patch 'fixes' it. I've no idea if it's the right fix though, having
> not paid much attention to the locking issues here... 

I think this is on the right track.  Yes, you found a pretty subtle
race.  It looks like there's no point to the get_driver() and
put_driver() calls here, and they should be removed.

> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index b0088b0..90ab311 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -301,7 +301,6 @@ static void __device_release_driver(struct device * dev)
>  		get_driver(drv);
>  		driver_sysfs_remove(dev);
>  		sysfs_remove_link(&dev->kobj, "driver");
> -		klist_remove(&dev->knode_driver);
>  
>  		if (dev->bus)
>  			blocking_notifier_call_chain(&dev->bus->bus_notifier,
> @@ -314,6 +313,7 @@ static void __device_release_driver(struct device * dev)
>  			drv->remove(dev);
>  		devres_release_all(dev);
>  		dev->driver = NULL;
> +		klist_remove(&dev->knode_driver);
>  		put_driver(drv);
>  	}
>  }

Right; dev->knode_driver shouldn't be removed from its klist until the 
driver is no longer accessible through the device -- which means until 
after the remove method has returned.  The existing locking should take 
care of everything else.

An even more intricate case to consider is what happens when a new 
device is added just as the driver is being unloaded!  I think we're 
okay there because the driver gets removed from the bus's klist 
(preventing any new probing) _before_ it gets detached from all its 
devices.

> Another fix might be for driver_detach() to actually wait until the use
> count on the driver becomes zero.

It used to be that struct device_driver included a completion which was
used for exactly this purpose (although the wait_for_completion call
wasn't in driver_detach() but somewhere higher up the call stack).  I
don't understand why it's gone now, and it isn't clear what -- if
anything! -- prevents drivers from having a positive refcount even
after their module count has dropped to 0.

Alan Stern



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel