On 20-Jan-2021, at 1:20 PM, Nicholas Piggin <npig...@gmail.com> wrote:
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 <npig...@gmail.com>
Thanks for the fix Nick.
Tested this below scenario where previously it was resulting in soft lockup’s with the trace described in the commit message.
With the patch, I don’t see soft lockup’s.
Test scenario: My test kernel module below tries to create one of performance monitor
counter ( PMC6 ) overflow between local_irq_save/local_irq_restore. I am also configuring ftrace.
Environment :One CPU online and Bare Metal system
prerequisite for ftrace:
# cd /sys/kernel/debug/tracing
# echo 100 > buffer_percent
# echo 200000 > buffer_size_kb
# echo ppc-tb > trace_clock
# echo function > current_tracer
Part of sample kernel test module to trigger a PMI between
local_irq_save and local_irq_restore:
<<>>
static ulong delay = 1;
static void busy_wait(ulong time)
{
udelay(delay);
}
static __always_inline void irq_test(void)
{
unsigned long flags;
local_irq_save(flags);
trace_printk("IN IRQ TEST\n");
mtspr(SPRN_MMCR0, 0x80000000);
mtspr(SPRN_PMC6, 0x80000000 - 100);
mtspr(SPRN_MMCR0, 0x6004000);
busy_wait(delay);
trace_printk("IN IRQ TEST DONE\n");
local_irq_restore(flags);
mtspr(SPRN_MMCR0, 0x80000000);
mtspr(SPRN_PMC6, 0);
}
<<>>
With the patch, there is no soft lockup’s.
Tested-by: Athira Rajeev <atraj...@linux.vnet.ibm.com>
---
arch/powerpc/kernel/irq.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 6b1eca53e36c..7064135f9dc3 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -188,6 +188,18 @@ void replay_soft_interrupts(void)
unsigned char happened = local_paca->irq_happened;
struct pt_regs regs;
+ /*
+ * Prevent softirqs from being run when an interrupt handler returns
+ * and calls irq_exit(), because softirq processing enables interrupts.
+ * If an interrupt is taken, it may then call replay_soft_interrupts
+ * on its way out, which gets messy and recursive.
+ *
+ * softirqs created by replayed interrupts will be run at the end of
+ * this function when bhs are enabled (if they were enabled in our
+ * caller).
+ */
+ local_bh_disable();
+
ppc_save_regs(®s);
regs.softe = IRQS_ENABLED;
@@ -263,6 +275,8 @@ void replay_soft_interrupts(void)
trace_hardirqs_off();
goto again;
}
+
+ local_bh_enable();
}
notrace void arch_local_irq_restore(unsigned long mask)
--
2.23.0
