Re: Alans example against preemtive kernel (Was: Re: [2.4.17/18pre] VM and swap - it's really unusable)

Roger Larsson
2002-01-14T07:26:47Z

On Saturday den 12 January 2002 19.54, Alan Cox wrote:
> Another example is in the network drivers. The 8390 core for one example
> carefully disables an IRQ on the card so that it can avoid spinlocking on
> uniprocessor boxes.
>
> So with pre-empt this happens
>
> driver magic
> disable_irq(dev->irq)
> PRE-EMPT:
> [large periods of time running other code]
> PRE-EMPT:
> We get back and we've missed 300 packets, the serial port sharing
> the IRQ has dropped our internet connection completely.
>
> ["Don't do that then" isnt a valid answer here. If I did hold a lock
> it would be for several milliseconds at a time anyway and would reliably
> trash performance this time]
>

./drivers/net/8390.c
I checked the code ./drivers/net/8390.c - this is how it REALLY looks like...

/* Ugly but a reset can be slow, yet must be protected */

disable_irq_nosync(dev->irq);
spin_lock(&ei_local->page_lock);

/* Try to restart the card. Perhaps the user has fixed something. */
ei_reset_8390(dev);
NS8390_init(dev, 1);

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

This should be mostly OK for the preemptive kernel. Swapping the irq and spin
lock lines should be preferred. But I think that is the case in SMP too...

Suppose two processors does the disable_irq_nosync - unlikely but possible...
One gets the spinlock, the other waits
The first runs through the code, exits the spin lock, enables irq
The second starts running the code - without irq disabled!!!

This would work in both cases.
/* Ugly but a reset can be slow, yet must be protected */

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);
NS8390_init(dev, 1);

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

/RogerL

--
Roger Larsson
Skellefteå
Sweden