[PATCH] 8390.c: preemption and disable_irq

Robert Love (rml@tech9.net)
01 Oct 2002 17:58:00 -0400


Jeff,

Alan pointed out to me that drivers/net/8390.c calls disable_irq() while
non-atomic. This leads to the possibility that the kernel is preempted
while the IRQ line is disabled.

This is not a bug but obviously not optimal. The solution is simply to
disable preemption prior to calling disable_irq().

Patch against 2.5.40 is attached. Look OK?

Robert Love

P.S. Note there is a much simpler solution of merely flipping the
spin_lock() and disable_irq() calls - the spin_lock() will provide the
atomicity without the explicit preempt_disable(). I do not think this
is safe, however, since the IRQ handler could deadlock if it grabbed
this lock (which I assume it does).

diff -urN linux-2.5.40/drivers/net/8390.c linux/drivers/net/8390.c
--- linux-2.5.40/drivers/net/8390.c Tue Oct 1 03:06:17 2002
+++ linux/drivers/net/8390.c Tue Oct 1 17:49:11 2002
@@ -243,7 +243,8 @@
}

/* Ugly but a reset can be slow, yet must be protected */
-
+
+ preempt_disable();
disable_irq_nosync(dev->irq);
spin_lock(&ei_local->page_lock);

@@ -253,6 +254,7 @@

spin_unlock(&ei_local->page_lock);
enable_irq(dev->irq);
+ preempt_enable();
netif_wake_queue(dev);
}

@@ -286,9 +288,9 @@
/*
* Slow phase with lock held.
*/
-
+
+ preempt_disable();
disable_irq_nosync(dev->irq);
-
spin_lock(&ei_local->page_lock);

ei_local->irqlock = 1;
@@ -331,6 +333,7 @@
outb_p(ENISR_ALL, e8390_base + EN0_IMR);
spin_unlock(&ei_local->page_lock);
enable_irq(dev->irq);
+ preempt_enable();
ei_local->stat.tx_errors++;
return 1;
}
@@ -387,6 +390,7 @@

spin_unlock(&ei_local->page_lock);
enable_irq(dev->irq);
+ preempt_enable();

dev_kfree_skb (skb);
ei_local->stat.tx_bytes += send_length;

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