From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-01-20 07:57:01
Running softirqs enables interrupts, which can then end up recursing
into the irq soft-mask code we're adjusting, including replaying
interrupts itself, which might be theoretically unbounded.
This abridged trace shows how this can occur:
! NIP replay_soft_interrupts
LR interrupt_exit_kernel_prepare
Call Trace:
interrupt_exit_kernel_prepare (unreliable)
interrupt_return
--- interrupt: ea0 at __rb_reserve_next
NIP __rb_reserve_next
LR __rb_reserve_next
Call Trace:
ring_buffer_lock_reserve
trace_function
function_trace_call
ftrace_call
__do_softirq
irq_exit
timer_interrupt
! replay_soft_interrupts
interrupt_exit_kernel_prepare
interrupt_return
--- interrupt: ea0 at arch_local_irq_restore
Fix this by disabling bhs (softirqs) around the interrupt replay.
I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
interrupt replay in C") actually introduced the problem. Prior to that
change, the interrupt replay seems like it should still be subect to
this recusion, however it's done after all the irq state has been fixed
up at the end of the replay, so it seems reasonable to fix back to this
commit.
Fixes: 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in C")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/irq.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
<html><head></head><body dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="ApplePlainTextBody"><div class="ApplePlainTextBody"><br><br><blockquote type="cite">On 20-Jan-2021, at 1:20 PM, Nicholas Piggin <npiggin@gmail.com> wrote:<br><br>Running softirqs enables interrupts, which can then end up recursing<br>into the irq soft-mask code we're adjusting, including replaying<br>interrupts itself, which might be theoretically unbounded.<br><br>This abridged trace shows how this can occur:<br><br>! NIP replay_soft_interrupts<br> LR interrupt_exit_kernel_prepare<br> Call Trace:<br> interrupt_exit_kernel_prepare (unreliable)<br> interrupt_return<br> --- interrupt: ea0 at __rb_reserve_next<br> NIP __rb_reserve_next<br> LR __rb_reserve_next<br> Call Trace:<br> ring_buffer_lock_reserve<br> trace_function<br> function_trace_call<br> ftrace_call<br> __do_softirq<br> irq_exit<br> timer_interrupt<br>! replay_soft_interrupts<br> interrupt_exit_kernel_prepare<br> interrupt_return<br> --- interrupt: ea0 at arch_local_irq_restore<br><br>Fix this by disabling bhs (softirqs) around the interrupt replay.<br><br>I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft<br>interrupt replay in C") actually introduced the problem. Prior to that<br>change, the interrupt replay seems like it should still be subect to<br>this recusion, however it's done after all the irq state has been fixed<br>up at the end of the replay, so it seems reasonable to fix back to this<br>commit.<br><br>Fixes: 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in C")<br>Signed-off-by: Nicholas Piggin <npiggin@gmail.com><br></blockquote><br>Thanks for the fix Nick.<br><br>Tested this below scenario where previously it was resulting in soft lockup’s with the trace described in the commit message. <br>With the patch, I don’t see soft lockup’s.<br><br>Test scenario: My test kernel module below tries to create one of performance monitor<br>counter ( PMC6 ) overflow between local_irq_save/local_irq_restore. I am also configuring ftrace.<br><br>Environment :One CPU online and Bare Metal system<br>prerequisite for ftrace:<br># cd /sys/kernel/debug/tracing<br># echo 100 > buffer_percent<br># echo 200000 > buffer_size_kb <br># echo ppc-tb > trace_clock<br># echo function > current_tracer<br><br>Part of sample kernel test module to trigger a PMI between <br>local_irq_save and local_irq_restore:<br><br><<>><br>static ulong delay = 1;<br>static void busy_wait(ulong time)<br>{<br> udelay(delay);<br>}<br>static __always_inline void irq_test(void)<br>{<br> unsigned long flags;<br> local_irq_save(flags);<br> trace_printk("IN IRQ TEST\n");<br> mtspr(SPRN_MMCR0, 0x80000000);<br> mtspr(SPRN_PMC6, 0x80000000 - 100);<br> mtspr(SPRN_MMCR0, 0x6004000);<br> busy_wait(delay);<br> trace_printk("IN IRQ TEST DONE\n");<br> local_irq_restore(flags);<br> mtspr(SPRN_MMCR0, 0x80000000);<br> mtspr(SPRN_PMC6, 0);<br>}<br><<>><br><br>With the patch, there is no soft lockup’s.<br><br>Tested-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com><br><br><blockquote type="cite">---<br>arch/powerpc/kernel/irq.c | 14 ++++++++++++++<br>1 file changed, 14 insertions(+)<br><br>diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c<br>index 6b1eca53e36c..7064135f9dc3 100644<br>--- a/arch/powerpc/kernel/irq.c<br>+++ b/arch/powerpc/kernel/irq.c<br>@@ -188,6 +188,18 @@ void replay_soft_interrupts(void)<br><span class="Apple-tab-span" style="white-space:pre"> </span>unsigned char happened = local_paca->irq_happened;<br><span class="Apple-tab-span" style="white-space:pre"> </span>struct pt_regs regs;<br><br>+<span class="Apple-tab-span" style="white-space:pre"> </span>/*<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * Prevent softirqs from being run when an interrupt handler returns<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * and calls irq_exit(), because softirq processing enables interrupts.<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * If an interrupt is taken, it may then call replay_soft_interrupts<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * on its way out, which gets messy and recursive.<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> *<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * softirqs created by replayed interrupts will be run at the end of<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * this function when bhs are enabled (if they were enabled in our<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * caller).<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>local_bh_disable();<br>+<br><span class="Apple-tab-span" style="white-space:pre"> </span>ppc_save_regs(&regs);<br><span class="Apple-tab-span" style="white-space:pre"> </span>regs.softe = IRQS_ENABLED;<br><br>@@ -263,6 +275,8 @@ void replay_soft_interrupts(void)<br><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>trace_hardirqs_off();<br><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto again;<br><span class="Apple-tab-span" style="white-space:pre"> </span>}<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>local_bh_enable();<br>}<br><br>notrace void arch_local_irq_restore(unsigned long mask)<br>-- <br>2.23.0<br><br></blockquote><br></div></body></html>
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-01-21 13:03:36
Nicholas Piggin [off-list ref] writes:
Running softirqs enables interrupts, which can then end up recursing
into the irq soft-mask code we're adjusting, including replaying
interrupts itself, which might be theoretically unbounded.
This abridged trace shows how this can occur:
! NIP replay_soft_interrupts
LR interrupt_exit_kernel_prepare
Call Trace:
interrupt_exit_kernel_prepare (unreliable)
interrupt_return
--- interrupt: ea0 at __rb_reserve_next
NIP __rb_reserve_next
LR __rb_reserve_next
Call Trace:
ring_buffer_lock_reserve
trace_function
function_trace_call
ftrace_call
__do_softirq
irq_exit
timer_interrupt
! replay_soft_interrupts
interrupt_exit_kernel_prepare
interrupt_return
--- interrupt: ea0 at arch_local_irq_restore
Fix this by disabling bhs (softirqs) around the interrupt replay.
I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
interrupt replay in C") actually introduced the problem. Prior to that
change, the interrupt replay seems like it should still be subect to
this recusion, however it's done after all the irq state has been fixed
up at the end of the replay, so it seems reasonable to fix back to this
commit.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-01-22 00:34:58
Excerpts from Michael Ellerman's message of January 21, 2021 10:50 pm:
Nicholas Piggin [off-list ref] writes:
quoted
Running softirqs enables interrupts, which can then end up recursing
into the irq soft-mask code we're adjusting, including replaying
interrupts itself, which might be theoretically unbounded.
This abridged trace shows how this can occur:
! NIP replay_soft_interrupts
LR interrupt_exit_kernel_prepare
Call Trace:
interrupt_exit_kernel_prepare (unreliable)
interrupt_return
--- interrupt: ea0 at __rb_reserve_next
NIP __rb_reserve_next
LR __rb_reserve_next
Call Trace:
ring_buffer_lock_reserve
trace_function
function_trace_call
ftrace_call
__do_softirq
irq_exit
timer_interrupt
! replay_soft_interrupts
interrupt_exit_kernel_prepare
interrupt_return
--- interrupt: ea0 at arch_local_irq_restore
Fix this by disabling bhs (softirqs) around the interrupt replay.
I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
interrupt replay in C") actually introduced the problem. Prior to that
change, the interrupt replay seems like it should still be subect to
this recusion, however it's done after all the irq state has been fixed
up at the end of the replay, so it seems reasonable to fix back to this
commit.
This seems very unhappy for me (on P9 bare metal):
Oh, damn lockdep I always forget it has a bunch of interrupt checks.
In this case it might have a point though. Thanks, I'll try to fix it.
Thanks,
Nick