Re: Sleeping function called from illegal context...

[Robert Love <rml@tech9.net>]

On Sat, 2002-09-28 at 06:06, Alan Cox wrote:

> Reminds me though Robert (and Jeff)
> 
> drivers/net/8390.c still needs ei_start_xmit fixing
> 
> pre-emption should be disabled between
> 
>        /* Mask interrupts from the ethercard.
>            SMP: We have to grab the lock here otherwise the IRQ handler
> 
> and
>        disable_irq_nosync(dev->irq);
> 
>         spin_lock(&ei_local->page_lock);

Sounds reasonable enough.  What about the attached patch?  If we flip
the order of the disable_irq and spin_lock, we do not need to actually
explicitly disable preemption... the lock will do that for us.

Is this safe?

This also has the general benefit of not spinning on the lock with the
irq disabled (same sort of downside has preempting with the irq
disabled).

	Robert Love

diff -urN linux-2.5.39/drivers/net/8390.c linux/drivers/net/8390.c
--- linux-2.5.39/drivers/net/8390.c	Fri Sep 27 17:49:05 2002
+++ linux/drivers/net/8390.c	Sat Sep 28 13:02:47 2002
@@ -243,9 +243,9 @@
 	}
 
 	/* Ugly but a reset can be slow, yet must be protected */
-		
-	disable_irq_nosync(dev->irq);
+
 	spin_lock(&ei_local->page_lock);
+	disable_irq_nosync(dev->irq);
 		
 	/* Try to restart the card.  Perhaps the user has fixed something. */
 	ei_reset_8390(dev);
@@ -286,10 +286,9 @@
 	/*
 	 *	Slow phase with lock held.
 	 */
-	 
-	disable_irq_nosync(dev->irq);
-	
+
 	spin_lock(&ei_local->page_lock);
+	disable_irq_nosync(dev->irq);
 	
 	ei_local->irqlock = 1;