LWN.net Logo

Advertisement

E-Commerce & credit card processing - the Open Source way!

Advertise here

[patch 2.6.0-test6] usbfs updates (2/3)

From:  David Brownell <david-b@pacbell.net>
To:  Greg KH <greg@kroah.com>, linux-usb-devel@lists.sourceforge.net
Subject:  [linux-usb-devel] [patch 2.6.0-test6] usbfs updates (2/3)
Date:  Tue, 07 Oct 2003 13:10:31 -0700

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);
 		}
 	}
 


Copyright © 2003, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds