lkml.org 
[lkml]   [2000]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: 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/

\
 
 \ /
  Last update: 2005-03-22 12:41    [from the cache]
©2003-2011 Jasper Spaans