lkml.org 
[lkml]   [1998]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] plip breakage (Re: Linux-2.2.0 (pre1))


On Mon, 28 Dec 1998, Linus Torvalds wrote:

>
>
> On Mon, 28 Dec 1998, Alan Cox wrote:
> > > we're in the pre-2.2.0 series now, I'm all synched up with Alan, and I
> > > don't have anything pending any more. Over the internet nobody can hear
> >
> > Almost ;) 100,000fd's and vma patch short. To follow once I can seperate them
> > from the other -ac stuff.
>
> The multi-fd patch wasn't stable the last I heard - unless the multi-fd
> patch has magically stabilized since, it's not 2.2.0 material.
>
> The vma one I just forgot about.
>
> Linus


Linus, PLIP got broken by changes in enable/disable_irq. Driver
assumed that disable_irq() is idempotent. Well, since '132 it isn't.
Result: any dropped package => disabled IRQ => no incoming traffic
accepted. Reproducible within 5 seconds of pingflood from K5-75 to 486DX2.
Or just several minutes of normal traffic. I've finally written the darn
finite automate down and rebalanced disable_irq()/enable_irq() stuff ;-/
It seems to be OK now (compiles, works, doesn't hang, survives *bad*
loads and I don't see any holes in the correctness proof).
I'm afraid that we'll have to check quite a few drivers and it's
not going to be a pleasant work ;-<<<
Patch for PLIP follows. Could you apply it?
TIA,
Al

diff -urN linux.vanilla/drivers/net/plip.c linux.bird/drivers/net/plip.c
--- linux.vanilla/drivers/net/plip.c Fri Nov 13 18:01:56 1998
+++ linux.bird/drivers/net/plip.c Mon Dec 28 21:52:33 1998
@@ -21,6 +21,10 @@
* - Make sure other end is OK, before sending a packet.
* - Fix immediate timer problem.
*
+ * Al Viro
+ * - Changed {enable,disable}_irq handling to make it work
+ * with new ("stack") semantics.
+ *
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
@@ -120,6 +124,9 @@
#endif
static unsigned int net_debug = NET_DEBUG;

+#define ENABLE(irq) enable_irq(irq)
+#define DISABLE(irq) disable_irq(irq)
+
/* In micro second */
#define PLIP_DELAY_UNIT 1

@@ -333,6 +340,7 @@
#define OK 0
#define TIMEOUT 1
#define ERROR 2
+#define HS_TIMEOUT 3

typedef int (*plip_func)(struct device *dev, struct net_local *nl,
struct plip_local *snd, struct plip_local *rcv);
@@ -371,13 +379,22 @@
int error)
{
unsigned char c0;
+ /*
+ * This is tricky. If we got here from the beginning of send (either
+ * with ERROR or HS_TIMEOUT) we have IRQ enabled. Otherwise it's
+ * already disabled. With the old variant of {enable,disable}_irq()
+ * extra disable_irq() was a no-op. Now it became mortal - it's
+ * unbalanced and thus we'll never re-enable IRQ (until rmmod plip,
+ * that is). So we have to treat HS_TIMEOUT and ERROR from send
+ * in a special way.
+ */

spin_lock_irq(&nl->lock);
if (nl->connection == PLIP_CN_SEND) {

if (error != ERROR) { /* Timeout */
nl->timeout_count++;
- if ((snd->state == PLIP_PK_TRIGGER
+ if ((error == HS_TIMEOUT
&& nl->timeout_count <= 10)
|| nl->timeout_count <= 3) {
spin_unlock_irq(&nl->lock);
@@ -387,7 +404,8 @@
c0 = inb(PAR_STATUS(dev));
printk(KERN_WARNING "%s: transmit timeout(%d,%02x)\n",
dev->name, snd->state, c0);
- }
+ } else
+ error = HS_TIMEOUT;
nl->enet_stats.tx_errors++;
nl->enet_stats.tx_aborted_errors++;
} else if (nl->connection == PLIP_CN_RECEIVE) {
@@ -419,8 +437,10 @@
snd->skb = NULL;
}
spin_unlock_irq(&nl->lock);
- disable_irq(dev->irq);
- synchronize_irq();
+ if (error == HS_TIMEOUT) {
+ DISABLE(dev->irq);
+ synchronize_irq();
+ }
outb(PAR_INTR_OFF, PAR_CONTROL(dev));
dev->tbusy = 1;
nl->connection = PLIP_CN_ERROR;
@@ -498,7 +518,7 @@

switch (rcv->state) {
case PLIP_PK_TRIGGER:
- disable_irq(dev->irq);
+ DISABLE(dev->irq);
/* Don't need to synchronize irq, as we can safely ignore it */
outb(PAR_INTR_OFF, PAR_CONTROL(dev));
dev->interrupt = 0;
@@ -518,7 +538,7 @@
nl->connection = PLIP_CN_SEND;
queue_task(&nl->deferred, &tq_timer);
outb(PAR_INTR_ON, PAR_CONTROL(dev));
- enable_irq(dev->irq);
+ ENABLE(dev->irq);
return OK;
}
} else {
@@ -592,13 +612,13 @@
queue_task(&nl->immediate, &tq_immediate);
mark_bh(IMMEDIATE_BH);
outb(PAR_INTR_ON, PAR_CONTROL(dev));
- enable_irq(dev->irq);
+ ENABLE(dev->irq);
return OK;
} else {
nl->connection = PLIP_CN_NONE;
spin_unlock_irq(&nl->lock);
outb(PAR_INTR_ON, PAR_CONTROL(dev));
- enable_irq(dev->irq);
+ ENABLE(dev->irq);
return OK;
}
}
@@ -674,7 +694,7 @@
switch (snd->state) {
case PLIP_PK_TRIGGER:
if ((inb(PAR_STATUS(dev)) & 0xf8) != 0x80)
- return TIMEOUT;
+ return HS_TIMEOUT;

/* Trigger remote rx interrupt. */
outb(0x08, data_addr);
@@ -691,12 +711,16 @@
c0 = inb(PAR_STATUS(dev));
if (c0 & 0x08) {
spin_unlock_irq(&nl->lock);
- disable_irq(dev->irq);
+ DISABLE(dev->irq);
synchronize_irq();
if (nl->connection == PLIP_CN_RECEIVE) {
/* Interrupted.
We don't need to enable irq,
as it is soon disabled. */
+ /* Yes, we do. New variant of
+ {enable,disable}_irq *counts*
+ them. -- AV */
+ ENABLE(dev->irq);
nl->enet_stats.collisions++;
return OK;
}
@@ -711,7 +735,7 @@
spin_unlock_irq(&nl->lock);
if (--cx == 0) {
outb(0x00, data_addr);
- return TIMEOUT;
+ return HS_TIMEOUT;
}
}

@@ -760,7 +784,7 @@
nl->is_deferred = 1;
queue_task(&nl->deferred, &tq_timer);
outb(PAR_INTR_ON, PAR_CONTROL(dev));
- enable_irq(dev->irq);
+ ENABLE(dev->irq);
return OK;
}
return OK;
@@ -800,7 +824,7 @@
dev->tbusy = 0;
dev->interrupt = 0;
outb(PAR_INTR_ON, PAR_CONTROL(dev));
- enable_irq(dev->irq);
+ ENABLE(dev->irq);
mark_bh(NET_BH);
} else {
nl->is_deferred = 1;
@@ -1000,7 +1024,7 @@

dev->tbusy = 1;
dev->start = 0;
- disable_irq(dev->irq);
+ DISABLE(dev->irq);
synchronize_irq();

#ifdef NOTDEF


-
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/

\
 
 \ /
  Last update: 2005-03-22 13:46    [W:0.706 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site