Re: [PATCH]Re: spin_locks without smp.

From: Alan Cox (alan@lxorguk.ukuu.org.uk)
Date: Fri Jan 10 2003 - 08:20:48 EST


On Fri, 2003-01-10 at 12:09, Maciej Soltysiak wrote:
> > On Fri, Jan 10, 2003 at 03:45:46AM -0800, William Lee Irwin III wrote:
> > > Buggy on preempt. Remove the #ifdef
> Yes sir. :)
>
> Is that okay?

I'm not convinced its the right way. The driver does the things it does in order
to keep performance acceptable. eexpress, 8390 and one or two other
drivers have a paticular problem that is hard to handle with our current
locks (and which at the time Linus made a decision wasn't a good thing
to try and handle generically).

We have to ensure that the IRQ path doesn't execute in parallel with
the transmit/timeout path. At the same time the packet upload to the
card is extremely slow. Sufficiently slow in fact that serial ports
just stop working when you use it without the ifdef paths.

On uniprocessor systems even with pre-empt the IRQ handler cannot be
pre-empted by normal code execution. On SMP they can run across two
processors. What the disable_irq path is doing for uniprocessor is
implementing

        spin_lock_irqsavesomeirqsonly()

and on the kind of boxes that have these old cards its pretty important
to keep this.

I would argue that if we have an IRQ disabled we should forbid pre-empt.
If an IRQ is disabled and we pre-empt to a task that needs to allocate
memory and we swap to a device on that IRQ we may deadlock.

So the fix is either to make disable_irq()/enable_irq() correctly
adjust the pre-empt restrictions, which is actually quite hard to see
how to do right as the disable/enable may be in different tasks, or to
change the code to do the following

        preempt_disable()
        disable_irq()
#ifdef CONFIG_SMP
        spin_lock_...
#endif

Note that we must disable the irq before taking the spinlock or we
have another deadlock with our irq path versus disable_irq waiting
for the IRQ completion before returning.

If my analysis of the disable_irq versus pre-empt and memory allocation
deadlock is correct we have some other cases we need to address too.

Alan

-
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 : Wed Jan 15 2003 - 22:00:32 EST