To: torvalds@transmeta.com Cc: linux-usb-devel@lists.sourceforge.net Subject: [PATCH 1 of 2] USB urb reference counting Hi, Here's a patch against 2.5.3-pre6 for the USB core and the HCD core that adds proper reference counting logic for the USB urb structures. The other HCD drivers will need to also add this logic, but everything still works properly with this patch (i.e. only the ohci-hcd and ehci-hcd drivers will take ownership of a urb right now.) It also updates the documentation due to these changes. thanks, greg k-h diff -Nru a/drivers/usb/hcd.c b/drivers/usb/hcd.c --- a/drivers/usb/hcd.c Tue Jan 29 13:27:27 2002 +++ b/drivers/usb/hcd.c Tue Jan 29 13:27:27 2002 @@ -996,6 +1002,9 @@ // hcd_monitor_hook(MONITOR_URB_SUBMIT, urb) // It would catch submission paths for all urbs. + /* increment the reference count of the urb, as we now also control it. */ + urb = usb_get_urb(urb); + /* * Atomically queue the urb, first to our records, then to the HCD. * Access to urb->status is controlled by urb->lock ... changes on @@ -1328,5 +1337,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 13:27:27 2002 +++ b/drivers/usb/usb.c Tue Jan 29 13:27:27 2002 @@ -1074,16 +1075,18 @@ * ----------------------------------------------------------------------*/ /** - * usb_alloc_urb - creates a new urb for a USB driver to use - * @iso_packets: number of iso packets for this urb + * usb_alloc_urb - creates a new urb for a USB driver to use + * @iso_packets: number of iso packets for this urb * - * Creates an urb for the USB driver to use and returns a pointer to it. - * If no memory is available, NULL is returned. + * Creates an urb for the USB driver to use, initializes a few internal + * structures, incrementes the usage counter, and returns a pointer to it. * - * If the driver want to use this urb for interrupt, control, or bulk - * endpoints, pass '0' as the number of iso packets. + * If no memory is available, NULL is returned. * - * The driver should call usb_free_urb() when it is finished with the urb. + * If the driver want to use this urb for interrupt, control, or bulk + * endpoints, pass '0' as the number of iso packets. + * + * The driver must call usb_free_urb() when it is finished with the urb. */ struct urb *usb_alloc_urb(int iso_packets) { @@ -1098,25 +1101,49 @@ } memset(urb, 0, sizeof(*urb)); - + atomic_inc(&urb->count); spin_lock_init(&urb->lock); return urb; } /** - * usb_free_urb - frees the memory used by a urb - * @urb: pointer to the urb to free + * usb_free_urb - frees the memory used by a urb when all users of it are finished + * @urb: pointer to the urb to free * - * If an urb is created with a call to usb_create_urb() it should be - * cleaned up with a call to usb_free_urb() when the driver is finished - * with it. + * Must be called when a user of a urb is finished with it. When the last user + * of the urb calls this function, the memory of the urb is freed. + * + * Note: The transfer buffer associated with the urb is not freed, that must be + * done elsewhere. */ void usb_free_urb(struct urb *urb) { if (urb) - kfree(urb); + if (atomic_dec_and_test(&urb->count)) + kfree(urb); +} + +/** + * usb_get_urb - incrementes the reference count of the urb + * @urb: pointer to the urb to modify + * + * This must be called whenever a urb is transfered from a device driver to a + * host controller driver. This allows proper reference counting to happen + * for urbs. + * + * A pointer to the urb with the incremented reference counter is returned. + */ +struct urb * usb_get_urb(struct urb *urb) +{ + if (urb) { + atomic_inc(&urb->count); + return urb; + } else + return NULL; } + + /*-------------------------------------------------------------------*/ /** @@ -1129,7 +1156,7 @@ * This call may be issued in interrupt context. * * The caller must have correctly initialized the URB before submitting - * it. Macros such as FILL_BULK_URB() and FILL_CONTROL_URB() are + * it. Functions such as usb_fill_bulk_urb() and usb_fill_control_urb() are * available to ensure that most fields are correctly initialized, for * the particular kind of transfer, although they will not initialize * any transfer flags. @@ -2794,6 +2822,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 13:27:27 2002 +++ b/include/linux/usb.h Tue Jan 29 13:27:27 2002 @@ -641,11 +643,11 @@ * @iso_frame_desc: Used to provide arrays of ISO transfer buffers and to * collect the transfer status for each buffer. * - * This structure identifies USB transfer requests. URBs may be allocated - * in any way, although usb_alloc_urb() is often convenient. Initialization - * may be done using various usb_fill_*_urb() functions. URBs are submitted - * using usb_submit_urb(), and pending requests may be canceled using - * usb_unlink_urb(). + * This structure identifies USB transfer requests. URBs must be allocated by + * calling usb_alloc_urb() and freed with a call to usb_free_urb(). + * Initialization may be done using various usb_fill_*_urb() functions. URBs + * are submitted using usb_submit_urb(), and pending requests may be canceled + * using usb_unlink_urb(). * * Initialization: * @@ -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);