Fix SMP locking, and add special cases for SMP in order to preserve
UP's slightly better efficiency and reliability.
Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/drivers/net/smc91x.c | 86 +++++++++++++++++++++++++++++++++----------
1 files changed, 66 insertions(+), 20 deletions(-)
diff -puN drivers/net/smc91x.c~smc91x-straighten-smp-locking drivers/net/smc91x.c
--- 25/drivers/net/smc91x.c~smc91x-straighten-smp-locking Thu Sep 23 15:00:22 2004
+++ 25-akpm/drivers/net/smc91x.c Thu Sep 23 15:00:22 2004
@@ -260,24 +260,22 @@ static void PRINT_PKT(u_char *buf, int l
/* this enables an interrupt in the interrupt mask register */
#define SMC_ENABLE_INT(x) do { \
- unsigned long flags; \
unsigned char mask; \
- spin_lock_irqsave(&lp->lock, flags); \
+ spin_lock_irq(&lp->lock); \
mask = SMC_GET_INT_MASK(); \
mask |= (x); \
SMC_SET_INT_MASK(mask); \
- spin_unlock_irqrestore(&lp->lock, flags); \
+ spin_unlock_irq(&lp->lock); \
} while (0)
/* this disables an interrupt from the interrupt mask register */
#define SMC_DISABLE_INT(x) do { \
- unsigned long flags; \
unsigned char mask; \
- spin_lock_irqsave(&lp->lock, flags); \
+ spin_lock_irq(&lp->lock); \
mask = SMC_GET_INT_MASK(); \
mask &= ~(x); \
SMC_SET_INT_MASK(mask); \
- spin_unlock_irqrestore(&lp->lock, flags); \
+ spin_unlock_irq(&lp->lock); \
} while (0)
/*
@@ -312,8 +310,10 @@ static void smc_reset(struct net_device
DBG(2, "%s: %s\n", dev->name, __FUNCTION__);
/* Disable all interrupts */
+ spin_lock(&lp->lock);
SMC_SELECT_BANK(2);
SMC_SET_INT_MASK(0);
+ spin_unlock(&lp->lock);
/*
* This resets the registers mostly to defaults, but doesn't
@@ -427,13 +427,18 @@ static void smc_enable(struct net_device
/*
* this puts the device in an inactive state
*/
-static void smc_shutdown(unsigned long ioaddr)
+static void smc_shutdown(struct net_device *dev)
{
+ unsigned long ioaddr = dev->base_addr;
+ struct smc_local *lp = netdev_priv(dev);
+
DBG(2, "%s: %s\n", CARDNAME, __FUNCTION__);
/* no more interrupts for me */
+ spin_lock(&lp->lock);
SMC_SELECT_BANK(2);
SMC_SET_INT_MASK(0);
+ spin_unlock(&lp->lock);
/* and tell the card to stay away from that nasty outside world */
SMC_SELECT_BANK(0);
@@ -535,6 +540,37 @@ done:
SMC_SET_MMU_CMD(MC_RELEASE);
}
+#ifdef CONFIG_SMP
+/*
+ * On SMP we have the following problem:
+ *
+ * A = smc_hardware_send_pkt()
+ * B = smc_hard_start_xmit()
+ * C = smc_interrupt()
+ *
+ * A and B can never be executed simultaneously. However, at least on UP,
+ * it is possible (and even desirable) for C to interrupt execution of
+ * A or B in order to have better RX reliability and avoid overruns.
+ * C, just like A and B, must have exclusive access to the chip and
+ * each of them must lock against any other concurrent access.
+ * Unfortunately this is not possible to have C suspend execution of A or
+ * B taking place on another CPU. On UP this is no an issue since A and B
+ * are run from softirq context and C from hard IRQ context, and there is
+ * no other CPU where concurrent access can happen.
+ * If ever there is a way to force at least B and C to always be executed
+ * on the same CPU then we could use read/write locks to protect against
+ * any other concurrent access and C would always interrupt B. But life
+ * isn't that easy in a SMP world...
+ */
+#define smc_special_trylock(lock) spin_trylock_irq(lock)
+#define smc_special_lock(lock) spin_lock_irq(lock)
+#define smc_special_unlock(lock) spin_unlock_irq(lock)
+#else
+#define smc_special_trylock(lock) (1)
+#define smc_special_lock(lock) do { } while (0)
+#define smc_special_unlock(lock) do { } while (0)
+#endif
+
/*
* This is called to actually send a packet to the chip.
* Returns non-zero when successful.
@@ -550,14 +586,18 @@ static void smc_hardware_send_pkt(unsign
DBG(3, "%s: %s\n", dev->name, __FUNCTION__);
+ if (!smc_special_trylock(&lp->lock)) {
+ tasklet_schedule(&lp->tx_task);
+ return;
+ }
+
lp->saved_skb = NULL;
packet_no = SMC_GET_AR();
if (unlikely(packet_no & AR_FAILED)) {
printk("%s: Memory allocation failed.\n", dev->name);
lp->stats.tx_errors++;
lp->stats.tx_fifo_errors++;
- dev_kfree_skb(skb);
- return;
+ goto done;
}
/* point to the beginning of the packet */
@@ -597,16 +637,18 @@ static void smc_hardware_send_pkt(unsign
/* queue the packet for TX */
SMC_SET_MMU_CMD(MC_ENQUEUE);
SMC_ACK_INT(IM_TX_EMPTY_INT);
-
- if (!THROTTLE_TX_PKTS)
- netif_wake_queue(dev);
-
SMC_ENABLE_INT(IM_TX_INT | IM_TX_EMPTY_INT);
- dev_kfree_skb(skb);
dev->trans_start = jiffies;
lp->stats.tx_packets++;
lp->stats.tx_bytes += len;
+
+done: smc_special_unlock(&lp->lock);
+
+ if (!THROTTLE_TX_PKTS)
+ netif_wake_queue(dev);
+
+ dev_kfree_skb(skb);
}
/*
@@ -647,6 +689,8 @@ static int smc_hard_start_xmit(struct sk
return 0;
}
+ smc_special_lock(&lp->lock);
+
/* now, try to allocate the memory */
SMC_SET_MMU_CMD(MC_ALLOC | numPages);
@@ -663,6 +707,8 @@ static int smc_hard_start_xmit(struct sk
}
} while (--poll_count);
+ smc_special_unlock(&lp->lock);
+
if (!poll_count) {
/* oh well, wait until the chip finds memory later */
netif_stop_queue(dev);
@@ -1175,6 +1221,8 @@ static irqreturn_t smc_interrupt(int irq
DBG(3, "%s: %s\n", dev->name, __FUNCTION__);
+ spin_lock(&lp->lock);
+
saved_pointer = SMC_GET_PTR();
mask = SMC_GET_INT_MASK();
SMC_SET_INT_MASK(0);
@@ -1196,8 +1244,6 @@ static irqreturn_t smc_interrupt(int irq
if (!status)
break;
- spin_lock(&lp->lock);
-
if (status & IM_RCV_INT) {
DBG(3, "%s: RX irq\n", dev->name);
smc_rcv(dev);
@@ -1240,14 +1286,14 @@ static irqreturn_t smc_interrupt(int irq
SMC_ACK_INT(IM_ERCV_INT);
PRINTK("%s: UNSUPPORTED: ERCV INTERRUPT \n", dev->name);
}
-
- spin_unlock(&lp->lock);
} while (--timeout);
/* restore register states */
SMC_SET_PTR(saved_pointer);
SMC_SET_INT_MASK(mask);
+ spin_unlock(&lp->lock);
+
DBG(3, "%s: Interrupt done (%d loops)\n", dev->name, 8-timeout);
/*
@@ -1471,7 +1517,7 @@ static int smc_close(struct net_device *
netif_carrier_off(dev);
/* clear everything */
- smc_shutdown(dev->base_addr);
+ smc_shutdown(dev);
if (lp->phy_type != 0) {
flush_scheduled_work();
@@ -2093,7 +2139,7 @@ static int smc_drv_suspend(struct device
if (ndev && level == SUSPEND_DISABLE) {
if (netif_running(ndev)) {
netif_device_detach(ndev);
- smc_shutdown(ndev->base_addr);
+ smc_shutdown(ndev);
}
}
return 0;
_