This teaches usbfs how to play better with the driver model core.

   - The bus rwsem serializes driver model changes, not BKL.
     (But there are still odd-looking BKL usages here.)

   - Use the driver model pointer, not interface.driver (gone).
     It doesn't always need casting to a usb_driver either.

   - Don't use usb_interface_claimed(); instead, check return
     code for usb_driver_claim_interface().

Compiles, but only /proc/bus/usb/devices was much tested.

- Dave


--- 1.22/drivers/usb/core/devices.c     Tue Jul 29 04:28:37 2003
+++ edited/drivers/usb/core/devices.c   Mon Oct  6 13:18:56 2003
@@ -238,7 +238,8 @@
 
        if (start > end)
                return start;
-       lock_kernel(); /* driver might be unloaded */
+       /* lock against driver removal */
+       down_read(&iface->dev.bus->subsys.rwsem);
        start += sprintf(start, format_iface,
                         desc->bInterfaceNumber,
                         desc->bAlternateSetting,
@@ -247,8 +248,8 @@
                         class_decode(desc->bInterfaceClass),
                         desc->bInterfaceSubClass,
                         desc->bInterfaceProtocol,
-                        iface->driver ? iface->driver->name : "(none)");
-       unlock_kernel();
+                        iface->dev.driver ? iface->dev.driver->name : "(none)");
+       up_read(&iface->dev.bus->subsys.rwsem);
        return start;
 }
 
--- 1.53/drivers/usb/core/devio.c       Tue Aug 12 17:19:25 2003
+++ edited/drivers/usb/core/devio.c     Tue Oct  7 12:38:44 2003
@@ -362,14 +362,12 @@
        if (test_bit(intf, &ps->ifclaimed))
                return 0;
        iface = dev->actconfig->interface[intf];
-       err = -EBUSY;
-       lock_kernel();
-       if (!usb_interface_claimed(iface)) {
-               usb_driver_claim_interface(&usbdevfs_driver, iface, ps);
+
+       down_write(&iface->dev.bus->subsys.rwsem);
+       err = usb_driver_claim_interface(&usbdevfs_driver, iface, ps);
+       if (err == 0)
                set_bit(intf, &ps->ifclaimed);
-               err = 0;
-       }
-       unlock_kernel();
+       up_write(&iface->dev.bus->subsys.rwsem);
        return err;
 }
 
@@ -386,7 +384,9 @@
        down(&dev->serialize);
        if (dev && test_and_clear_bit(intf, &ps->ifclaimed)) {
                iface = dev->actconfig->interface[intf];
+               down_write(&iface->dev.bus->subsys.rwsem);
                usb_driver_release_interface(&usbdevfs_driver, iface);
+               up_write(&iface->dev.bus->subsys.rwsem);
                err = 0;
        }
        up(&dev->serialize);
@@ -691,12 +691,18 @@
        interface = usb_ifnum_to_if(ps->dev, gd.interface);
        if (!interface)
                return -EINVAL;
-       if (!interface->driver)
-               return -ENODATA;
-       strcpy(gd.driver, interface->driver->name);
-       if (copy_to_user(arg, &gd, sizeof(gd)))
-               return -EFAULT;
-       return 0;
+       down_read(&interface->dev.bus->subsys.rwsem);
+       if (!interface->dev.driver)
+               ret = -ENODATA;
+       else {
+               strncpy(gd.driver, interface->dev.driver->name,
+                               sizeof gd.driver);
+               ret = 0;
+       }
+       up_read(&interface->dev.bus->subsys.rwsem);
+       if (!ret && copy_to_user(arg, &gd, sizeof(gd)))
+               ret = -EFAULT;
+       return ret;
 }
 
 static int proc_connectinfo(struct dev_state *ps, void __user *arg)
@@ -726,7 +732,7 @@
                        continue;
 
                err ("%s - this function is broken", __FUNCTION__);
-               if (intf->driver && ps->dev) {
+               if (intf->dev.driver && ps->dev) {
                        usb_probe_interface (&intf->dev);
                }
        }
@@ -747,7 +753,7 @@
        interface = usb_ifnum_to_if(ps->dev, setintf.interface);
        if (!interface)
                return -EINVAL;
-       if (interface->driver) {
+       if (interface->dev.driver) {
                if ((ret = checkintf(ps, ret)))
                        return ret;
        }
@@ -1095,50 +1101,37 @@
                retval = -EINVAL;
        else switch (ctrl.ioctl_code) {
 
-       /* disconnect kernel driver from interface, leaving it unbound.  */
-       /* maybe unbound - you get no guarantee it stays unbound */
+       /* disconnect kernel driver from interface */
        case USBDEVFS_DISCONNECT:
-               /* this function is misdesigned - retained for compatibility */
-               lock_kernel();
-               driver = ifp->driver;
-               if (driver) {
-                       dbg ("disconnect '%s' from dev %d interface %d",
-                            driver->name, ps->dev->devnum, ctrl.ifno);
-                       usb_unbind_interface(&ifp->dev);
+               down_write(&ifp->dev.bus->subsys.rwsem);
+               if (ifp->dev.driver) {
+                       dev_dbg(&ifp->dev, "disconnect using usbfs\n");
+                       device_release_driver(&ifp->dev);
                } else
                        retval = -ENODATA;
-               unlock_kernel();
+               up_write(&ifp->dev.bus->subsys.rwsem);
+               /* any driver, even usbfs, may now (re)bind to this interface */
                break;
 
        /* let kernel drivers try to (re)bind to the interface */
        case USBDEVFS_CONNECT:
-               lock_kernel();
+               down_write(&ifp->dev.bus->subsys.rwsem);
                retval = usb_probe_interface (&ifp->dev);
-               unlock_kernel();
+               up_write(&ifp->dev.bus->subsys.rwsem);
                break;
 
        /* talk directly to the interface's driver */
        default:
-               /* BKL used here to protect against changing the binding
-                * of this driver to this device, as well as unloading its
-                * driver module.
-                */
-               lock_kernel ();
-               driver = ifp->driver;
-               if (driver == 0 || driver->ioctl == 0) {
-                       unlock_kernel();
+               down_read(&ifp->dev.bus->subsys.rwsem);
+               driver = to_usb_driver(ifp->dev.driver);
+               if (ifp->dev.driver == 0 || driver->ioctl == 0) {
+                       up_read(&ifp->dev.bus->subsys.rwsem);
                        retval = -ENOSYS;
                } else {
-                       if (!try_module_get (driver->owner)) {
-                               unlock_kernel();
-                               retval = -ENOSYS;
-                               break;
-                       }
-                       unlock_kernel ();
                        retval = driver->ioctl (ifp, ctrl.ioctl_code, buf);
+                       up_read(&ifp->dev.bus->subsys.rwsem);
                        if (retval == -ENOIOCTLCMD)
                                retval = -ENOTTY;
-                       module_put (driver->owner);
                }
        }
 

Reply via email to