[249] in linux-net channel archive

home help back first fref pref prev next nref lref last post

NE2000 fixes --- #3A

daemon@ATHENA.MIT.EDU (Paul Gortmaker)
Sat Apr 29 01:10:11 1995

From: Paul Gortmaker <paul@rasty.anu.edu.au>
To: linux-net@vger.rutgers.edu
Date: Sat, 29 Apr 1995 14:19:21 +1000 (EST)

	
	Okay one more fix. There is a race condition in 8390.c where
you can end up walking ei_start_xmit() twice at the same time, when
it was never designed to handle such abuse. If you are unlucky then
this can still lock a ne2000 card with a DMA conflict message.
(Shared memory cards are much more forgiving...)

	This patch (to be applied over top of the previous one
that was marked for 1.2.6) will block that race from happening.
Note that this will cause more Tx conflicts to be reported than
you would have seen before because quite a few were slipping 
through. This will also stop you from getting messages like:

eth0: No packet buffer space for ping-pong use.

as they originated from the same race. I also shortened the DMA
timeout value to one jiffy (10ms). This may be too short for
8 bit ne1000 cards, but seems like plenty for the 16 bit cards.
Please mention it if you see "timeout waiting for Tx RDC" messages.

Apply this over top of my previous 1.2.6 patch and post results
(good and bad) here.

Paul.

diff -ur linux-fh/drivers/net/8390.c linux/drivers/net/8390.c
--- linux-fh/drivers/net/8390.c	Fri Apr 28 16:47:26 1995
+++ linux/drivers/net/8390.c	Fri Apr 28 16:27:57 1995
@@ -129,6 +129,7 @@
     int e8390_base = dev->base_addr;
     struct ei_device *ei_local = (struct ei_device *) dev->priv;
     int length, send_length;
+    unsigned long flags;
     
 /*
  *  We normally shouldn't be called if dev->tbusy is set, but the
@@ -176,15 +177,22 @@
     if (skb->len <= 0)
 		return 0;
 
-	/* Block a timer-based transmit from overlapping. */
-	if (set_bit(0, (void*)&dev->tbusy) != 0) {
-		printk("%s: Transmitter access conflict.\n", dev->name);
-		return 1;
-	}
+    save_flags(flags);
+    cli();
+
+    /* Block a timer-based transmit from overlapping. */
+    if ((set_bit(0, (void*)&dev->tbusy) != 0) || ei_local->irqlock) {
+	printk("%s: Tx access conflict. irq=%d lock=%d tx1=%d tx2=%d last=%d\n",
+		dev->name, dev->interrupt, ei_local->irqlock, ei_local->tx1,
+		ei_local->tx2, ei_local->lasttx);
+	restore_flags(flags);
+	return 1;
+    }
 
     /* Mask interrupts from the ethercard. */
     outb(0x00, e8390_base + EN0_IMR);
     ei_local->irqlock = 1;
+    restore_flags(flags);
 
     send_length = ETH_ZLEN < length ? length : ETH_ZLEN;
 
@@ -206,8 +214,9 @@
 					   ei_local->txing);
 		} else {	/* We should never get here. */
 			if (ei_debug)
-				printk("%s: No packet buffer space for ping-pong use.\n",
-					   dev->name);
+				printk("%s: No Tx buffers free. irq=%d tx1=%d tx2=%d last=%d\n",
+					dev->name, dev->interrupt, ei_local->tx1, 
+					ei_local->tx2, ei_local->lasttx);
 			ei_local->irqlock = 0;
 			dev->tbusy = 1;
 			outb_p(ENISR_ALL, e8390_base + EN0_IMR);
@@ -215,19 +224,20 @@
 		}
 		ei_block_output(dev, length, skb->data, output_page);
 		if (! ei_local->txing) {
+			ei_local->txing = 1;
 			NS8390_trigger_send(dev, send_length, output_page);
 			dev->trans_start = jiffies;
 			if (output_page == ei_local->tx_start_page)
 				ei_local->tx1 = -1, ei_local->lasttx = -1;
 			else
 				ei_local->tx2 = -1, ei_local->lasttx = -2;
-			ei_local->txing = 1;
 		} else
 			ei_local->txqueue++;
 
 		dev->tbusy = (ei_local->tx1  &&  ei_local->tx2);
     } else {  /* No pingpong, just a single Tx buffer. */
 		ei_block_output(dev, length, skb->data, ei_local->tx_start_page);
+		ei_local->txing = 1;
 		NS8390_trigger_send(dev, send_length, ei_local->tx_start_page);
 		dev->trans_start = jiffies;
 		dev->tbusy = 1;
@@ -346,9 +356,9 @@
 			ei_local->tx1 = 0;
 			dev->tbusy = 0;
 			if (ei_local->tx2 > 0) {
+				ei_local->txing = 1;
 				NS8390_trigger_send(dev, ei_local->tx2, ei_local->tx_start_page + 6);
 				dev->trans_start = jiffies;
-				ei_local->txing = 1;
 				ei_local->tx2 = -1,
 				ei_local->lasttx = 2;
 			} else
@@ -360,9 +370,9 @@
 			ei_local->tx2 = 0;
 			dev->tbusy = 0;
 			if (ei_local->tx1 > 0) {
+				ei_local->txing = 1;
 				NS8390_trigger_send(dev, ei_local->tx1, ei_local->tx_start_page);
 				dev->trans_start = jiffies;
-				ei_local->txing = 1;
 				ei_local->tx1 = -1;
 				ei_local->lasttx = 1;
 			} else
@@ -621,6 +631,7 @@
     struct ei_device *ei_local = (struct ei_device *) dev->priv;
     int i;
     int endcfg = ei_local->word16 ? (0x48 | ENDCFG_WTS) : 0x48;
+    unsigned long flags;
     
     /* Follow National Semi's recommendations for initing the DP83902. */
     outb_p(E8390_NODMA+E8390_PAGE0+E8390_STOP, e8390_base); /* 0x21 */
@@ -644,6 +655,7 @@
     
     /* Copy the station address into the DS8390 registers,
        and set the multicast hash bitmap to receive all multicasts. */
+    save_flags(flags);
     cli();
     outb_p(E8390_NODMA + E8390_PAGE1 + E8390_STOP, e8390_base); /* 0x61 */
     for(i = 0; i < 6; i++) {
@@ -656,7 +668,7 @@
     
     outb_p(ei_local->rx_start_page,	 e8390_base + EN1_CURPAG);
     outb_p(E8390_NODMA+E8390_PAGE0+E8390_STOP, e8390_base);
-    sti();
+    restore_flags(flags);
     dev->tbusy = 0;
     dev->interrupt = 0;
     ei_local->tx1 = ei_local->tx2 = 0;
@@ -678,7 +690,6 @@
 {
     int e8390_base = dev->base_addr;
     
-    ei_status.txing = 1;
     outb_p(E8390_NODMA+E8390_PAGE0, e8390_base);
     
     if (inb_p(e8390_base) & E8390_TRANS) {
diff -ur linux-fh/drivers/net/ne.c linux/drivers/net/ne.c
--- linux-fh/drivers/net/ne.c	Fri Apr 28 16:47:29 1995
+++ linux/drivers/net/ne.c	Fri Apr 28 16:22:21 1995
@@ -86,7 +86,7 @@
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
 
-#define NE_RDC_TIMEOUT	0x02	/* Max wait in jiffies for Tx RDC */
+#define NE_RDC_TIMEOUT	0x01	/* Max wait in jiffies for Tx RDC */
 
 int ne_probe(struct device *dev);
 static int ne_probe1(struct device *dev, int ioaddr);
@@ -358,6 +358,7 @@
 #endif
     int nic_base = dev->base_addr;
 
+    /* This *shouldn't* happen. If it does, it's the last thing you'll see */
     if (set_bit(0,(void*)&ei_status.dmaing)) {
 	if (ei_debug > 0)
 	    printk("%s: DMAing conflict in ne_block_input "
@@ -427,6 +428,8 @@
        I should check someday. */
     if (ei_status.word16 && (count & 0x01))
       count++;
+
+    /* This *shouldn't* happen. If it does, it's the last thing you'll see */
     if (set_bit(0,(void*)&ei_status.dmaing)) {
 	if (ei_debug > 0)
 	    printk("%s: DMAing conflict in ne_block_output."
@@ -499,8 +502,7 @@
 
     while ((inb_p(nic_base + EN0_ISR) & ENISR_RDC) == 0)
 	if (jiffies - dma_start > NE_RDC_TIMEOUT) {
-		printk("%s: timeout waiting for Tx RDC, DMAstat: %d", 
-				dev->name, ei_status.dmaing);
+		printk("%s: timeout waiting for Tx RDC.\n", dev->name);
 		ne_reset_8390(dev);
 		NS8390_init(dev,1);
 		break;


home help back first fref pref prev next nref lref last post