![[LWN Logo]](/images/lcorner.png) |
|
![[LWN.net]](/images/Included.png) |
From: Greg KH <greg@kroah.com>
To: Johannes Erdfelt <johannes@erdfelt.com>
Subject: [linux-usb-devel] Re: [patch] 2.5.15 USB device reference counting
Date: Mon, 13 May 2002 00:47:35 -0700
Cc: linux-usb-devel@lists.sourceforge.net
Here's a follow up patch to be applied on top of your previous patch.
Much of the confusion for me (and probably others) was the fact that we
had to modify reference counts of devices and then free them up when we
were finished with them. Why make the driver authors have to figure out
the count logic (i.e. when to call usb_free_dev()) when we were already
using reference counts?
This patch replaces the awkwardly named usb_inc_dev_use() and
usb_dec_dev_use() with usb_get_dev() and usb_put_dev() to match the
naming convention of the rest of the kernel's reference counted
structures. It also does away with the special case of usb_free_dev(),
and has usb_put_dev() be the same thing (through a #define, just like
usb_free_urb() works.)
Now when the last person calls usb_put_dev() or usb_free_dev() the
structure is cleaned up. This allows the different host controller
drivers to implement their logic differently if they want to (as they
do), and everyone can be happy and stop arguing about the "proper" way
to write their host controller drivers :)
One question remains, should device drivers call usb_get_dev() like some
currently do (storage, some network, and the usbvideo core)? Does this
protect something from happening that I don't see? And if all drivers
should call it, shouldn't a successful return from probe() back to the
usb core do the call for the driver?
thanks,
greg k-h
diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/core/hcd.c Mon May 13 01:46:04 2002
@@ -1320,7 +1320,7 @@
list_del_init (&urb->urb_list);
dev = urb->dev;
urb->dev = NULL;
- usb_dec_dev_use (dev);
+ usb_put_dev (dev);
spin_unlock_irqrestore (&hcd_data_lock, flags);
}
@@ -1516,7 +1516,7 @@
spin_lock_irqsave (&hcd_data_lock, flags);
if (HCD_IS_RUNNING (hcd->state) && hcd->state != USB_STATE_QUIESCING) {
- usb_inc_dev_use (urb->dev);
+ usb_get_dev (urb->dev);
list_add (&urb->urb_list, &dev->urb_list);
status = 0;
} else {
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/core/usb.c Mon May 13 01:46:04 2002
@@ -969,21 +969,47 @@
}
/**
- * usb_free_dev - free a usb device structure (usbcore-internal)
+ * usb_get_dev - increments the reference count of the device
+ * @dev: the device being referenced
+ *
+ * Each live reference to a device should be refcounted.
+ *
+ * Drivers for USB interfaces should normally record such references in
+ * their probe() methods, when they bind to an interface, and release
+ * them by calling usb_put_dev(), in their disconnect() methods.
+ *
+ * A pointer to the device with the incremented reference counter is returned.
+ */
+struct usb_device *usb_get_dev (struct usb_device *dev)
+{
+ if (dev) {
+ atomic_inc (&dev->refcnt);
+ return dev;
+ }
+ return NULL;
+}
+
+/**
+ * usb_free_dev - free a usb device structure when all users of it are finished.
* @dev: device that's been disconnected
* Context: !in_interrupt ()
*
+ * Must be called when a user of a device is finished with it. When the last
+ * user of the device calls this function, the memory of the device is freed.
+ *
* Used by hub and virtual root hub drivers. The device is completely
* gone, everything is cleaned up, so it's time to get rid of these last
* records of this device.
*/
void usb_free_dev(struct usb_device *dev)
{
- if (dev->bus->op->deallocate)
- dev->bus->op->deallocate(dev);
- usb_destroy_configuration (dev);
- usb_bus_put (dev->bus);
- kfree (dev);
+ if (atomic_dec_and_test(&dev->refcnt)) {
+ if (dev->bus->op->deallocate)
+ dev->bus->op->deallocate(dev);
+ usb_destroy_configuration (dev);
+ usb_bus_put (dev->bus);
+ kfree (dev);
+ }
}
/**
@@ -1039,7 +1065,7 @@
}
/**
- * usb_get_urb - incrementes the reference count of the urb
+ * usb_get_urb - increments 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
@@ -1928,7 +1954,7 @@
/* Decrement the reference count, it'll auto free everything when */
/* it hits 0 which could very well be now */
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
}
/**
@@ -2760,6 +2786,7 @@
EXPORT_SYMBOL(usb_alloc_dev);
EXPORT_SYMBOL(usb_free_dev);
+EXPORT_SYMBOL(usb_get_dev);
EXPORT_SYMBOL(usb_hub_tt_clear_buffer);
EXPORT_SYMBOL(usb_find_interface_driver_for_ifnum);
diff -Nru a/drivers/usb/host/uhci.c b/drivers/usb/host/uhci.c
--- a/drivers/usb/host/uhci.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/host/uhci.c Mon May 13 01:46:04 2002
@@ -165,7 +165,7 @@
INIT_LIST_HEAD(&td->list);
INIT_LIST_HEAD(&td->fl_list);
- usb_inc_dev_use(dev);
+ usb_get_dev(dev);
return td;
}
@@ -317,7 +317,7 @@
dbg("td is still in URB list!");
if (td->dev)
- usb_dec_dev_use(td->dev);
+ usb_put_dev(td->dev);
pci_pool_free(uhci->td_pool, td, td->dma_handle);
}
@@ -342,7 +342,7 @@
INIT_LIST_HEAD(&qh->list);
INIT_LIST_HEAD(&qh->remove_list);
- usb_inc_dev_use(dev);
+ usb_get_dev(dev);
return qh;
}
@@ -355,7 +355,7 @@
dbg("qh still in remove_list!");
if (qh->dev)
- usb_dec_dev_use(qh->dev);
+ usb_put_dev(qh->dev);
pci_pool_free(uhci->qh_pool, qh, qh->dma_handle);
}
@@ -1492,7 +1492,7 @@
uhci = (struct uhci *)urb->dev->bus->hcpriv;
INIT_LIST_HEAD(&urb->urb_list);
- usb_inc_dev_use(urb->dev);
+ usb_get_dev(urb->dev);
spin_lock_irqsave(&uhci->urb_list_lock, flags);
spin_lock(&urb->lock);
@@ -1503,7 +1503,7 @@
/* Since we can have problems on the out path */
spin_unlock(&urb->lock);
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
- usb_dec_dev_use(urb->dev);
+ usb_put_dev(urb->dev);
usb_put_urb(urb);
return ret;
@@ -2376,7 +2376,7 @@
} else {
/* We decrement the usage count after we're done */
/* with everything */
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
usb_put_urb(urb);
}
}
diff -Nru a/drivers/usb/host/usb-ohci.c b/drivers/usb/host/usb-ohci.c
--- a/drivers/usb/host/usb-ohci.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/host/usb-ohci.c Mon May 13 01:46:04 2002
@@ -210,7 +210,7 @@
}
urb_free_priv ((struct ohci *)urb->dev->bus->hcpriv, urb_priv);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
urb->dev = NULL;
usb_put_urb (urb);
}
@@ -563,7 +563,7 @@
/* increment the reference count of the urb, as we now also control it */
urb = usb_get_urb (urb);
- usb_inc_dev_use (urb->dev);
+ usb_get_dev (urb->dev);
ohci = (ohci_t *) urb->dev->bus->hcpriv;
#ifdef DEBUG
@@ -577,14 +577,14 @@
/* when controller's hung, permit only roothub cleanup attempts
* such as powering down ports */
if (ohci->disabled) {
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -ESHUTDOWN;
}
/* every endpoint has a ed, locate and fill it */
if (!(ed = ep_add_ed (urb->dev, pipe, urb->interval, 1, mem_flags))) {
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -ENOMEM;
}
@@ -606,7 +606,7 @@
case PIPE_ISOCHRONOUS: /* number of packets from URB */
size = urb->number_of_packets;
if (size <= 0) {
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -EINVAL;
}
@@ -627,7 +627,7 @@
/* allocate the private part of the URB */
urb_priv = kmalloc (sizeof (urb_priv_t) + size * sizeof (td_t *), mem_flags);
if (!urb_priv) {
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -ENOMEM;
}
@@ -645,7 +645,7 @@
urb_priv->length = i;
urb_free_priv (ohci, urb_priv);
spin_unlock_irqrestore (&usb_ed_lock, flags);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -ENOMEM;
}
@@ -654,7 +654,7 @@
if (ed->state == ED_NEW || (ed->state & ED_DEL)) {
urb_free_priv (ohci, urb_priv);
spin_unlock_irqrestore (&usb_ed_lock, flags);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return -EINVAL;
}
@@ -677,7 +677,7 @@
if (bustime < 0) {
urb_free_priv (ohci, urb_priv);
spin_unlock_irqrestore (&usb_ed_lock, flags);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
return bustime;
}
@@ -802,7 +802,7 @@
return -ETIMEDOUT;
}
} else {
- /* usb_dec_dev_use done in dl_del_list() */
+ /* usb_put_dev done in dl_del_list() */
urb->status = -EINPROGRESS;
spin_unlock_irqrestore (&usb_ed_lock, flags);
return -EINPROGRESS;
@@ -2109,7 +2109,7 @@
#endif
urb->hcpriv = NULL;
- usb_dec_dev_use (usb_dev);
+ usb_put_dev (usb_dev);
urb->dev = NULL;
if (urb->complete)
urb->complete (urb);
@@ -2129,7 +2129,7 @@
ohci->rh.urb = NULL;
urb->hcpriv = NULL;
- usb_dec_dev_use(urb->dev);
+ usb_put_dev (urb->dev);
urb->dev = NULL;
if (urb->transfer_flags & USB_ASYNC_UNLINK) {
urb->status = -ECONNRESET;
diff -Nru a/drivers/usb/host/usb-uhci.c b/drivers/usb/host/usb-uhci.c
--- a/drivers/usb/host/usb-uhci.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/host/usb-uhci.c Mon May 13 01:46:04 2002
@@ -1217,7 +1217,7 @@
urb->dev = NULL;
urb->complete ((struct urb *) urb);
}
- usb_dec_dev_use (usb_dev);
+ usb_put_dev (usb_dev);
usb_put_urb (urb);
}
else
@@ -1301,7 +1301,7 @@
uhci_urb_dma_unmap(s, urb, urb_priv);
- usb_dec_dev_use (dev);
+ usb_put_dev (dev);
#ifdef DEBUG_SLAB
kmem_cache_free (urb_priv_kmem, urb_priv);
#else
@@ -1655,7 +1655,7 @@
/* increment the reference count of the urb, as we now also control it */
urb = usb_get_urb (urb);
- usb_inc_dev_use (urb->dev);
+ usb_get_dev (urb->dev);
spin_lock_irqsave (&s->urb_list_lock, flags);
@@ -1669,7 +1669,7 @@
((type == PIPE_BULK) &&
(!(urb->transfer_flags & USB_QUEUE_BULK) || !(queued_urb->transfer_flags & USB_QUEUE_BULK)))) {
spin_unlock_irqrestore (&s->urb_list_lock, flags);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
usb_put_urb (urb);
err("ENXIO %08x, flags %x, urb %p, burb %p",urb->pipe,urb->transfer_flags,urb,queued_urb);
return -ENXIO; // urb already queued
@@ -1682,7 +1682,7 @@
urb_priv = kmalloc (sizeof (urb_priv_t), mem_flags);
#endif
if (!urb_priv) {
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
spin_unlock_irqrestore (&s->urb_list_lock, flags);
usb_put_urb (urb);
return -ENOMEM;
@@ -1767,7 +1767,7 @@
if (ret != 0) {
uhci_urb_dma_unmap(s, urb, urb_priv);
- usb_dec_dev_use (urb->dev);
+ usb_put_dev (urb->dev);
#ifdef DEBUG_SLAB
kmem_cache_free(urb_priv_kmem, urb_priv);
#else
@@ -2737,7 +2737,7 @@
spin_lock(&s->urb_list_lock);
}
- usb_dec_dev_use (usb_dev);
+ usb_put_dev (usb_dev);
usb_put_urb (urb);
}
}
diff -Nru a/drivers/usb/media/usbvideo.c b/drivers/usb/media/usbvideo.c
--- a/drivers/usb/media/usbvideo.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/media/usbvideo.c Mon May 13 01:46:04 2002
@@ -970,7 +970,7 @@
for (i=0; i < USBVIDEO_NUMSBUF; i++)
usb_free_urb(uvd->sbuf[i].urb);
- usb_dec_dev_use(uvd->dev);
+ usb_put_dev(uvd->dev);
uvd->dev = NULL; /* USB device is no more */
video_unregister_device(&uvd->vdev);
@@ -1176,7 +1176,7 @@
}
#endif
- usb_inc_dev_use(uvd->dev);
+ usb_get_dev(uvd->dev);
return 0;
}
diff -Nru a/drivers/usb/net/cdc-ether.c b/drivers/usb/net/cdc-ether.c
--- a/drivers/usb/net/cdc-ether.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/net/cdc-ether.c Mon May 13 01:46:04 2002
@@ -1256,7 +1256,7 @@
ether_dev );
// Does this REALLY do anything???
- usb_inc_dev_use( usb );
+ usb_get_dev( usb );
// TODO - last minute HACK
ether_dev->comm_ep_in = 5;
@@ -1298,7 +1298,7 @@
ether_dev->net = NULL;
// I ask again, does this do anything???
- usb_dec_dev_use( usb );
+ usb_put_dev( usb );
// We are done with this interface
usb_driver_release_interface( &CDCEther_driver,
diff -Nru a/drivers/usb/net/pegasus.c b/drivers/usb/net/pegasus.c
--- a/drivers/usb/net/pegasus.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/net/pegasus.c Mon May 13 01:46:04 2002
@@ -981,7 +981,7 @@
return NULL;
}
- usb_inc_dev_use(dev);
+ usb_get_dev(dev);
memset(pegasus, 0, sizeof(struct pegasus));
pegasus->dev_index = dev_index;
init_waitqueue_head(&pegasus->ctrl_wait);
@@ -1086,7 +1086,7 @@
pegasus->flags |= PEGASUS_UNPLUG;
unregister_netdev(pegasus->net);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
usb_unlink_urb(pegasus->intr_urb);
usb_unlink_urb(pegasus->tx_urb);
usb_unlink_urb(pegasus->rx_urb);
diff -Nru a/drivers/usb/net/usbnet.c b/drivers/usb/net/usbnet.c
--- a/drivers/usb/net/usbnet.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/net/usbnet.c Mon May 13 01:46:04 2002
@@ -1951,7 +1951,7 @@
flush_scheduled_tasks ();
kfree (dev);
- usb_dec_dev_use (udev);
+ usb_put_dev (udev);
}
@@ -1997,7 +1997,7 @@
memset (dev, 0, sizeof *dev);
init_MUTEX_LOCKED (&dev->mutex);
- usb_inc_dev_use (udev);
+ usb_get_dev (udev);
dev->udev = udev;
dev->driver_info = info;
dev->msg_level = msg_level;
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c Mon May 13 01:46:04 2002
+++ b/drivers/usb/storage/usb.c Mon May 13 01:46:04 2002
@@ -670,7 +670,7 @@
}
/* At this point, we're committed to using the device */
- usb_inc_dev_use(dev);
+ usb_get_dev(dev);
/* clear the GUID and fetch the strings */
GUID_CLEAR(guid);
@@ -729,14 +729,14 @@
/* allocate an IRQ callback if one is needed */
if ((ss->protocol == US_PR_CBI) && usb_stor_allocate_irq(ss)) {
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
/* allocate the URB we're going to use */
ss->current_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!ss->current_urb) {
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
@@ -754,7 +754,7 @@
if ((ss = (struct us_data *)kmalloc(sizeof(struct us_data),
GFP_KERNEL)) == NULL) {
printk(KERN_WARNING USB_STORAGE "Out of memory\n");
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
memset(ss, 0, sizeof(struct us_data));
@@ -763,7 +763,7 @@
ss->current_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!ss->current_urb) {
kfree(ss);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
@@ -904,7 +904,7 @@
ss->transport_name = "Unknown";
kfree(ss->current_urb);
kfree(ss);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
break;
}
@@ -959,7 +959,7 @@
ss->protocol_name = "Unknown";
kfree(ss->current_urb);
kfree(ss);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
break;
}
@@ -969,7 +969,7 @@
if ((ss->protocol == US_PR_CBI) && usb_stor_allocate_irq(ss)) {
kfree(ss->current_urb);
kfree(ss);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
@@ -1005,7 +1005,7 @@
"Unable to start control thread\n");
kfree(ss->current_urb);
kfree(ss);
- usb_dec_dev_use(dev);
+ usb_put_dev(dev);
return NULL;
}
@@ -1072,7 +1072,7 @@
ss->current_urb = NULL;
/* mark the device as gone */
- usb_dec_dev_use(ss->pusb_dev);
+ usb_put_dev(ss->pusb_dev);
ss->pusb_dev = NULL;
atomic_set(&ss->sm_state, US_STATE_DETACHED);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Mon May 13 01:46:04 2002
+++ b/include/linux/usb.h Mon May 13 01:46:04 2002
@@ -426,10 +426,10 @@
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 struct usb_device *usb_alloc_dev(struct usb_device *parent, struct usb_bus *);
+extern struct usb_device *usb_get_dev(struct usb_device *dev);
extern void usb_free_dev(struct usb_device *);
+#define usb_put_dev usb_free_dev
/* for when layers above USB add new non-USB drivers */
extern void usb_scan_devices(void);
@@ -439,34 +439,6 @@
/* for drivers using iso endpoints */
extern int usb_get_current_frame_number (struct usb_device *usb_dev);
-
-/**
- * usb_inc_dev_use - record another reference to a device
- * @dev: the device being referenced
- *
- * Each live reference to a device should be refcounted.
- *
- * Drivers for USB interfaces should normally record such references in
- * their probe() methods, when they bind to an interface, and release
- * them usb_dec_dev_use(), in their disconnect() methods.
- */
-static inline void usb_inc_dev_use (struct usb_device *dev)
-{
- atomic_inc (&dev->refcnt);
-}
-
-/**
- * usb_dec_dev_use - drop a reference to a device
- * @dev: the device no longer being referenced
- *
- * Each live reference to a device should be refcounted.
- */
-static inline void usb_dec_dev_use (struct usb_device *dev)
-{
- if (atomic_dec_and_test(&dev->refcnt))
- usb_free_dev(dev);
-}
-
/* used these for multi-interface device registration */
extern int usb_find_interface_driver_for_ifnum(struct usb_device *dev, unsigned int ifnum);
_______________________________________________________________
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