From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752584Ab1JLH3S (ORCPT ); Wed, 12 Oct 2011 03:29:18 -0400 Received: from mx3.mail.elte.hu ([157.181.1.138]:54742 "EHLO mx3.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751016Ab1JLH3Q (ORCPT ); Wed, 12 Oct 2011 03:29:16 -0400 Date: Wed, 12 Oct 2011 09:27:18 +0200 From: Ingo Molnar To: Don Zickus Cc: Andi Kleen , x86@kernel.org, LKML , Peter Zijlstra , Robert Richter , Andrew Morton , seiji.aguchi@hds.com, vgoyal@redhat.com, mjg@redhat.com, tony.luck@intel.com, gong.chen@intel.com, satoru.moriya@hds.com, avi@redhat.com Subject: Re: [PATCH 2/3] x86, NMI: Add NMI IPI selftest Message-ID: <20111012072718.GG18618@elte.hu> References: <1318346686-12349-1-git-send-email-dzickus@redhat.com> <1318346686-12349-3-git-send-email-dzickus@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1318346686-12349-3-git-send-email-dzickus@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-ELTE-SpamScore: -2.0 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-2.0 required=5.9 tests=AWL,BAYES_00 autolearn=no SpamAssassin version=3.3.1 -2.0 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.0 AWL AWL: From: address is in the auto white-list Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Don Zickus wrote: > The previous patch modified the stop cpus path to use NMI instead of IRQ > as the way to communicate to the other cpus to shutdown. There were some > concerns that various machines may have problems with using an NMI IPI. > > This patch creates a selftest to check if NMI IPI is working at boot. > The idea is to help catch any issues before the machine panics and we > learn the hard way. :-) > > If the selftest fails, the code tries to fall back to the original IRQ > method. > > Signed-off-by: Don Zickus > --- > arch/x86/Kconfig.debug | 12 +++++++++ > arch/x86/include/asm/smp.h | 1 + > arch/x86/kernel/smp.c | 5 ++++ > arch/x86/kernel/smpboot.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 75 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug > index c0f8a5c..18261c2 100644 > --- a/arch/x86/Kconfig.debug > +++ b/arch/x86/Kconfig.debug > @@ -284,4 +284,16 @@ config DEBUG_STRICT_USER_COPY_CHECKS > > If unsure, or if you run an older (pre 4.4) gcc, say N. > > +config DEBUG_NMI_IPI_SELFTEST > + bool "NMI IPI Selftest" > + depends on DEBUG_KERNEL > + ---help--- > + Enabling this option turns on a quick NMI IPI selftest to > + verify that sending an NMI this way works for the panic and > + kdump paths. > + > + This might help diagnose strange hangs in those paths. > + > + If unsure, say N. > + > endmenu > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h > index 73b11bc..916f89d 100644 > --- a/arch/x86/include/asm/smp.h > +++ b/arch/x86/include/asm/smp.h > @@ -149,6 +149,7 @@ static inline void arch_send_call_function_ipi_mask(const struct cpumask *mask) > } > > void cpu_disable_common(void); > +void native_smp_disable_nmi_ipi(void); > void native_smp_prepare_boot_cpu(void); > void native_smp_prepare_cpus(unsigned int max_cpus); > void native_smp_cpus_done(unsigned int max_cpus); > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c > index 7bdbf6a..f54e07a 100644 > --- a/arch/x86/kernel/smp.c > +++ b/arch/x86/kernel/smp.c > @@ -247,6 +247,11 @@ static void native_irq_stop_other_cpus(int wait) > local_irq_restore(flags); > } > > +void native_smp_disable_nmi_ipi(void) > +{ > + smp_ops.stop_other_cpus = native_irq_stop_other_cpus; > +} > + > /* > * Reschedule call back. > */ > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index 9f548cb..dcb889f 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -71,6 +71,7 @@ > > #include > #include > +#include > > /* State of each CPU */ > DEFINE_PER_CPU(int, cpu_state) = { 0 }; > @@ -415,6 +416,57 @@ const struct cpumask *cpu_coregroup_mask(int cpu) > return cpu_llc_shared_mask(cpu); > } > > +#ifdef CONFIG_DEBUG_NMI_IPI_SELFTEST > +/* check to see if NMI IPIs work on this machine */ > +static DECLARE_BITMAP(nmi_ipi_mask, NR_CPUS) __read_mostly; > + > +static int check_nmi_ipi_callback(unsigned int val, struct pt_regs *regs) > +{ > + int cpu = raw_smp_processor_id(); > + > + if (cpumask_test_cpu(cpu, to_cpumask(nmi_ipi_mask))) { > + cpumask_clear_cpu(cpu, to_cpumask(nmi_ipi_mask)); > + return NMI_HANDLED; > + } > + > + return NMI_DONE; > +} > + > +static bool check_nmi_ipi(void) > +{ > + unsigned long timeout; > + > + if (num_online_cpus() > 1) { > + cpumask_copy(to_cpumask(nmi_ipi_mask), cpu_online_mask); > + cpumask_clear_cpu(smp_processor_id(), to_cpumask(nmi_ipi_mask)); > + > + if (register_nmi_handler(NMI_LOCAL, check_nmi_ipi_callback, > + NMI_FLAG_FIRST, "check_nmi_ipi")) > + return false; > + > + /* sync above data before sending NMI */ > + wmb(); > + > + apic->send_IPI_allbutself(NMI_VECTOR); > + > + /* Don't wait longer than a second */ > + timeout = USEC_PER_SEC; > + while (!cpumask_empty(to_cpumask(nmi_ipi_mask)) && timeout--) > + udelay(1); > + > + /* What happens if we timeout, do we still unregister?? */ > + unregister_nmi_handler(NMI_LOCAL, "check_nmi_ipi"); > + > + if (!timeout) { > + pr_warn("CPU does not support NMI IPIs\n"); > + return false; > + } > + } > + pr_info("NMI IPIs selftest passed\n"); > + return true; > +} > +#endif > + > static void impress_friends(void) > { > int cpu; > @@ -1142,6 +1194,11 @@ void __init native_smp_cpus_done(unsigned int max_cpus) > { > pr_debug("Boot done.\n"); > > +#ifdef CONFIG_DEBUG_NMI_IPI_SELFTEST > + /* if NMI IPI doesn't work, fallback to old irq method for panic */ > + if (check_nmi_ipi()) > + native_smp_disable_nmi_ipi(); > +#endif Looks good in principle, but instead of the #ifdeffery i'd really suggest to create a separate (small) .c module and .h header file for this, to hide all those self-test details from the generic path. The callback in native_smp_cpus_done() callback can be a plain nmi_selftest_callback() call or such. Thanks, Ingo