Hi all,

Here's a patch against 2.5.3-pre6 that is a first cut at adding proper
reference counting logic to the USB urb structures.

There are two new functions:
	usb_get_urb
	usb_put_urb

usb_put_urb() maps directly to usb_free_urb() to prevent any device
drivers from having to change at this time.  usb_get_urb() is used by
any driver that gets control over an urb (like a USB host controller
driver.)

So with this scheme, it is now possible to do the following from a USB
device driver:
	urb = usb_alloc_urb(0);
	usb_fill_urb_bulk(...);
	usb_submit_urb(urb);
	usb_free_urb(urb);
and then when the host controller driver is finished with the urb, it
will be destroyed properly.

This patch only modifies the hcd.c file, so only the ohci-hcd and
ehci-hcd drivers will take ownership over a urb, but the other drivers
still work right now.

I haven't really tested this out much yet, but want to give everyone
else a chance for comments on the implementation.

thanks,

greg k-h



diff -Nru a/drivers/usb/hcd.c b/drivers/usb/hcd.c
--- a/drivers/usb/hcd.c	Tue Jan 29 00:21:56 2002
+++ b/drivers/usb/hcd.c	Tue Jan 29 00:21:57 2002
@@ -916,8 +922,9 @@
 /* may be called in any context with a valid urb->dev usecount */
 /* caller surrenders "ownership" of urb (and chain at urb->next).  */
 
-static int hcd_submit_urb (struct urb *urb)
+static int hcd_submit_urb (struct urb *new_urb)
 {
+	struct urb		*urb;
 	int			status;
 	struct usb_hcd		*hcd;
 	struct hcd_dev		*dev;
@@ -925,27 +932,40 @@
 	int			pipe;
 	int			mem_flags;
 
-	if (!urb || urb->hcpriv || !urb->complete)
+	if (!new_urb)
 		return -EINVAL;
+	urb = usb_get_urb (new_urb);
+	if (urb->hcpriv || !urb->complete) {
+		usb_put_urb (urb);
+		return -EINVAL;
+	}
 
 	urb->status = -EINPROGRESS;
 	urb->actual_length = 0;
 	INIT_LIST_HEAD (&urb->urb_list);
 
-	if (!urb->dev || !urb->dev->bus || urb->dev->devnum <= 0)
+	if (!urb->dev || !urb->dev->bus || urb->dev->devnum <= 0) {
+		usb_put_urb (urb);
 		return -ENODEV;
+	}
 	hcd = urb->dev->bus->hcpriv;
 	dev = urb->dev->hcpriv;
-	if (!hcd || !dev)
+	if (!hcd || !dev) {
+		usb_put_urb (urb);
 		return -ENODEV;
+	}
 
 	/* can't submit new urbs when quiescing, halted, ... */
-	if (hcd->state == USB_STATE_QUIESCING || !HCD_IS_RUNNING (hcd->state))
+	if (hcd->state == USB_STATE_QUIESCING || !HCD_IS_RUNNING (hcd->state)) {
+		usb_put_urb (urb);
 		return -ESHUTDOWN;
+	}
 	pipe = urb->pipe;
 	if (usb_endpoint_halted (urb->dev, usb_pipeendpoint (pipe),
-			usb_pipeout (pipe)))
+			usb_pipeout (pipe))) {
+		usb_put_urb (urb);
 		return -EPIPE;
+	}
 
 	// FIXME paging/swapping requests over USB should not use GFP_KERNEL
 	// and might even need to use GFP_NOIO ... that flag actually needs
@@ -1328,5 +1348,6 @@
 	/* pass ownership to the completion handler */
 	usb_dec_dev_use (dev);
 	urb->complete (urb);
+	usb_put_urb (urb);
 }
 EXPORT_SYMBOL (usb_hcd_giveback_urb);
diff -Nru a/drivers/usb/usb.c b/drivers/usb/usb.c
--- a/drivers/usb/usb.c	Tue Jan 29 00:21:56 2002
+++ b/drivers/usb/usb.c	Tue Jan 29 00:21:56 2002
@@ -1098,7 +1099,7 @@
 	}
 
 	memset(urb, 0, sizeof(*urb));
-
+	atomic_inc(&urb->count);
 	spin_lock_init(&urb->lock);
 
 	return urb;
@@ -1115,8 +1116,20 @@
 void usb_free_urb(struct urb *urb)
 {
 	if (urb)
-		kfree(urb);
+		if (atomic_dec_and_test(&urb->count))
+			kfree(urb);
+}
+
+struct urb * usb_get_urb(struct urb *urb)
+{
+	if (urb) {
+		atomic_inc(&urb->count);
+		return urb;
+	} else
+		return NULL;
 }
+		
+		
 /*-------------------------------------------------------------------*/
 
 /**
@@ -2794,6 +2808,7 @@
 // asynchronous request completion model
 EXPORT_SYMBOL(usb_alloc_urb);
 EXPORT_SYMBOL(usb_free_urb);
+EXPORT_SYMBOL(usb_get_urb);
 EXPORT_SYMBOL(usb_submit_urb);
 EXPORT_SYMBOL(usb_unlink_urb);
 
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h	Tue Jan 29 00:21:56 2002
+++ b/include/linux/usb.h	Tue Jan 29 00:21:56 2002
@@ -721,6 +723,7 @@
 struct urb
 {
 	spinlock_t lock;		/* lock for the URB */
+	atomic_t count;			/* reference count of the URB */
 	void *hcpriv;			/* private data for host controller */
 	struct list_head urb_list;	/* list pointer to all active urbs */
 	struct urb *next; 		/* (in) pointer to next URB */
@@ -854,6 +857,8 @@
 
 extern struct urb *usb_alloc_urb(int iso_packets);
 extern void usb_free_urb(struct urb *urb);
+#define usb_put_urb usb_free_urb
+extern struct urb *usb_get_urb(struct urb *urb);
 extern int usb_submit_urb(struct urb *urb);
 extern int usb_unlink_urb(struct urb *urb);