Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1457881 > unrolled thread

[PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI

Started byChris Metcalf <cmetcalf@mellanox.com>
First post2016-08-08 18:10 +0200
Last post2016-08-09 14:40 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI Chris Metcalf <cmetcalf@mellanox.com> - 2016-08-08 18:10 +0200
    Re: [PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of  a self-NMI Petr Mladek <pmladek@suse.com> - 2016-08-09 14:40 +0200

#1457881 — [PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI

FromChris Metcalf <cmetcalf@mellanox.com>
Date2016-08-08 18:10 +0200
Subject[PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI
Message-ID<s3VhM-3zC-19@gated-at.bofh.it>
Currently on arm there is code that checks whether it should call
dump_stack() explicitly, to avoid trying to raise an NMI when the
current context is not preemptible by the backtrace IPI.  Similarly,
the forthcoming arch/tile support uses an IPI mechanism that does
not support generating an NMI to self.

Accordingly, move the code that guards this case into the generic
mechanism, and invoke it unconditionally whenever we want a
backtrace of the current cpu.  It seems plausible that in all cases,
dump_stack() will generate better information than generating a
stack from the NMI handler.  The register state will be missing,
but that state is likely not particularly helpful in any case.

Or, if we think it is helpful, we should be capturing and emitting
the current register state in all cases when regs == NULL is passed
to nmi_cpu_backtrace().

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
Acked-by: Aaron Tomlin <atomlin@redhat.com>
---
 arch/arm/kernel/smp.c | 9 ---------
 lib/nmi_backtrace.c   | 9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 732583dcb8f8..157c991f8de2 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -748,15 +748,6 @@ core_initcall(register_cpufreq_notifier);
 
 static void raise_nmi(cpumask_t *mask)
 {
-	/*
-	 * Generate the backtrace directly if we are running in a calling
-	 * context that is not preemptible by the backtrace IPI. Note
-	 * that nmi_cpu_backtrace() automatically removes the current cpu
-	 * from mask.
-	 */
-	if (cpumask_test_cpu(smp_processor_id(), mask) && irqs_disabled())
-		nmi_cpu_backtrace(NULL);
-
 	smp_cross_call(mask, IPI_CPU_BACKTRACE);
 }
 
diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
index 5448d6621102..2933f0680174 100644
--- a/lib/nmi_backtrace.c
+++ b/lib/nmi_backtrace.c
@@ -49,6 +49,15 @@ void nmi_trigger_cpumask_backtrace(bool include_self,
 	if (!include_self)
 		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
 
+	/*
+	 * Don't try to send an NMI to this cpu; it may work on some
+	 * architectures, but on others it may not, and we'll get
+	 * information at least as useful just by doing a dump_stack() here.
+	 * Note that nmi_cpu_backtrace(NULL) will clear the cpu bit.
+	 */
+	if (cpumask_test_cpu(this_cpu, to_cpumask(backtrace_mask)))
+		nmi_cpu_backtrace(NULL);
+
 	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
 		pr_info("Sending NMI from CPU %d to CPUs %*pbl:\n",
 			this_cpu, nr_cpumask_bits, to_cpumask(backtrace_mask));
-- 
2.7.2

[toc] | [next] | [standalone]


#1458668 — Re: [PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI

FromPetr Mladek <pmladek@suse.com>
Date2016-08-09 14:40 +0200
SubjectRe: [PATCH v7 2/4] nmi_backtrace: do a local dump_stack() instead of a self-NMI
Message-ID<s4eu6-7y7-9@gated-at.bofh.it>
In reply to#1457881
On Mon 2016-08-08 12:03:36, Chris Metcalf wrote:
> Currently on arm there is code that checks whether it should call
> dump_stack() explicitly, to avoid trying to raise an NMI when the
> current context is not preemptible by the backtrace IPI.  Similarly,
> the forthcoming arch/tile support uses an IPI mechanism that does
> not support generating an NMI to self.
> 
> Accordingly, move the code that guards this case into the generic
> mechanism, and invoke it unconditionally whenever we want a
> backtrace of the current cpu.  It seems plausible that in all cases,
> dump_stack() will generate better information than generating a
> stack from the NMI handler.  The register state will be missing,
> but that state is likely not particularly helpful in any case.
> 
> Or, if we think it is helpful, we should be capturing and emitting
> the current register state in all cases when regs == NULL is passed
> to nmi_cpu_backtrace().
> 
> Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
> Acked-by: Aaron Tomlin <atomlin@redhat.com>

Sounds and looks fine to me.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr Mladek

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web