Messages in this thread | | | Date | Tue, 17 Oct 2000 02:55:52 +0200 | From | Andrea Arcangeli <> | Subject | Re: PATCH 2.4.0.10.3: pc_keyb and q40_keyb cleanup |
| |
On Mon, Oct 16, 2000 at 05:31:47PM -0400, Jeff Garzik wrote: > Andrea Arcangeli wrote: > > > > On Sun, Oct 15, 2000 at 03:48:55PM -0400, Jeff Garzik wrote: > > > Changes: > > > * both: we know we are in an interrupt, so > > > s/spin_lock_irqsave/spin_lock/ > > > > There request_irq is not called passing the SA_INTERRUPT flag so the irq > > handler is recalled with irqs enabled and in turn irqsave is necessary. > > hmmm. Can you elaborate on this a bit? > > I didn't know that bit about !SA_INTERRUPT, but why is irqsave necessary > in these drivers? [..]
if you do:
request_irq(&my_irq_handler,... 0, ...)
then my_irq_handler will be recalled with irq enabled.
If you do:
request_irq(&my_irq_handler,... SA_INTERRUPT, ...)
then my_irq_handler will be recalled with irq disabled.
SA_INTERRUPT only controls if an irq handler is recalled with irq enabled or disabled (locally of course).
keyboard_interrupt is registered w/o SA_INTERRUPT so it will be recalled with irq enabled and so you someway need to do a __cli() before spinning on the lock.
>[..] I don't see handle_kbd_event() being called from a > timer or anything special like that..
Timer, bottomhalves (softirq) and tasklets (and softnet) are always recalled with irq enabled. So if it would be called by timer/tasklet/bhhandler it should use irq version of the spinlocks too if it needs to run with irq locally disabled.
One thing you could safely change in keyboard_interrupt is to remove the save part of the spinlock by using spin_lock_irq (we don't need to save anything since keyboard_interrupt is only recalled as an irq handler).
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/
| |