[PATCH] USB device reference counting fix for uhci.c and usb core

Earlier in the 2.5 development cycle a patch was applied that changed
the reference counting behaviour for USB devices.

There are a couple of problems with the change:
- It made the USB code more complicated as a whole with the introduction
  of an additional cleanup path for devices. Using the traditional method
  of reference counting, cleanup is handled implictly
- It reduces functionality by requiring a callback for all references to
  the device, but doesn't provide a method of providing callbacks for
  references. It relies on the hardcoded device driver ->disconnect and
  HCD ->deallocate method for callbacks

The traditional method of using reference counting supports as many
reference users as needed, without complicating it with mandatory
callbacks to cleanup references.

The change in 2.5 also only helps catch one subset of programming
problem in device drivers, the case where it decrements too many times.
That is of dubious debugging value.

So, this patch reverts the change and makes the reference counting
behave like it does in the rest of the kernel as well as how the USB
code does in 2.4.

This patch doesn't remove all of the superfluous code. Some drivers,
like usb-ohci, ohci-hcd and ehci-hcd have some code that is no longer
needed. I wanted to spend some more time with those drivers since the
changes weren't as trivial as uhci.c and usb-uhci.c.

I've tested with uhci and usb-ohci with no adverse effects.

drivers/usb/core/hcd.h

@@ -182,9 +182,6 @@ extern int usb_hcd_pci_resume (struct pci_dev *dev);
 /* -------------------------------------------------------------------------- */
 
 /* Enumeration is only for the hub driver, or HCD virtual root hubs */
-extern struct usb_device *usb_alloc_dev(struct usb_device *parent,
-	struct usb_bus *);
-extern void usb_free_dev(struct usb_device *);
 extern int usb_new_device(struct usb_device *dev);
 extern void usb_connect(struct usb_device *dev);
 extern void usb_disconnect(struct usb_device **);

drivers/usb/core/usb.c

@@ -962,6 +962,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, struct usb_bus *bus)
 
 	init_MUTEX(&dev->serialize);
 
+	if (dev->bus->op->allocate)
 		dev->bus->op->allocate(dev);
 
 	return dev;
@@ -978,16 +979,8 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, struct usb_bus *bus)
  */
 void usb_free_dev(struct usb_device *dev)
 {
-	if (in_interrupt ())
-		BUG ();
-	if (!atomic_dec_and_test (&dev->refcnt)) {
-		/* MUST go to zero here, else someone's hanging on to
-		 * a device that's supposed to have been cleaned up!!
-		 */
-		BUG ();
-	}
-
-	dev->bus->op->deallocate (dev);
+	if (dev->bus->op->deallocate)
+		dev->bus->op->deallocate(dev);
 	usb_destroy_configuration (dev);
 	usb_bus_put (dev->bus);
 	kfree (dev);
@@ -1933,8 +1926,9 @@ void usb_disconnect(struct usb_device **pdev)
 		put_device(&dev->dev);
 	}
 
-	/* Free up the device itself */
-	usb_free_dev(dev);
+	/* Decrement the reference count, it'll auto free everything when */
+	/* it hits 0 which could very well be now */
+	usb_dec_dev_use(dev);
 }
 
 /**

drivers/usb/host/uhci.c

@@ -108,19 +108,6 @@ static void wakeup_hc(struct uhci *uhci);
 
 #define MAX_URB_LOOP	2048		/* Maximum number of linked URB's */
 
-/*
- * Only the USB core should call uhci_alloc_dev and uhci_free_dev
- */
-static int uhci_alloc_dev(struct usb_device *dev)
-{
-	return 0;
-}
-
-static int uhci_free_dev(struct usb_device *dev)
-{
-	return 0;
-}
-
 /*
  * Technically, updating td->status here is a race, but it's not really a
  * problem. The worst that can happen is that we set the IOC bit again
@@ -1882,8 +1869,6 @@ static int uhci_get_current_frame_number(struct usb_device *dev)
 }
 
 struct usb_operations uhci_device_operations = {
-	allocate:		uhci_alloc_dev,
-	deallocate:		uhci_free_dev,
 	get_frame_number:	uhci_get_current_frame_number,
 	submit_urb:		uhci_submit_urb,
 	unlink_urb:		uhci_unlink_urb,

include/linux/usb.h

@@ -426,6 +426,11 @@ struct usb_device {
 	struct usb_device *children[USB_MAXCHILDREN];
 };
 
+/* usb_free_dev can be called anywhere from usb_dec_dev_use */
+extern struct usb_device *usb_alloc_dev(struct usb_device *parent,
+	struct usb_bus *);
+extern void usb_free_dev(struct usb_device *);
+
 /* for when layers above USB add new non-USB drivers */
 extern void usb_scan_devices(void);
 
@@ -455,27 +460,11 @@ static inline void usb_inc_dev_use (struct usb_device *dev)
  * @dev: the device no longer being referenced
  *
  * Each live reference to a device should be refcounted.
- *
- * Drivers for USB interfaces should normally release such references in
- * their disconnect() methods, and record them in probe().
- *
- * Note that driver disconnect() methods must guarantee that when they
- * return, all of their outstanding references to the device (and its
- * interfaces) are cleaned up.  That means that all pending URBs from
- * this driver must have completed, and that no more copies of the device
- * handle are saved in driver records (including other kernel threads).
  */
 static inline void usb_dec_dev_use (struct usb_device *dev)
 {
-	if (atomic_dec_and_test (&dev->refcnt)) {
-		/* May only go to zero when usbcore finishes
-		 * usb_disconnect() processing:  khubd or HCDs.
-		 *
-		 * If you hit this BUG() it's likely a problem
-		 * with some driver's disconnect() routine.
-		 */
-		BUG ();
-	}
+	if (atomic_dec_and_test(&dev->refcnt))
+		usb_free_dev(dev);
 }