[LWN Logo]
[LWN.net]
From:	 Johannes Erdfelt <johannes@erdfelt.com>
To:	 greg@kroah.com
Subject: [linux-usb-devel] [patch] 2.5.15 USB device reference counting
Date:	 Sun, 12 May 2002 15:28:49 -0400
Cc:	 linux-usb-devel@lists.sourceforge.net

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.

JE

diff -ur linux-2.5.15.orig/drivers/usb/core/hcd.h linux-2.5.15/drivers/usb/core/hcd.h
--- linux-2.5.15.orig/drivers/usb/core/hcd.h	Thu May  9 15:23:31 2002
+++ linux-2.5.15/drivers/usb/core/hcd.h	Sun May 12 09:15:30 2002
@@ -182,9 +182,6 @@
 /* -------------------------------------------------------------------------- */
 
 /* 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 **);
diff -ur linux-2.5.15.orig/drivers/usb/core/usb.c linux-2.5.15/drivers/usb/core/usb.c
--- linux-2.5.15.orig/drivers/usb/core/usb.c	Thu May  9 15:24:45 2002
+++ linux-2.5.15/drivers/usb/core/usb.c	Sun May 12 09:07:19 2002
@@ -962,7 +962,8 @@
 
 	init_MUTEX(&dev->serialize);
 
-	dev->bus->op->allocate(dev);
+	if (dev->bus->op->allocate)
+		dev->bus->op->allocate(dev);
 
 	return dev;
 }
@@ -978,16 +979,8 @@
  */
 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);
@@ -1928,8 +1921,9 @@
 		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);
 }
 
 /**
diff -ur linux-2.5.15.orig/drivers/usb/host/uhci.c linux-2.5.15/drivers/usb/host/uhci.c
--- linux-2.5.15.orig/drivers/usb/host/uhci.c	Sun May 12 09:47:56 2002
+++ linux-2.5.15/drivers/usb/host/uhci.c	Sun May 12 09:07:35 2002
@@ -109,19 +109,6 @@
 #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
  * generating a spurios interrupt. We could fix this by creating another
@@ -1882,8 +1869,6 @@
 }
 
 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,
diff -ur linux-2.5.15.orig/drivers/usb/host/usb-uhci.c linux-2.5.15/drivers/usb/host/usb-uhci.c
--- linux-2.5.15.orig/drivers/usb/host/usb-uhci.c	Thu May  9 15:24:15 2002
+++ linux-2.5.15/drivers/usb/host/usb-uhci.c	Sun May 12 09:08:34 2002
@@ -2221,14 +2221,6 @@
 	return -EPROTO;
 }
 
-/*
- * Only the USB core should call uhci_alloc_dev and uhci_free_dev
- */
-_static int uhci_alloc_dev (struct usb_device *usb_dev)
-{
-	return 0;
-}
-
 _static void uhci_unlink_urbs(uhci_t *s, struct usb_device *usb_dev, int remove_all)
 {
 	unsigned long flags;
@@ -2258,20 +2250,6 @@
 	spin_unlock_irqrestore (&s->urb_list_lock, flags);
 }
 
-_static int uhci_free_dev (struct usb_device *usb_dev)
-{
-	uhci_t *s;
-	
-
-	if(!usb_dev || !usb_dev->bus || !usb_dev->bus->hcpriv)
-		return -EINVAL;
-	
-	s=(uhci_t*) usb_dev->bus->hcpriv;	
-	uhci_unlink_urbs(s, usb_dev, 0);
-
-	return 0;
-}
-
 /*
  * uhci_get_current_frame_number()
  *
@@ -2284,8 +2262,6 @@
 
 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,
diff -ur linux-2.5.15.orig/include/linux/usb.h linux-2.5.15/include/linux/usb.h
--- linux-2.5.15.orig/include/linux/usb.h	Thu May  9 15:24:11 2002
+++ linux-2.5.15/include/linux/usb.h	Sun May 12 09:17:52 2002
@@ -440,6 +440,11 @@
 	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);
 
@@ -469,27 +474,11 @@
  * @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);
 }
 
 

_______________________________________________________________

Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth@sourceforge.net
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel