From 986cb48c5a4de0085db94d343b4e7dcf54355ec1 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sun, 19 Feb 2012 11:46:36 -0800 Subject: [PATCH 1/9] x86-32/irq: Don't switch to irq stack for a user-mode irq If the irq happens in user mode, our kernel stack is empty (apart from the pt_regs themselves, of course), so there's no need or advantage to switch. And it really doesn't save any stack space, quite the reverse: it means that a nested interrupt cannot switch irq stacks. So instead of saving kernel stack space, it actually causes the potential for *more* stack usage. Also simplify the preemption count copy when we do switch stacks: just copy the whole preemption count, rather than just the softirq parts of it. There is no advantage to the partial copy: it is more effort to get a less correct result. Signed-off-by: Linus Torvalds Link: http://lkml.kernel.org/r/alpine.LFD.2.02.1202191139260.10000@i5.linux-foundation.org Signed-off-by: Ingo Molnar --- arch/x86/kernel/irq_32.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/arch/x86/kernel/irq_32.c b/arch/x86/kernel/irq_32.c index 40fc86161d92..58b7f27cb3e9 100644 --- a/arch/x86/kernel/irq_32.c +++ b/arch/x86/kernel/irq_32.c @@ -100,13 +100,8 @@ execute_on_irq_stack(int overflow, struct irq_desc *desc, int irq) irqctx->tinfo.task = curctx->tinfo.task; irqctx->tinfo.previous_esp = current_stack_pointer; - /* - * Copy the softirq bits in preempt_count so that the - * softirq checks work in the hardirq context. - */ - irqctx->tinfo.preempt_count = - (irqctx->tinfo.preempt_count & ~SOFTIRQ_MASK) | - (curctx->tinfo.preempt_count & SOFTIRQ_MASK); + /* Copy the preempt_count so that the [soft]irq checks work. */ + irqctx->tinfo.preempt_count = curctx->tinfo.preempt_count; if (unlikely(overflow)) call_on_stack(print_stack_overflow, isp); @@ -196,7 +191,7 @@ bool handle_irq(unsigned irq, struct pt_regs *regs) if (unlikely(!desc)) return false; - if (!execute_on_irq_stack(overflow, desc, irq)) { + if (user_mode_vm(regs) || !execute_on_irq_stack(overflow, desc, irq)) { if (unlikely(overflow)) print_stack_overflow(); desc->handle_irq(irq, desc); From a09b659cd68c10ec6a30cb91ebd2c327fcd5bfe5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Mon, 5 Mar 2012 15:07:25 -0800 Subject: [PATCH 2/9] genirq: Fix long-term regression in genirq irq_set_irq_type() handling In 2008, commit 0c5d1eb77a8be ("genirq: record trigger type") modified the way set_irq_type() handles the 'no trigger' condition. However, this has an adverse effect on PCMCIA support on Intel StrongARM and probably PXA platforms. PCMCIA has several status signals on the socket which can trigger interrupts; some of these status signals depend on the card's mode (whether it is configured in memory or IO mode). For example, cards have a 'Ready/IRQ' signal: in memory mode, this provides an indication to PCMCIA that the card has finished its power up initialization. In IO mode, it provides the device interrupt signal. Other status signals switch between on-board battery status and loud speaker output. In classical PCMCIA implementations, where you have a specific socket controller, the controller provides a method to mask interrupts from the socket, and importantly ignore any state transitions on the pins which correspond with interrupts once masked. This masking prevents unwanted events caused by the removal and application of socket power being forwarded. However, on platforms where there is no socket controller, the PCMCIA status and interrupt signals are routed to standard edge-triggered GPIOs. These GPIOs can be configured to interrupt on rising edge, falling edge, or never. This is where the problems start. Edge triggered interrupts are required to record events while disabled via the usual methods of {free,request,disable,enable}_irq() to prevent problems with dropped interrupts (eg, the 8390 driver uses disable_irq() to defer the delivery of interrupts). As a result, these interfaces can not be used to implement the desired behaviour. The side effect of this is that if the 'Ready/IRQ' GPIO is disabled via disable_irq() on suspend, and enabled via enable_irq() after resume, we will record the state transitions caused by powering events as valid interrupts, and foward them to the card driver, which may attempt to access a card which is not powered up. This leads delays resume while drivers spin in their interrupt handlers, and complaints from drivers before they realize what's happened. Moreover, in the case of the 'Ready/IRQ' signal, this is requested and freed by the card driver itself; the PCMCIA core has no idea whether the interrupt is requested, and, therefore, whether a call to disable_irq() would be valid. (We tried this around 2.4.17 / 2.5.1 kernel era, and ended up throwing it out because of this problem.) Therefore, it was decided back in around 2002 to disable the edge triggering instead, resulting in all state transitions on the GPIO being ignored. That's what we actually need the hardware to do. The commit above changes this behaviour; it explicitly prevents the 'no trigger' state being selected. The reason that request_irq() does not accept the 'no trigger' state is for compatibility with existing drivers which do not provide their desired triggering configuration. The set_irq_type() function is 'new' and not used by non-trigger aware drivers. Therefore, revert this change, and restore previously working platforms back to their former state. Signed-off-by: Russell King Cc: linux@arm.linux.org.uk Cc: Ingo Molnar Cc: stable@vger.kernel.org Signed-off-by: Andrew Morton Signed-off-by: Thomas Gleixner --- kernel/irq/chip.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index f7c543a801d9..4dbc9c9d641b 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -61,8 +61,7 @@ int irq_set_irq_type(unsigned int irq, unsigned int type) return -EINVAL; type &= IRQ_TYPE_SENSE_MASK; - if (type != IRQ_TYPE_NONE) - ret = __irq_set_trigger(desc, irq, type); + ret = __irq_set_trigger(desc, irq, type); irq_put_desc_busunlock(desc, flags); return ret; } From b2a00178614e2cdd981a708d22a05c1ce4eadfd7 Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Mon, 5 Mar 2012 15:07:25 -0800 Subject: [PATCH 3/9] softirq: Reduce invoke_softirq() code duplication The two invoke_softirq() variants are identical except for a single line. So move the #ifdef __ARCH_IRQ_EXIT_IRQS_DISABLED inside one of the functions and get rid of the other one. Signed-off-by: Heiko Carstens Cc: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Thomas Gleixner --- kernel/softirq.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index 4eb3a0fa351e..c82d95a022ef 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -310,31 +310,21 @@ void irq_enter(void) __irq_enter(); } +static inline void invoke_softirq(void) +{ + if (!force_irqthreads) { #ifdef __ARCH_IRQ_EXIT_IRQS_DISABLED -static inline void invoke_softirq(void) -{ - if (!force_irqthreads) __do_softirq(); - else { - __local_bh_disable((unsigned long)__builtin_return_address(0), - SOFTIRQ_OFFSET); - wakeup_softirqd(); - __local_bh_enable(SOFTIRQ_OFFSET); - } -} #else -static inline void invoke_softirq(void) -{ - if (!force_irqthreads) do_softirq(); - else { +#endif + } else { __local_bh_disable((unsigned long)__builtin_return_address(0), SOFTIRQ_OFFSET); wakeup_softirqd(); __local_bh_enable(SOFTIRQ_OFFSET); } } -#endif /* * Exit an interrupt context. Process softirqs if needed and possible: From 540b60e24f3f4781d80e47122f0c4486a03375b8 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 9 Mar 2012 14:59:13 +0100 Subject: [PATCH 4/9] genirq: Fix incorrect check for forced IRQ thread handler We do not want a bitwise AND between boolean operands Signed-off-by: Alexander Gordeev Cc: Oleg Nesterov Link: http://lkml.kernel.org/r/20120309135912.GA2114@dhcp-26-207.brq.redhat.com Cc: stable@vger.kernel.org Signed-off-by: Thomas Gleixner --- kernel/irq/manage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index a9a9dbe49fea..c0730ad8a117 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -773,7 +773,7 @@ static int irq_thread(void *data) struct irqaction *action); int wake; - if (force_irqthreads & test_bit(IRQTF_FORCED_THREAD, + if (force_irqthreads && test_bit(IRQTF_FORCED_THREAD, &action->thread_flags)) handler_fn = irq_forced_thread_fn; else From 4bcdf1d0b652bc33d52f2322b77463e4dc58abf8 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 9 Mar 2012 14:59:26 +0100 Subject: [PATCH 5/9] genirq: Get rid of unnecessary irqaction field in task_struct When a new thread handler is created, an irqaction is passed to it as data. Not only that irqaction is stored in task_struct by the handler for later use, but also a structure associated with the kernel thread keeps this value as long as the thread exists. This fix kicks irqaction out off task_struct. Yes, I introduce new bit field. But it allows not only to eliminate the duplicate, but also shortens size of task_struct. Reported-by: Oleg Nesterov Signed-off-by: Alexander Gordeev Link: http://lkml.kernel.org/r/20120309135925.GB2114@dhcp-26-207.brq.redhat.com Signed-off-by: Thomas Gleixner --- include/linux/sched.h | 10 +++++----- kernel/irq/manage.c | 19 +++++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 7d379a6bfd88..07f537a371ce 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1319,6 +1319,11 @@ struct task_struct { unsigned sched_reset_on_fork:1; unsigned sched_contributes_to_load:1; +#ifdef CONFIG_GENERIC_HARDIRQS + /* IRQ handler threads */ + unsigned irq_thread:1; +#endif + pid_t pid; pid_t tgid; @@ -1427,11 +1432,6 @@ struct task_struct { * mempolicy */ spinlock_t alloc_lock; -#ifdef CONFIG_GENERIC_HARDIRQS - /* IRQ handler threads */ - struct irqaction *irqaction; -#endif - /* Protection of the PI data structures: */ raw_spinlock_t pi_lock; diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index c0730ad8a117..0fa3ce998ecb 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -780,7 +780,7 @@ static int irq_thread(void *data) handler_fn = irq_thread_fn; sched_setscheduler(current, SCHED_FIFO, ¶m); - current->irqaction = action; + current->irq_thread = 1; while (!irq_wait_for_interrupt(action)) { @@ -818,10 +818,10 @@ static int irq_thread(void *data) irq_finalize_oneshot(desc, action, true); /* - * Clear irqaction. Otherwise exit_irq_thread() would make + * Clear irq_thread. Otherwise exit_irq_thread() would make * fuzz about an active irq thread going into nirvana. */ - current->irqaction = NULL; + current->irq_thread = 0; return 0; } @@ -832,27 +832,30 @@ void exit_irq_thread(void) { struct task_struct *tsk = current; struct irq_desc *desc; + struct irqaction *action; - if (!tsk->irqaction) + if (!tsk->irq_thread) return; + action = kthread_data(tsk); + printk(KERN_ERR "exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n", - tsk->comm ? tsk->comm : "", tsk->pid, tsk->irqaction->irq); + tsk->comm ? tsk->comm : "", tsk->pid, action->irq); - desc = irq_to_desc(tsk->irqaction->irq); + desc = irq_to_desc(action->irq); /* * Prevent a stale desc->threads_oneshot. Must be called * before setting the IRQTF_DIED flag. */ - irq_finalize_oneshot(desc, tsk->irqaction, true); + irq_finalize_oneshot(desc, action, true); /* * Set the THREAD DIED flag to prevent further wakeups of the * soon to be gone threaded handler. */ - set_bit(IRQTF_DIED, &tsk->irqaction->flags); + set_bit(IRQTF_DIED, &action->flags); } static void irq_setup_forced_threading(struct irqaction *new) From 05d74efa3c72a5c40b0edeb15856c0230126313b Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 9 Mar 2012 14:59:40 +0100 Subject: [PATCH 6/9] genirq: No need to check IRQTF_DIED before stopping a thread handler Since 63706172f332fd3f6e7458ebfb35fa6de9c21dc5 kthread_stop() is not afraid of dead kernel threads. So no need to check if a thread is alive before stopping it. These checks still were racy. Reported-by: Oleg Nesterov Signed-off-by: Alexander Gordeev Link: http://lkml.kernel.org/r/20120309135939.GC2114@dhcp-26-207.brq.redhat.com Signed-off-by: Thomas Gleixner --- kernel/irq/manage.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 0fa3ce998ecb..3feab4ab6877 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1106,8 +1106,7 @@ out_thread: struct task_struct *t = new->thread; new->thread = NULL; - if (likely(!test_bit(IRQTF_DIED, &new->thread_flags))) - kthread_stop(t); + kthread_stop(t); put_task_struct(t); } out_mput: @@ -1217,8 +1216,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) #endif if (action->thread) { - if (!test_bit(IRQTF_DIED, &action->thread_flags)) - kthread_stop(action->thread); + kthread_stop(action->thread); put_task_struct(action->thread); } From 5234ffb9f74cfa8993d174782bc861dd9b7b5bfb Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 9 Mar 2012 14:59:59 +0100 Subject: [PATCH 7/9] genirq: Get rid of unnecessary IRQTF_DIED flag Currently IRQTF_DIED flag is set when a IRQ thread handler calls do_exit() But also PF_EXITING per process flag gets set when a thread exits. This fix eliminates the duplicate by using PF_EXITING flag. Also, there is a race condition in exit_irq_thread(). In case a thread's bit is cleared in desc->threads_oneshot (and the IRQ line gets unmasked), but before IRQTF_DIED flag is set, a new interrupt might come in and set just cleared bit again, this time forever. This fix throws IRQTF_DIED flag away, eliminating the race as a result. [ tglx: Test THREAD_EXITING first as suggested by Oleg ] Reported-by: Oleg Nesterov Signed-off-by: Alexander Gordeev Link: http://lkml.kernel.org/r/20120309135958.GD2114@dhcp-26-207.brq.redhat.com Signed-off-by: Thomas Gleixner --- kernel/exit.c | 4 ++-- kernel/irq/handle.c | 2 +- kernel/irq/internals.h | 2 -- kernel/irq/manage.c | 11 +---------- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 4b4042f9bc6a..752d2c0abd19 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -935,8 +935,6 @@ void do_exit(long code) schedule(); } - exit_irq_thread(); - exit_signals(tsk); /* sets PF_EXITING */ /* * tsk->flags are checked in the futex code to protect against @@ -945,6 +943,8 @@ void do_exit(long code) smp_mb(); raw_spin_unlock_wait(&tsk->pi_lock); + exit_irq_thread(); + if (unlikely(in_atomic())) printk(KERN_INFO "note: %s[%d] exited with preempt_count %d\n", current->comm, task_pid_nr(current), diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c index 470d08c82bbe..500aaf67c546 100644 --- a/kernel/irq/handle.c +++ b/kernel/irq/handle.c @@ -60,7 +60,7 @@ static void irq_wake_thread(struct irq_desc *desc, struct irqaction *action) * device interrupt, so no irq storm is lurking. If the * RUNTHREAD bit is already set, nothing to do. */ - if (test_bit(IRQTF_DIED, &action->thread_flags) || + if ((action->thread->flags & PF_EXITING) || test_and_set_bit(IRQTF_RUNTHREAD, &action->thread_flags)) return; diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index b7952316016a..5c233e0ea2c3 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -20,14 +20,12 @@ extern bool noirqdebug; /* * Bits used by threaded handlers: * IRQTF_RUNTHREAD - signals that the interrupt handler thread should run - * IRQTF_DIED - handler thread died * IRQTF_WARNED - warning "IRQ_WAKE_THREAD w/o thread_fn" has been printed * IRQTF_AFFINITY - irq thread is requested to adjust affinity * IRQTF_FORCED_THREAD - irq action is force threaded */ enum { IRQTF_RUNTHREAD, - IRQTF_DIED, IRQTF_WARNED, IRQTF_AFFINITY, IRQTF_FORCED_THREAD, diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 3feab4ab6877..a94466db73b2 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -845,17 +845,8 @@ void exit_irq_thread(void) desc = irq_to_desc(action->irq); - /* - * Prevent a stale desc->threads_oneshot. Must be called - * before setting the IRQTF_DIED flag. - */ + /* Prevent a stale desc->threads_oneshot */ irq_finalize_oneshot(desc, action, true); - - /* - * Set the THREAD DIED flag to prevent further wakeups of the - * soon to be gone threaded handler. - */ - set_bit(IRQTF_DIED, &action->flags); } static void irq_setup_forced_threading(struct irqaction *new) From 7140ea1980f2fae9c7aaeac5f6b35317e1389ee6 Mon Sep 17 00:00:00 2001 From: Ido Yariv Date: Fri, 2 Dec 2011 18:24:12 +0200 Subject: [PATCH 8/9] 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 Link: http://lkml.kernel.org/r/1322843052-7166-1-git-send-email-ido@wizery.com Signed-off-by: Thomas Gleixner --- kernel/irq/handle.c | 12 +++++++++ kernel/irq/manage.c | 60 ++++++++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c index 500aaf67c546..6ff84e6a954c 100644 --- 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 1786cf7dac54..453feedbb390 100644 --- 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); } From e04268b0effc0ceea366c50b3107baad9edadafa Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Thu, 15 Mar 2012 22:55:21 +0100 Subject: [PATCH 9/9] genirq: Remove paranoid warnons and bogus fixups Alexander pointed out that the warnons in the regular exit path are bogus and the thread_mask one actually could be triggered when __setup_irq() hands out that thread_mask again after __free_irq() dropped irq_desc->lock. Thinking more about it, neither IRQTF_RUNTHREAD nor the bit in thread_mask can be set as this is the regular exit path. We come here due to: __free_irq() remove action from desc synchronize_irq() kthread_stop() So synchronize_irq() makes sure that the thread finished running and cleaned up both the thread_active count and thread_mask. After that point nothing can set IRQTF_RUNTHREAD on this action. So the warnons and the cleanups are pointless. Reported-by: Alexander Gordeev Cc: Ido Yariv Link: http://lkml.kernel.org/r/20120315190755.GA6732@dhcp-26-207.brq.redhat.com Signed-off-by: Thomas Gleixner --- kernel/irq/manage.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 453feedbb390..b0ccd1ac2d6a 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -804,17 +804,11 @@ static int irq_thread(void *data) * 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. + * oneshot mask bit can be set. We cannot verify that as we + * cannot touch the oneshot mask at this point anymore as + * __setup_irq() might have given out currents thread_mask + * again. * - * 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 * fuzz about an active irq thread going into nirvana. */