Roman, Pat:

Here is a fix for driver_detach().  It's a little ugly because it avoids
using the klist iterator, but there's no way around it -- the iterator
simply can't be made to work here.  Not only does it prevent
device_release_driver() from calling klist_remove(), but also it prevents
doing get_device() while holding the klist spinlock.

With this patch in place, the code in usb_driver_release_interface() 
doesn't really need to be changed.  However I think it should be.  As it 
is now, it relies on internal knowledge of how the driver-model core 
works, namely that dev->knode_driver is released _before_ drv->remove() 
gets called.  They could easily go the other way around.

What do you think?

Alan Stern



Index: driver-2.6/drivers/base/dd.c
===================================================================
--- driver-2.6.orig/drivers/base/dd.c
+++ driver-2.6/drivers/base/dd.c
@@ -184,37 +184,37 @@ void driver_attach(struct device_driver 
  *     @dev:   device.
  *
  *     Manually detach device from driver.
- *     Note that this is called without incrementing the bus
- *     reference count nor taking the bus's rwsem. Be sure that
- *     those are accounted for before calling this function.
+ *
+ *     __device_release_driver() must be called with @dev->sem held.
  */
 
-void device_release_driver(struct device * dev)
+static void __device_release_driver(struct device * dev)
 {
        struct device_driver * drv;
 
-       down(&dev->sem);
-       if (dev->driver) {
-               drv = dev->driver;
+       drv = dev->driver;
+       if (drv) {
+               get_driver(drv);
                sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
                sysfs_remove_link(&dev->kobj, "driver");
-               klist_del(&dev->knode_driver);
+               klist_remove(&dev->knode_driver);
 
                device_detach_shutdown(dev);
                if (drv->remove)
                        drv->remove(dev);
                dev->driver = NULL;
+               put_driver(drv);
        }
-       up(&dev->sem);
 }
 
-
-static int __remove_driver(struct device * dev, void * unused)
+void device_release_driver(struct device * dev)
 {
-       device_release_driver(dev);
-       return 0;
+       down(&dev->sem);
+       __device_release_driver(dev);
+       up(&dev->sem);
 }
 
+
 /**
  *     driver_detach - detach driver from all devices it controls.
  *     @drv:   driver.
@@ -222,7 +222,25 @@ static int __remove_driver(struct device
 
 void driver_detach(struct device_driver * drv)
 {
-       driver_for_each_device(drv, NULL, NULL, __remove_driver);
+       struct device * dev;
+
+       for (;;) {
+               spin_lock_irq(&drv->klist_devices.k_lock);
+               if (list_empty(&drv->klist_devices.k_list)) {
+                       spin_unlock_irq(&drv->klist_devices.k_lock);
+                       break;
+               }
+               dev = list_entry(drv->klist_devices.k_list.prev,
+                               struct device, knode_driver.n_node);
+               get_device(dev);
+               spin_unlock_irq(&drv->klist_devices.k_lock);
+
+               down(&dev->sem);
+               if (dev->driver == drv)
+                       __device_release_driver(dev);
+               up(&dev->sem);
+               put_device(dev);
+       }
 }
 
 EXPORT_SYMBOL_GPL(device_bind_driver);



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start!  http://www.idcswdc.com/cgi-bin/survey?id=105hix
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to