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 @@ -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.