genirq: Flush the irq thread on synchronization

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.

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().

[ tglx: Massaged comments, added WARN_ONs and the missing
  	IRQTF_RUNTHREAD check in exit_irq_thread() ]

Signed-off-by: Ido Yariv <ido@wizery.com>
Link: http://lkml.kernel.org/r/1322843052-7166-1-git-send-email-ido@wizery.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 500aaf67c546ca2912ae5bd0e697ae1325583a8e..6ff84e6a954c052ed1f7f483559728a6154647d6 100644 (file)
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -110,6 +110,18 @@ static void irq_wake_thread(struct irq_desc *desc, struct irqaction *action)
         * threads_oneshot untouched and runs the thread another time.
         */
        desc->threads_oneshot |= action->thread_mask;
+
+       /*
+        * We increment the threads_active counter in case we wake up
+        * the irq thread. The irq thread decrements the counter when
+        * it returns from the handler or in the exit path and wakes
+        * up waiters which are stuck in synchronize_irq() when the
+        * active count becomes zero. synchronize_irq() is serialized
+        * against this code (hard irq handler) via IRQS_INPROGRESS
+        * like the finalize_oneshot() code. See comment above.
+        */
+       atomic_inc(&desc->threads_active);
+
        wake_up_process(action->thread);
 }
 

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1786cf7dac545098034a7fed8ee9a846c4ffab55..453feedbb3903e26cf94e4102e5d787c4aecb72f 100644 (file)
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -759,6 +759,13 @@ static irqreturn_t irq_thread_fn(struct irq_desc *desc,
        return ret;
 }
 
+static void wake_threads_waitq(struct irq_desc *desc)
+{
+       if (atomic_dec_and_test(&desc->threads_active) &&
+           waitqueue_active(&desc->wait_for_threads))
+               wake_up(&desc->wait_for_threads);
+}
+
 /*
  * Interrupt handler thread
  */
@@ -771,7 +778,6 @@ static int irq_thread(void *data)
        struct irq_desc *desc = irq_to_desc(action->irq);
        irqreturn_t (*handler_fn)(struct irq_desc *desc,
                        struct irqaction *action);
-       int wake;
 
        if (force_irqthreads && test_bit(IRQTF_FORCED_THREAD,
                                        &action->thread_flags))
@@ -783,39 +789,30 @@ static int irq_thread(void *data)
        current->irq_thread = 1;
 
        while (!irq_wait_for_interrupt(action)) {
+               irqreturn_t action_ret;
 
                irq_thread_check_affinity(desc, action);
 
-               atomic_inc(&desc->threads_active);
+               action_ret = handler_fn(desc, action);
+               if (!noirqdebug)
+                       note_interrupt(action->irq, desc, action_ret);
 
-               raw_spin_lock_irq(&desc->lock);
-               if (unlikely(irqd_irq_disabled(&desc->irq_data))) {
-                       /*
-                        * CHECKME: We might need a dedicated
-                        * IRQ_THREAD_PENDING flag here, which
-                        * retriggers the thread in check_irq_resend()
-                        * but AFAICT IRQS_PENDING should be fine as it
-                        * retriggers the interrupt itself --- tglx
-                        */
-                       desc->istate |= IRQS_PENDING;
-                       raw_spin_unlock_irq(&desc->lock);
-               } else {
-                       irqreturn_t action_ret;
-
-                       raw_spin_unlock_irq(&desc->lock);
-                       action_ret = handler_fn(desc, action);
-                       if (!noirqdebug)
-                               note_interrupt(action->irq, desc, action_ret);
-               }
-
-               wake = atomic_dec_and_test(&desc->threads_active);
-
-               if (wake && waitqueue_active(&desc->wait_for_threads))
-                       wake_up(&desc->wait_for_threads);
+               wake_threads_waitq(desc);
        }
 
-       /* Prevent a stale desc->threads_oneshot */
-       irq_finalize_oneshot(desc, action, true);
+       /*
+        * This is the regular exit path. __free_irq() is stopping the
+        * thread via kthread_stop() after calling
+        * synchronize_irq(). So neither IRQTF_RUNTHREAD nor the
+        * oneshot mask bit should be set.
+        *
+        * Verify that this is true.
+        */
+       if (WARN_ON(test_and_clear_bit(IRQTF_RUNTHREAD, &action->thread_flags)))
+               wake_threads_waitq(desc);
+
+       if (WARN_ON(desc->threads_oneshot & action->thread_mask))
+               irq_finalize_oneshot(desc, action, true);
 
        /*
         * Clear irq_thread. Otherwise exit_irq_thread() would make
@@ -845,6 +842,13 @@ void exit_irq_thread(void)
 
        desc = irq_to_desc(action->irq);
 
+       /*
+        * If IRQTF_RUNTHREAD is set, we need to decrement
+        * desc->threads_active and wake possible waiters.
+        */
+       if (test_and_clear_bit(IRQTF_RUNTHREAD, &action->thread_flags))
+               wake_threads_waitq(desc);
+
        /* Prevent a stale desc->threads_oneshot */
        irq_finalize_oneshot(desc, action, true);
 }