[PATCH] 8390.c: preemption and disable_irq

From: Robert Love (rml@tech9.net)
Date: Tue Oct 01 2002 - 16:58:00 EST


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/



This archive was generated by hypermail 2b29 : Mon Oct 07 2002 - 22:00:29 EST