Messages in this thread |  | | Date | Tue, 17 Oct 2000 19:00:54 +0200 | From | Andrea Arcangeli <> | Subject | Re: PATCH 2.4.0.10.3: pc_keyb and q40_keyb cleanup |
| |
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/
|  |