[PATCH v3] powerpc: Fix smp_send_stop NMI IPI handling

[PATCH v3] powerpc: Fix smp_send_stop NMI IPI handling

[Michael Ellerman]

From: Nicholas Piggin <npiggin@gmail.com>

The NMI IPI handler for a receiving CPU increments nmi_ipi_busy_count
over the handler function call, which causes later smp_send_nmi_ipi()
callers to spin until the call is finished.

The stop_this_cpu() function never returns, so the busy count is never
decremeted, which can cause the system to hang in some cases. For
example panic() will call smp_send_stop() early on which calls
stop_this_cpu() on other CPUs, then later in the reboot path,
pnv_restart() will call smp_send_stop() again, which hangs.

Fix this by adding a special case to the stop_this_cpu() handler to
decrement the busy count, because it will never return.

Now that the NMI/non-NMI versions of stop_this_cpu() are different,
split them out into separate functions rather than doing #ifdef tricks
to share the body between the two functions.

Fixes: 6bed3237624e3 ("powerpc: use NMI IPI for smp_send_stop")
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[mpe: Split out the functions, tweak change log a bit]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/smp.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

v3: Split into separate functions, rather than doing #ifdef tricks.

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index e16ec7b3b427..3582f30b60b7 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -565,11 +565,7 @@ void crash_send_ipi(void (*crash_ipi_callback)(struct pt_regs *))
 }
 #endif
 
-#ifdef CONFIG_NMI_IPI
-static void stop_this_cpu(struct pt_regs *regs)
-#else
 static void stop_this_cpu(void *dummy)
-#endif
 {
 	/* Remove this CPU */
 	set_cpu_online(smp_processor_id(), false);
@@ -580,10 +576,26 @@ static void stop_this_cpu(void *dummy)
 		spin_cpu_relax();
 }
 
+#ifdef CONFIG_NMI_IPI
+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.
+	 */
+	nmi_ipi_lock();
+	nmi_ipi_busy_count--;
+	nmi_ipi_unlock();
+
+	stop_this_cpu(NULL);
+}
+#endif
+
 void smp_send_stop(void)
 {
 #ifdef CONFIG_NMI_IPI
-	smp_send_nmi_ipi(NMI_IPI_ALL_OTHERS, stop_this_cpu, 1000000);
+	smp_send_nmi_ipi(NMI_IPI_ALL_OTHERS, nmi_stop_this_cpu, 1000000);
 #else
 	smp_call_function(stop_this_cpu, NULL, 0);
 #endif
-- 
2.14.1

Re: [v3] powerpc: Fix smp_send_stop NMI IPI handling

[Michael Ellerman]

On Thu, 2018-04-26 at 10:19:02 UTC, Michael Ellerman wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
> 
> The NMI IPI handler for a receiving CPU increments nmi_ipi_busy_count
> over the handler function call, which causes later smp_send_nmi_ipi()
> callers to spin until the call is finished.
> 
> The stop_this_cpu() function never returns, so the busy count is never
> decremeted, which can cause the system to hang in some cases. For
> example panic() will call smp_send_stop() early on which calls
> stop_this_cpu() on other CPUs, then later in the reboot path,
> pnv_restart() will call smp_send_stop() again, which hangs.
> 
> Fix this by adding a special case to the stop_this_cpu() handler to
> decrement the busy count, because it will never return.
> 
> Now that the NMI/non-NMI versions of stop_this_cpu() are different,
> split them out into separate functions rather than doing #ifdef tricks
> to share the body between the two functions.
> 
> Fixes: 6bed3237624e3 ("powerpc: use NMI IPI for smp_send_stop")
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> [mpe: Split out the functions, tweak change log a bit]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Applied to powerpc fixes.

https://git.kernel.org/powerpc/c/ac61c1156623455c46701654abd8c9

cheers