Re: The Deadly `ping -f'

[Philip Blundell <philb@gnu.org>]

>Yes, it is SMP indeed. A UP kernel+modules works allright, using either
>3c509 or eexpress. A SMP kernel fails with both. The eexpress driver is
>more interesting, though, it spits out these:

I had a look through the eexpress driver and it did seem to be missing some 
SMP locking.  I'm not especially hopeful that this will fix the problems you 
were seeing, but here's a patch that should do roughly the right thing anyway.
I'd also be somewhat interested to hear if it makes any difference for better 
or worse on UP systems.

p.

--- clean/linux/drivers/net/eexpress.c	Sat May  1 14:18:14 1999
+++ linux/drivers/net/eexpress.c	Sat May  1 14:51:36 1999
@@ -57,6 +57,12 @@
  * We don't use the shared-memory system because it allegedly doesn't work on
  * all cards, and because it's a bit more prone to go wrong (it's one more
  * thing to configure...).
+ *
+ * Note that using dataport access requires us to be a little bit careful
+ * about concurrency.  Currently the READ_PTR is only used inside the
+ * receive interrupt routine so no special precautions are needed.  The
+ * WRITE_PTR, though, is used by the interrupt handler and the transmit
+ * code.  We use a spin lock to protect against conflicts here.
  */
 
 /* Known bugs:
@@ -141,6 +147,8 @@
 	unsigned char width;         /* 0 for 16bit, 1 for 8bit */
 	unsigned char was_promisc;
 	unsigned char old_mc_count;
+
+	spinlock_t lock;
 };
 
 /* This is the code and data that is downloaded to the EtherExpress card's
@@ -634,7 +642,6 @@
 	struct device *dev = dev_info;
 	struct net_local *lp;
 	unsigned short ioaddr,status,ack_cmd;
-	unsigned short old_read_ptr, old_write_ptr;
 
 	if (dev==NULL)
 	{
@@ -646,9 +653,6 @@
 	lp = (struct net_local *)dev->priv;
 	ioaddr = dev->base_addr;
 
-	old_read_ptr = inw(ioaddr+READ_PTR);
-	old_write_ptr = inw(ioaddr+WRITE_PTR);
-
 	outb(SIRQ_dis|irqrmap[irq],ioaddr+SET_IRQ);
 
 	dev->interrupt = 1;
@@ -724,9 +728,6 @@
 #if NET_DEBUG > 6 
 	printk("%s: leaving eexp_irq()\n", dev->name);
 #endif
-	outw(old_read_ptr, ioaddr+READ_PTR);
-	outw(old_write_ptr, ioaddr+WRITE_PTR);
-	return;
 }
 
 /*
@@ -837,9 +838,11 @@
 				lp->stats.rx_packets++;
 				lp->stats.rx_bytes += pkt_len;
 			}
+			spin_lock(&lp->lock);
 			outw(rx_block, ioaddr+WRITE_PTR);
 			outw(0, ioaddr+DATAPORT);
 			outw(0, ioaddr+DATAPORT);
+			spin_unlock(&lp->lock);
 			rx_block = rx_next;
 		}
 	} while (FD_Done(status) && boguscount--);
@@ -858,6 +861,7 @@
 {
 	struct net_local *lp = (struct net_local *)dev->priv;
 	unsigned short ioaddr = dev->base_addr;
+	unsigned long flags;
 
 	if (lp->width) {
 		/* Stop the CU so that there is no chance that it
@@ -869,6 +873,8 @@
 		outw(0xFFFF, ioaddr+SIGNAL_CA);
 	}
 
+	spin_lock_irqsave(&lp->lock, flags);
+
  	outw(lp->tx_head, ioaddr + WRITE_PTR);
 
 	outw(0x0000, ioaddr + DATAPORT);
@@ -890,6 +896,8 @@
 	outw(lp->tx_tail+0xc, ioaddr + WRITE_PTR);
 	outw(lp->tx_head, ioaddr + DATAPORT);
 
+	spin_unlock_irqrestore(&lp->lock, flags);
+
 	dev->trans_start = jiffies;
 	lp->tx_tail = lp->tx_head;
 	if (lp->tx_head==TX_BUF_START+((lp->num_tx_bufs-1)*TX_BUF_SIZE))
@@ -981,6 +989,8 @@
  	printk("(IRQ %d, %s connector, %d-bit bus", dev->irq, 
  	       eexp_ifmap[dev->if_port], buswidth?8:16);
  
+	spin_lock_init(&lp->lock);
+
  	eexp_hw_set_interface(dev);
   
 	/* Find out how much RAM we have on the card */
@@ -1428,7 +1438,6 @@
 #if NET_DEBUG > 6
         printk("%s: leaving eexp_hw_init586()\n", dev->name);
 #endif
-	return;
 }
 
 static void eexp_setup_filter(struct device *dev)



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/