ChangeSet 1.1290.15.6, 2004/02/05 16:26:27-08:00, david-b@pacbell.net

[PATCH] USB Gadget: ethernet gadget locking tweaks

[USB] ethernet gadget, locking tweaks

This problem showed pretty quickly on an SMP system.  Basically,
access to the freelist (tx more than rx) needs a spinlock.

Stop repeating some alloc_etherdev() work.  Disable DEBUG messages.


 drivers/usb/gadget/ether.c |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)


diff -Nru a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
--- a/drivers/usb/gadget/ether.c	Wed Mar 17 15:48:56 2004
+++ b/drivers/usb/gadget/ether.c	Wed Mar 17 15:48:56 2004
@@ -19,7 +19,7 @@
  */
 
 
-#define DEBUG 1
+// #define DEBUG 1
 // #define VERBOSE
 
 #include <linux/config.h>
@@ -897,8 +897,11 @@
 #ifndef	DEV_CONFIG_CDC
 	if (result == 0) {
 		netif_carrier_on (dev->net);
-		if (netif_running (dev->net))
+		if (netif_running (dev->net)) {
+			spin_unlock (&dev->lock);
 			eth_start (dev, GFP_ATOMIC);
+			spin_lock (&dev->lock);
+		}
 	} else {
 		(void) usb_ep_disable (dev->in_ep);
 		dev->in_ep = 0;
@@ -1258,8 +1261,11 @@
 #ifdef	EP_STATUS_NUM
 				issue_start_status (dev);
 #endif
-				if (netif_running (dev->net))
+				if (netif_running (dev->net)) {
+					spin_unlock (&dev->lock);
 					eth_start (dev, GFP_ATOMIC);
+					spin_lock (&dev->lock);
+				}
 			} else {
 				netif_stop_queue (dev->net);
 				netif_carrier_off (dev->net);
@@ -1426,16 +1432,14 @@
 rx_submit (struct eth_dev *dev, struct usb_request *req, int gfp_flags)
 {
 	struct sk_buff		*skb;
-	int			retval = 0;
+	int			retval = -ENOMEM;
 	size_t			size;
 
 	size = (sizeof (struct ethhdr) + dev->net->mtu + RX_EXTRA);
 
 	if ((skb = alloc_skb (size, gfp_flags)) == 0) {
 		DEBUG (dev, "no rx skb\n");
-		defer_kevent (dev, WORK_RX_MEMORY);
-		list_add (&req->list, &dev->rx_reqs);
-		return -ENOMEM;
+		goto enomem;
 	}
 
 	req->buf = skb->data;
@@ -1445,11 +1449,14 @@
 
 	retval = usb_ep_queue (dev->out_ep, req, gfp_flags);
 	if (retval == -ENOMEM)
+enomem:
 		defer_kevent (dev, WORK_RX_MEMORY);
 	if (retval) {
 		DEBUG (dev, "rx submit --> %d\n", retval);
 		dev_kfree_skb_any (skb);
+		spin_lock (&dev->lock);
 		list_add (&req->list, &dev->rx_reqs);
+		spin_unlock (&dev->lock);
 	}
 	return retval;
 }
@@ -1514,6 +1521,7 @@
 		dev_kfree_skb_any (skb);
 	if (!netif_running (dev->net)) {
 clean:
+		/* nobody reading rx_reqs, so no dev->lock */
 		list_add (&req->list, &dev->rx_reqs);
 		req = 0;
 	}
@@ -1580,19 +1588,26 @@
 static void rx_fill (struct eth_dev *dev, int gfp_flags)
 {
 	struct usb_request	*req;
+	unsigned long		flags;
 
 	clear_bit (WORK_RX_MEMORY, &dev->todo);
 
 	/* fill unused rxq slots with some skb */
+	spin_lock_irqsave (&dev->lock, flags);
 	while (!list_empty (&dev->rx_reqs)) {
 		req = container_of (dev->rx_reqs.next,
 				struct usb_request, list);
 		list_del_init (&req->list);
+		spin_unlock_irqrestore (&dev->lock, flags);
+
 		if (rx_submit (dev, req, gfp_flags) < 0) {
 			defer_kevent (dev, WORK_RX_MEMORY);
 			return;
 		}
+
+		spin_lock_irqsave (&dev->lock, flags);
 	}
+	spin_unlock_irqrestore (&dev->lock, flags);
 }
 
 static void eth_work (void *_dev)
@@ -1628,7 +1643,9 @@
 	}
 	dev->stats.tx_packets++;
 
+	spin_lock (&dev->lock);
 	list_add (&req->list, &dev->tx_reqs);
+	spin_unlock (&dev->lock);
 	dev_kfree_skb_any (skb);
 
 	atomic_dec (&dev->tx_qlen);
@@ -1642,11 +1659,14 @@
 	int			length = skb->len;
 	int			retval;
 	struct usb_request	*req = 0;
+	unsigned long		flags;
 
+	spin_lock_irqsave (&dev->lock, flags);
 	req = container_of (dev->tx_reqs.next, struct usb_request, list);
 	list_del (&req->list);
 	if (list_empty (&dev->tx_reqs))
 		netif_stop_queue (net);
+	spin_unlock_irqrestore (&dev->lock, flags);
 
 	/* no buffer copies needed, unless the network stack did it
 	 * or the hardware can't use skb buffers.
@@ -1687,9 +1707,11 @@
 	if (retval) {
 		dev->stats.tx_dropped++;
 		dev_kfree_skb_any (skb);
+		spin_lock_irqsave (&dev->lock, flags);
 		if (list_empty (&dev->tx_reqs))
 			netif_start_queue (net);
 		list_add (&req->list, &dev->tx_reqs);
+		spin_unlock_irqrestore (&dev->lock, flags);
 	}
 	return 0;
 }
@@ -1810,9 +1832,7 @@
 	/* network device setup */
 	dev->net = net;
 	SET_MODULE_OWNER (net);
-	net->priv = dev;
 	strcpy (net->name, "usb%d");
-	ether_setup (net);
 
 	/* one random address for the gadget device ... both of these could
 	 * reasonably come from an id prom or a module parameter.
