[PATCH 1/4 v2] nmi: Provide the option to issue an NMI back trace to every cpu but current

[PATCH 1/4 v2] nmi: Provide the option to issue an NMI back trace to every cpu but current

[Oleg Nesterov]

On 04/21, Don Zickus wrote:
>
> On Tue, Apr 15, 2014 at 07:26:49PM +0200, Oleg Nesterov wrote:
> > On 04/15, Oleg Nesterov wrote:
> > >
> > > Looking at https://lkml.org/lkml/2014/4/4/469... It seems that 2/4 can be
> > > simplified, you can simply remove smp_processor_id() from backtrace_mask
> > > if !include_self and use apic->send_IPI_mask(backtrace_mask). But this is
> > > minor, I won't insist.
> >
> > And in fact, I do not understand why arch_trigger_all_cpu_backtrace() doesn't
> > disable preemption. OK, probably we can simply ignore the race with cpu hotplug.
> >
> > But it seems that your patch makes the things worse. Lets look at, say,
> > numachip_send_IPI_mask_allbutself(). The usage of smp_processor_id() is
> > obviously racy but perhaps we do not care again. But we do not want a warning
> > from debug_smp_processor_id().
>
> Good point.  I forgot that going from all cpus down to allbutself,
> preemption now matters.

I am not sure it actually matters wrt "show other CPU's traces". If the preemption
is possible then the caller can be preempted even before it sends ipi.

OTOH I think it does matter anyway, even without your patch, otherwise the usage
of cpu_online_mask is racy and we can hit the "Wait for up to 10 seconds" case.

Btw...

	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
	for (i = 0; i < 10 * 1000; i++) {
		if (cpumask_empty(to_cpumask(backtrace_mask)))
			break;
		mdelay(1);
	}

OK, but perhaps we should clear backtrace_mask if we return due to timeout.

> does disabling preemption help in the cpu
> hotplug case?

Yes. But I'd suggest to change your patch to use get_cpu() instead of
preempt_disable/smp_processor_id.

And I think it would be better to not discuss this off-list, I added lkml.

Oleg.

Re: [PATCH 1/4 v2] nmi: Provide the option to issue an NMI back trace to every cpu but current

[Don Zickus]

On Mon, Apr 21, 2014 at 03:41:54PM +0200, Oleg Nesterov wrote:
> On 04/21, Don Zickus wrote:
> >
> > On Tue, Apr 15, 2014 at 07:26:49PM +0200, Oleg Nesterov wrote:
> > > On 04/15, Oleg Nesterov wrote:
> > > >
> > > > Looking at https://lkml.org/lkml/2014/4/4/469... It seems that 2/4 can be
> > > > simplified, you can simply remove smp_processor_id() from backtrace_mask
> > > > if !include_self and use apic->send_IPI_mask(backtrace_mask). But this is
> > > > minor, I won't insist.
> > >
> > > And in fact, I do not understand why arch_trigger_all_cpu_backtrace() doesn't
> > > disable preemption. OK, probably we can simply ignore the race with cpu hotplug.
> > >
> > > But it seems that your patch makes the things worse. Lets look at, say,
> > > numachip_send_IPI_mask_allbutself(). The usage of smp_processor_id() is
> > > obviously racy but perhaps we do not care again. But we do not want a warning
> > > from debug_smp_processor_id().
> >
> > Good point.  I forgot that going from all cpus down to allbutself,
> > preemption now matters.
> 
> I am not sure it actually matters wrt "show other CPU's traces". If the preemption
> is possible then the caller can be preempted even before it sends ipi.
> 
> OTOH I think it does matter anyway, even without your patch, otherwise the usage
> of cpu_online_mask is racy and we can hit the "Wait for up to 10 seconds" case.

Hmm,  I understand what you are saying now.

> 
> Btw...
> 
> 	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
> 	for (i = 0; i < 10 * 1000; i++) {
> 		if (cpumask_empty(to_cpumask(backtrace_mask)))
> 			break;
> 		mdelay(1);
> 	}
> 
> OK, but perhaps we should clear backtrace_mask if we return due to timeout.

I can look at that.

> 
> > does disabling preemption help in the cpu
> > hotplug case?
> 
> Yes. But I'd suggest to change your patch to use get_cpu() instead of
> preempt_disable/smp_processor_id.

ok.  Originally I was thinking of the remote hotplug cpu case, which
pre-emption won't block.  But forgot about the local cpu hotplug case.


> 
> And I think it would be better to not discuss this off-list, I added lkml.
> 
> Oleg.
> 

Thanks!

Cheers,
Don

[PATCH 0/4 V2] Print traces on softlockups

[Don Zickus]

Version 2 of this patchset.

Added more patches to handle the 'uniprocessor' panic case by sending NMIs
to every cpu but self.  Only affects x86, sparc.

Aaron Tomlin (4):
  nmi: Provide the option to issue an NMI back trace to every cpu but
    current
  x86, nmi: Add more flexible NMI back trace support
  sparc64, nmi: Add more flexible NMI back trace support
  watchdog: Printing traces for all cpus on lockup detection

 Documentation/kernel-parameters.txt |    5 +++++
 Documentation/sysctl/kernel.txt     |   17 +++++++++++++++++
 arch/sparc/include/asm/irq_64.h     |    2 +-
 arch/sparc/kernel/process_64.c      |   14 +++++++++-----
 arch/x86/include/asm/irq.h          |    2 +-
 arch/x86/kernel/apic/hw_nmi.c       |   16 +++++++++++++---
 include/linux/nmi.h                 |   12 +++++++++++-
 kernel/sysctl.c                     |    9 +++++++++
 kernel/watchdog.c                   |   32 ++++++++++++++++++++++++++++++++
 9 files changed, 98 insertions(+), 11 deletions(-)

[PATCH 1/4 v2] nmi: Provide the option to issue an NMI back trace to every cpu but current

[Don Zickus]

From: Aaron Tomlin <atomlin@redhat.com>

Some times it is preferred not to use the
trigger_all_cpu_backtrace() routine when one wants
to avoid capturing a back trace for current.
For instance if one was previously captured
recently.

This patch provides a new routine namely
trigger_allbutself_cpu_backtrace() which offers
the flexibility to issue an NMI to every cpu but
current and capture a back trace accordingly.

[Added stub in #else clause - dcz]

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 include/linux/nmi.h |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 6a45fb5..a17ab63 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -32,15 +32,24 @@ static inline void touch_nmi_watchdog(void)
 #ifdef arch_trigger_all_cpu_backtrace
 static inline bool trigger_all_cpu_backtrace(void)
 {
-	arch_trigger_all_cpu_backtrace();
+	arch_trigger_all_cpu_backtrace(true);
 
 	return true;
 }
+static inline bool trigger_allbutself_cpu_backtrace(void)
+{
+	arch_trigger_all_cpu_backtrace(false);
+	return true;
+}
 #else
 static inline bool trigger_all_cpu_backtrace(void)
 {
 	return false;
 }
+static inline bool trigger_allbutself_cpu_backtrace(void)
+{
+	return false;
+}
 #endif
 
 #ifdef CONFIG_LOCKUP_DETECTOR
-- 
1.7.1