powerpc: Fix deadlock with multiple calls to smp_send_stop
Nicholas Piggin
reviewerPATCH v2 1/1 powerpc: Fix deadlock with multiple calls to smp_send_stop
smp_send_stop can lock up the IPI path for any subsequent calls, because the receiving CPUs spin in their handler function. This started becoming a problem with the addition of an smp_send_stop call in the reboot path, because panics can reboot after doing their own smp_send_stop.
The NMI IPI variant was fixed with ac61c11566 ("powerpc: Fix smp_send_stop NMI IPI handling"), which leaves the smp_call_function variant.
This is fixed by having smp_send_stop only ever do the smp_call_function once. This is a bit less robust than the NMI IPI fix, because any other call to smp_call_function after smp_send_stop could deadlock, but that has always been the case, and it was not been a problem before.
---
Changes since v1:
- Rebased to powerpc fixes branch which has the NMI IPI fix.
arch/powerpc/kernel/smp.c | 55 +++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 16 deletions(-)
arch/powerpc/kernel/smp.c +39 -16
@@ -565,17 +565,6 @@ void crash_send_ipi(void (*crash_ipi_callback)(struct pt_regs *)) }#endif -static void stop_this_cpu (void *dummy) -{ -/* Remove this CPU */ -set_cpu_online (smp_processor_id (), false); - -hard_irq_disable (); -spin_begin (); -while (1 ) -spin_cpu_relax (); -} -#ifdef CONFIG_NMI_IPI static void nmi_stop_this_cpu (struct pt_regs *regs) {
@@ -583,23 +572,57 @@ static void nmi_stop_this_cpu(struct pt_regs *regs)* This is a special case because it never returns, so the NMI IPI * handling would never mark it as done, which makes any later * smp_send_nmi_ipi() call spin forever. Mark it done now. +* +* IRQs are already hard disabled by the smp_handle_nmi_ipi. */ nmi_ipi_lock (); nmi_ipi_busy_count--;nmi_ipi_unlock (); - stop_this_cpu(NULL); +/* Remove this CPU */ +set_cpu_online (smp_processor_id (), false); + +spin_begin (); +while (1 ) +spin_cpu_relax (); } -#endif void smp_send_stop (void ) { -#ifdef CONFIG_NMI_IPI smp_send_nmi_ipi (NMI_IPI_ALL_OTHERS , nmi_stop_this_cpu,1000000 ); -#else +} + +#else /* CONFIG_NMI_IPI */ + +static void stop_this_cpu (void *dummy) +{ +/* Remove this CPU */ +set_cpu_online (smp_processor_id (), false); + +hard_irq_disable (); +spin_begin (); +while (1 ) +spin_cpu_relax (); +} + +void smp_send_stop (void ) +{ +static bool stopped = false; + +/* +* Prevent waiting on csd lock from a previous smp_send_stop. +* This is racy, but in general callers try to do the right +* thing and only fire off one smp_send_stop (e.g., see +* kernel/panic.c) +*/ +if (stopped) +return ; + + stopped = true; +smp_call_function (stop_this_cpu,NULL ,0 ); -#endif } +#endif /* CONFIG_NMI_IPI */ struct thread_info *current_set[NR_CPUS ];
Signature · 1 line
2.17.0
Michael Ellerman
AppliedQuoted 16 lines · Nicholas Piggin
smp_send_stopcan lock up the IPI path for any subsequent calls, because the receiving CPUs spin in their handler function. This started becoming a problem with the addition of ansmp_send_stopcall in the reboot path, because panics can reboot after doing their ownsmp_send_stop.The NMI IPI variant was fixed with
ac61c11566("powerpc: Fixsmp_send_stopNMI IPI handling"), which leaves thesmp_call_functionvariant.This is fixed by having
smp_send_stoponly ever do thesmp_call_functiononce. This is a bit less robust than the NMI IPI fix, because any other call tosmp_call_functionaftersmp_send_stopcould deadlock, but that has always been the case, and it was not been a problem before.
Quoted 4 lines
Cc Michael Ellerman <[email protected]>Fixes f2748bdfe1573 ("powerpc/powernv: Always stop secondaries before reboot/shutdown")Reported-by Abdul Haleem <[email protected]>Signed-off-by Nicholas Piggin <[email protected]>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/6029755eed95e5c90f763188c87ae3
cheers