lkml.org 
[lkml]   [2011]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] genirq: Flush the irq thread on synchronization
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




\
 
 \ /
  Last update: 2011-12-03 00:23    [from the cache]
©2003-2011 Jasper Spaans