On Tue, Oct 17, 2000 at 05:26:23AM -0400, Jeff Garzik wrote:
> Well, the -spin lock- exists for serialization.  My question is....  Why
> does pc_keyb irq handler disable local irqs for this case?  What is the
> race/deadlock that exists with spin_lock in the irq handler, but does
> not exist with spin_lock_irqsave in the irq handler?

As said the save part isn't necessary.

This is a trace of the deadlock:

        irq 2 runs
        keyboard_interrupt
        irqs are been left enabled
        spin_lock()
        here irq 12 runs
        keyboard_interrupt
        here doesn't matter if irqs are enabled or disabled of course
        spin_lock() -> dealdock

Really if you really want to, you could remove the local irq disable by
replacing it with a disable_irq_nosync(other irq).  Something like this is safe
and obviously right. But I don't think the keyboard_interrupt is long enough to
justify playing with the irq controller [that in IA32 UP isn't memory mapped]
instead of using the fast in CPU core local cli, also the cli version allows
the other irq to be started in parallel to us in another CPU.

        keyboard_interrupt(irq, ...) {
                int other_irq;

                switch(irq) {
                case KEYBOARD_IRQ:
                        other_irq = AUX_IRQ;
                        break;
                case AUX_IRQ:
                        other_irq = KEYBOARD_IRQ;
                        break;
                default:
                        panic("wrong irq");
                }

                disable_irq_nosync(other_irq);
                spin_lock();
                -- critical section --
                spin_unlock();
                enable_irq(other_irq);
        }

BTW, after reading the code some more it seems you could remove the irq disable
also if CONFIG_PSMOUSE is not defined. So you can probably also do:

        keyboard_interrupt(irq, ...) {
#ifdef CONFIG_PSMOUSE
                int other_irq;

                switch(irq) {
                case KEYBOARD_IRQ:
                        other_irq = AUX_IRQ;
                        break;
                case AUX_IRQ:
                        other_irq = KEYBOARD_IRQ;
                        break;
                default:
                        panic("wrong irq");
                }

                disable_irq_nosync(other_irq);
#endif
                spin_lock();
                -- critical section --
                spin_unlock();
#ifdef CONFIG_PSMOUSE
                enable_irq(other_irq);
#endif
        }

Or the __cli version:

        keyboard_interrupt(irq, ...) {
#ifdef CONFIG_PSMOUSE
                __cli();
#endif
                spin_lock();
                -- critical section --
                spin_unlock();
#ifdef CONFIG_PSMOUSE
                __sti(); /* this wouldn't be necessary if 2.4.x would
                            honour SA_INTERRUPT for the shared irqs
                            as 2.2.x */
#endif
        }

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/

Reply via email to