Nicholas Piggin <npig...@gmail.com> writes: > On Wed, 25 Apr 2018 13:15:34 +1000 > Michael Ellerman <m...@ellerman.id.au> wrote: >> Nicholas Piggin <npig...@gmail.com> writes: >> >> > 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 smp_send_stop 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, 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 smp_send_stop handler to >> > decrement the busy count, because it will never return. >> > >> > Fixes: 6bed3237624e3 ("powerpc: use NMI IPI for smp_send_stop") >> > Reported-by: Abdul Haleem <abdha...@linux.vnet.ibm.com> >> > Signed-off-by: Nicholas Piggin <npig...@gmail.com> >> > --- >> > arch/powerpc/kernel/smp.c | 11 ++++++++++- >> > 1 file changed, 10 insertions(+), 1 deletion(-) >> > >> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c >> > index e16ec7b3b427..250fccf04c6e 100644 >> > --- a/arch/powerpc/kernel/smp.c >> > +++ b/arch/powerpc/kernel/smp.c >> > @@ -567,10 +567,19 @@ void crash_send_ipi(void >> > (*crash_ipi_callback)(struct pt_regs *)) >> > >> > #ifdef CONFIG_NMI_IPI >> > static void 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(); >> > #else >> > static void stop_this_cpu(void *dummy) >> > -#endif >> > { >> > +#endif >> >> I don't love this ifdef/endif business. >> >> Can we do it this way instead? > > Yeah that's better. Does stop_this_cpu give you an unused function warning > if you compile with NMI though? I think we need an #if/#else
No because it's called from nmi_stop_this_cpu(): >> +#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 cheers