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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Oct 23 2000 - 21:00:11 EST