handle_nested_irq() is supposed to be running inside the parent thread handler context. It per se has no dedicated kernel thread, thus shouldn't touch desc->threads_active. The parent kernel thread has already taken care of this.
Fixes: e2c12739ccf7 ("genirq: Prevent nested thread vs synchronize_hardirq() deadlock") Cc: stable@vger.kernel.org Signed-off-by: Peng Liu iwtbavbm@gmail.com ---
Despite of its correctness, I'm afraid the testing on my only PC can't cover the affected code path. So the patch may be totally -UNTESTED-.
kernel/irq/chip.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index dc94e0bf2c94..85d4f29134e9 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -478,7 +478,6 @@ void handle_nested_irq(unsigned int irq) }
kstat_incr_irqs_this_cpu(desc); - atomic_inc(&desc->threads_active); raw_spin_unlock_irq(&desc->lock);
action_ret = IRQ_NONE; @@ -487,8 +486,6 @@ void handle_nested_irq(unsigned int irq)
if (!irq_settings_no_debug(desc)) note_interrupt(desc, action_ret); - - wake_threads_waitq(desc); } EXPORT_SYMBOL_GPL(handle_nested_irq);
On Mon, Jun 10 2024 at 02:30, Peng Liu wrote:
handle_nested_irq() is supposed to be running inside the parent thread handler context. It per se has no dedicated kernel thread, thus shouldn't touch desc->threads_active. The parent kernel thread has already taken care of this.
No it has not. The parent thread has marked itself in the parent threads interrupt descriptor.
How does that help synchronizing the nested interrupt, which has a separate interrupt descriptor?
Fixes: e2c12739ccf7 ("genirq: Prevent nested thread vs synchronize_hardirq() deadlock") Cc: stable@vger.kernel.org
There is nothing to fix.
Signed-off-by: Peng Liu iwtbavbm@gmail.com
Despite of its correctness, I'm afraid the testing on my only PC can't cover the affected code path. So the patch may be totally -UNTESTED-.
Which correctness?
The change log of the commit you want to "fix" says:
Remove the incorrect usage in the nested threaded interrupt case and instead re-use the threads_active / wait_for_threads mechanism to wait for nested threaded interrupts to complete.
It's very clearly spelled out, no?
Thanks,
tglx
On Mon, Jun 10, 2024 at 08:23:09PM +0200, Thomas Gleixner wrote:
On Mon, Jun 10 2024 at 02:30, Peng Liu wrote:
handle_nested_irq() is supposed to be running inside the parent thread handler context. It per se has no dedicated kernel thread, thus shouldn't touch desc->threads_active. The parent kernel thread has already taken care of this.
No it has not. The parent thread has marked itself in the parent threads interrupt descriptor.
How does that help synchronizing the nested interrupt, which has a separate interrupt descriptor?
Right, I never thought there would be more than one interrupt descriptors involved which is quite common.
Fixes: e2c12739ccf7 ("genirq: Prevent nested thread vs synchronize_hardirq() deadlock") Cc: stable@vger.kernel.org
There is nothing to fix.
Signed-off-by: Peng Liu iwtbavbm@gmail.com
Despite of its correctness, I'm afraid the testing on my only PC can't cover the affected code path. So the patch may be totally -UNTESTED-.
Which correctness?
The change log of the commit you want to "fix" says:
Remove the incorrect usage in the nested threaded interrupt case and instead re-use the threads_active / wait_for_threads mechanism to wait for nested threaded interrupts to complete.It's very clearly spelled out, no?
Indeed, due to my ignorance, I never thought there might be more descriptors involved. Now think about it, I never really understood the meaning of the above change log.
Thanks for your time and concise explanation.
Peng
Thanks,
tglx
linux-stable-mirror@lists.linaro.org
-
Peng Liu -
Thomas Gleixner