Do I need to protect my interrupt handler being called many times for the same interrupt?
Given the following code, I am not sure on the system calls I should make. I am getting rare, random dead-locks with this current implementation :-
void interrupt_handler(void)
{
down_interruptible(&sem); // or use a lock here ?
clear_intr(); // clear interrupt source on H/W
wake_up_interruptible(...);
up(&sem); // unlock?
return IRQ_HANDLED;
}
void set/clear_intr()
{
spin_lock_irq(&lock);
RMW(x); // set/clear a bit by read/modify/write the H/W interrupt routing register
spin_unlock_irq(&lock);
}
void read()
{
set_intr(); // same as clear_intr, but sets a bit
wait_event_interruptible(...);
}
interrupt_handler
:down_interruptible
be spin_lock_irq
/ spin_lock_irqsave
/ local_irq_disable
?set/clear_intr
:spin_lock_irq
be spin_lock_irqsave
/ local_irq_disable
?interrupt_handler
keep getting called while within it?down_interruptible
?From LDD3 :-
must be reentrant—it must be capable of running in more than one context at the same time.
Edit 1) after some nice help, suggestions are :-
down_interruptible
from within interrupt_handler
spin_lock_irq
outside set/clear methods (no need for spin_lock_irqsave
you say?) I really don't see the benefit to this?!
Code :-
void interrupt_handler(void)
{
read_reg(y); // eg of other stuff in the handler
spin_lock_irq(&lock);
clear_intr(); // clear interrupt source on H/W
spin_unlock_irq(&lock);
wake_up_interruptible(...);
return IRQ_HANDLED;
}
void set/clear_intr()
{
RMW(x);
}
void read()
{
error_checks(); // eg of some other stuff in the read method
spin_lock_irq(&lock);
set_intr(); // same as clear_intr, but sets a bit
spin_unlock_irq(&lock);
wait_event_interruptible(...);
// more code here...
}
Edit2) After reading some more SO posts : reading Why kernel code/thread executing in interrupt context cannot sleep? which links to Robert Loves article, I read this :
some interrupt handlers (known in Linux as fast interrupt handlers) run with all interrupts on the local processor disabled. This is done to ensure that the interrupt handler runs without interruption, as quickly as possible. More so, all interrupt handlers run with their current interrupt line disabled on all processors. This ensures that two interrupt handlers for the same interrupt line do not run concurrently. It also prevents device driver writers from having to handle recursive interrupts, which complicate programming.
And I have fast interrupts enabled (SA_INTERRUPT)! So no need for mutex/locks/semaphores/spins/waits/sleeps/etc/etc!
The mutex is fine for arbitrating between two threads, eg. to allow only one at a time to access a tx queue in order to enqueue data for sending, but the mutex cannot be used to manage interaction with the interrupt-handler code that directly responds to the TX_EMPTY, TXFIFO_EMPTY. or whatever it's called, hardware ...
Locking mutex is prohibited within interrupt context. The reason is same as one for semaphores and is described in the first related question. A spin lock is a form of mutex.
However, such kernel control paths may be arbitrarily nested; an interrupt handler may be interrupted by another interrupt handler, thus giving raise to a nested execution of kernel threads.
if the handler sleeps, then the system may hang because the system clock interrupt is masked and incapable of scheduling the sleeping process.
Don't use semaphores in interrupt context, use spin_lock_irqsave
instead. quoting LDD3:
If you have a spinlock that can be taken by code that runs in (hardware or software) interrupt context, you must use one of the forms of spin_lock that disables interrupts. Doing otherwise can deadlock the system, sooner or later. If you do not access your lock in a hardware interrupt handler, but you do via software interrupts (in code that runs out of a tasklet, for example, a topic covered in Chapter 7), you can use spin_lock_bh to safely avoid deadlocks while still allowing hardware interrupts to be serviced.
As for point 2, make your set_intr
and clear_intr
require the caller to lock the spinlock, otherwise you'll find your code deadlocking. Again from LDD3:
To make your locking work properly, you have to write some functions with the assumption that their caller has already acquired the relevant lock(s). Usually, only your internal, static functions can be written in this way; functions called from outside must handle locking explicitly. When you write internal functions that make assumptions about locking, do yourself (and anybody else who works with your code) a favor and document those assumptions explicitly. It can be very hard to come back months later and figure out whether you need to hold a lock to call a particular function or not.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With