net/sonic: Add mutual exclusion for accessing shared state

BugLink: https://bugs.launchpad.net/bugs/1864261
commit 865ad2f2201dc18685ba2686f13217f8b3a9c52c upstream.

The netif_stop_queue() call in sonic_send_packet() races with the
netif_wake_queue() call in sonic_interrupt(). This causes issues
like "NETDEV WATCHDOG: eth0 (macsonic): transmit queue 0 timed out".
Fix this by disabling interrupts when accessing tx_skb[] and next_tx.
Update a comment to clarify the synchronization properties.

Fixes: efcce839360f ("[PATCH] macsonic/jazzsonic network drivers update")
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/drivers/net/ethernet/natsemi/sonic.c b/drivers/net/ethernet/natsemi/sonic.c
index a051dddcbd768c46622a1e987706aa369e922027..a885306a9e3b2b3089c42e530a238280df27fdcd 100644 (file)
--- a/drivers/net/ethernet/natsemi/sonic.c
+++ b/drivers/net/ethernet/natsemi/sonic.c
@@ -68,6 +68,8 @@ static int sonic_open(struct net_device *dev)
                lp->rx_skb[i] = skb;
        }
 
+       spin_lock_init(&lp->lock);
+
        for (i = 0; i < SONIC_NUM_RRS; i++) {
                dma_addr_t laddr = dma_map_single(lp->device, skb_put(lp->rx_skb[i], SONIC_RBSIZE),
                                                  SONIC_RBSIZE, DMA_FROM_DEVICE);
@@ -194,8 +196,6 @@ static void sonic_tx_timeout(struct net_device *dev)
  *   wake the tx queue
  * Concurrently with all of this, the SONIC is potentially writing to
  * the status flags of the TDs.
- * Until some mutual exclusion is added, this code will not work with SMP. However,
- * MIPS Jazz machines and m68k Macs were all uni-processor machines.
  */
 
 static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
@@ -203,7 +203,8 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
        struct sonic_local *lp = netdev_priv(dev);
        dma_addr_t laddr;
        int length;
-       int entry = lp->next_tx;
+       int entry;
+       unsigned long flags;
 
        if (sonic_debug > 2)
                printk("sonic_send_packet: skb=%p, dev=%p\n", skb, dev);
@@ -226,6 +227,10 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
                return NETDEV_TX_OK;
        }
 
+       spin_lock_irqsave(&lp->lock, flags);
+
+       entry = lp->next_tx;
+
        sonic_tda_put(dev, entry, SONIC_TD_STATUS, 0);       /* clear status */
        sonic_tda_put(dev, entry, SONIC_TD_FRAG_COUNT, 1);   /* single fragment */
        sonic_tda_put(dev, entry, SONIC_TD_PKTSIZE, length); /* length of packet */
@@ -235,10 +240,6 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
        sonic_tda_put(dev, entry, SONIC_TD_LINK,
                sonic_tda_get(dev, entry, SONIC_TD_LINK) | SONIC_EOL);
 
-       /*
-        * Must set tx_skb[entry] only after clearing status, and
-        * before clearing EOL and before stopping queue
-        */
        wmb();
        lp->tx_len[entry] = length;
        lp->tx_laddr[entry] = laddr;
@@ -263,6 +264,8 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
 
        SONIC_WRITE(SONIC_CMD, SONIC_CR_TXP);
 
+       spin_unlock_irqrestore(&lp->lock, flags);
+
        return NETDEV_TX_OK;
 }
 
@@ -275,9 +278,21 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
        struct net_device *dev = dev_id;
        struct sonic_local *lp = netdev_priv(dev);
        int status;
+       unsigned long flags;
+
+       /* The lock has two purposes. Firstly, it synchronizes sonic_interrupt()
+        * with sonic_send_packet() so that the two functions can share state.
+        * Secondly, it makes sonic_interrupt() re-entrant, as that is required
+        * by macsonic which must use two IRQs with different priority levels.
+        */
+       spin_lock_irqsave(&lp->lock, flags);
+
+       status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT;
+       if (!status) {
+               spin_unlock_irqrestore(&lp->lock, flags);
 
-       if (!(status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT))
                return IRQ_NONE;
+       }
 
        do {
                if (status & SONIC_INT_PKTRX) {
@@ -292,11 +307,12 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
                        int td_status;
                        int freed_some = 0;
 
-                       /* At this point, cur_tx is the index of a TD that is one of:
-                        *   unallocated/freed                          (status set   & tx_skb[entry] clear)
-                        *   allocated and sent                         (status set   & tx_skb[entry] set  )
-                        *   allocated and not yet sent                 (status clear & tx_skb[entry] set  )
-                        *   still being allocated by sonic_send_packet (status clear & tx_skb[entry] clear)
+                       /* The state of a Transmit Descriptor may be inferred
+                        * from { tx_skb[entry], td_status } as follows.
+                        * { clear, clear } => the TD has never been used
+                        * { set,   clear } => the TD was handed to SONIC
+                        * { set,   set   } => the TD was handed back
+                        * { clear, set   } => the TD is available for re-use
                         */
 
                        if (sonic_debug > 2)
@@ -398,7 +414,12 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
                /* load CAM done */
                if (status & SONIC_INT_LCD)
                        SONIC_WRITE(SONIC_ISR, SONIC_INT_LCD); /* clear the interrupt */
-       } while((status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT));
+
+               status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT;
+       } while (status);
+
+       spin_unlock_irqrestore(&lp->lock, flags);
+
        return IRQ_HANDLED;
 }
 

diff --git a/drivers/net/ethernet/natsemi/sonic.h b/drivers/net/ethernet/natsemi/sonic.h
index 421b1a283fedae971e0a487f75adfd7881ba4796..944f4830c4a1d9d5016e47f0f38af8f5574c71ce 100644 (file)
--- a/drivers/net/ethernet/natsemi/sonic.h
+++ b/drivers/net/ethernet/natsemi/sonic.h
@@ -321,6 +321,7 @@ struct sonic_local {
        unsigned int next_tx;          /* next free TD */
        struct device *device;         /* generic device */
        struct net_device_stats stats;
+       spinlock_t lock;
 };
 
 #define TX_TIMEOUT (3 * HZ)