Re: [RFC] genirq: Flush the irq thread on synchronization

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, 2 Dec 2011, Ido Yariv wrote:

> The current implementation does not always flush the threaded handler
> when disabling the irq. In case the irq handler was called, but the
> threaded handler hasn't started running yet, the interrupt will be
> flagged as pending, and the handler will not run. This implementation
> has some issues:
> 
> First, if the interrupt is a wake source and flagged as pending, the
> system will not be able to suspend.
> 
> Second, when quickly disabling and re-enabling the irq, the threaded
> handler might continue to run after the irq is re-enabled without the
> irq handler being called first. This might be an unexpected behavior.

I'd wish people would stop calling disable/enable_irq() in loops and
circles for no reason.

> In addition, it might be counter-intuitive that the threaded handler
> will not be called even though the irq handler was called and returned
> IRQ_WAKE_THREAD.
> 
> Fix this by always waiting for the threaded handler to complete in
> synchronize_irq().

I can see your problem, but this might lead to threads_active leaks
under certain conditions. desc->threads_active was only meant to deal
with shared interrupts.

We explicitely allow a design where the primary handler can leave the
device interrupt enabled and allow further interrupts to occur while
the handler is running. We only have a single bit to note that the
thread should run, but your wakeup would up the threads_active count
in that scenario several times w/o a counterpart which decrements it.

The solution for this is to keep the current threads_active semantics
and make the wait function different. Instead of waiting for
threads_active to become 0 it should wait for threads_active == 0 and
the IRQTF_RUNTHREAD for all actions to be cleared. To avoid looping
over the actions, we can take a similar approach as we take with the
desc->threads_oneshot bitfield.

Thanks,

	tglx