diff mbox

[4.0-rc1,v17,5/6] x86/nmi: Use common printk functions

Message ID 1425463974-23568-6-git-send-email-daniel.thompson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Thompson March 4, 2015, 10:12 a.m. UTC
Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support safe
all-cpu backtracing from NMI has been copied to printk.c to make it
accessible to other architectures.

Port the x86 NMI backtrace to the generic code.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/Kconfig              |   1 +
 arch/x86/kernel/apic/hw_nmi.c | 101 +++---------------------------------------
 2 files changed, 8 insertions(+), 94 deletions(-)

Comments

Ingo Molnar March 5, 2015, 12:54 a.m. UTC | #1
* Daniel Thompson <daniel.thompson@linaro.org> wrote:

> Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support 
> safe all-cpu backtracing from NMI has been copied to printk.c to 
> make it accessible to other architectures.
> 
> Port the x86 NMI backtrace to the generic code.

Is there any difference between the generic and the x86 code as they 
stand today?

Thanks,

	Ingo
Daniel Thompson March 5, 2015, 12:29 p.m. UTC | #2
On Thu, 2015-03-05 at 01:54 +0100, Ingo Molnar wrote:
> * Daniel Thompson <daniel.thompson@linaro.org> wrote:
> 
> > Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support 
> > safe all-cpu backtracing from NMI has been copied to printk.c to 
> > make it accessible to other architectures.
> > 
> > Port the x86 NMI backtrace to the generic code.
> 
> Is there any difference between the generic and the x86 code as they 
> stand today?

Shouldn't be any user observable change but there are some changes,
mostly due to review comments.

1. The seq_buf structures are initialized at boot and *after* they
   are consumed (originally they were initialized just before use).

2. The generic code doesn't maintain an equivalent of backtrace_mask
   (which was essentially a copy of cpus_online made when backtracing
   was requested) and instead iterates using for_each_possible_cpu()
   to initialize and dump the seq_buf:s.


Daniel.


PS
The main piece that git code motion tracking should follow if I squashed
the generic and x86 patches together would be nmi_vprintk(). I suspect
most of the rest would be missed as the code copies is in pretty small
fragments.
Ingo Molnar March 5, 2015, 7:46 p.m. UTC | #3
* Daniel Thompson <daniel.thompson@linaro.org> wrote:

> On Thu, 2015-03-05 at 01:54 +0100, Ingo Molnar wrote:
> > * Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > 
> > > Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support 
> > > safe all-cpu backtracing from NMI has been copied to printk.c to 
> > > make it accessible to other architectures.
> > > 
> > > Port the x86 NMI backtrace to the generic code.
> > 
> > Is there any difference between the generic and the x86 code as they 
> > stand today?
> 
> Shouldn't be any user observable change but there are some changes,
> mostly due to review comments.
> 
> 1. The seq_buf structures are initialized at boot and *after* they
>    are consumed (originally they were initialized just before use).
> 
> 2. The generic code doesn't maintain an equivalent of backtrace_mask
>    (which was essentially a copy of cpus_online made when backtracing
>    was requested) and instead iterates using for_each_possible_cpu()
>    to initialize and dump the seq_buf:s.

Ok, I have no fundamental objections:

Acked-by: Ingo Molnar <mingo@kernel.org>

I suspect you want to carry the x86 bits yourself?

Thanks,

	Ingo
Daniel Thompson March 6, 2015, 7:02 p.m. UTC | #4
On Thu, 2015-03-05 at 20:46 +0100, Ingo Molnar wrote:
> * Daniel Thompson <daniel.thompson@linaro.org> wrote:
> 
> > On Thu, 2015-03-05 at 01:54 +0100, Ingo Molnar wrote:
> > > * Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > > 
> > > > Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support 
> > > > safe all-cpu backtracing from NMI has been copied to printk.c to 
> > > > make it accessible to other architectures.
> > > > 
> > > > Port the x86 NMI backtrace to the generic code.
> > > 
> > > Is there any difference between the generic and the x86 code as they 
> > > stand today?
> > 
> > Shouldn't be any user observable change but there are some changes,
> > mostly due to review comments.
> > 
> > 1. The seq_buf structures are initialized at boot and *after* they
> >    are consumed (originally they were initialized just before use).
> > 
> > 2. The generic code doesn't maintain an equivalent of backtrace_mask
> >    (which was essentially a copy of cpus_online made when backtracing
> >    was requested) and instead iterates using for_each_possible_cpu()
> >    to initialize and dump the seq_buf:s.
> 
> Ok, I have no fundamental objections:
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> I suspect you want to carry the x86 bits yourself?

I've done plenty of bisectability testing on this set so patches 4 and 5
could be separated from the set and go via the x86 tree. However with
your ack I hope that taking the patchset via the irqchip route should be
possible.

Jason: After I've attended to Joe Perches/Steven Rostedt's comments will
you be comfortable enough to take patches 1-5 through one of your
trees? 

It would be great to deliver patch 6 too but rmk is having a short break
so getting an ack for that may not work out


Daniel.
diff mbox

Patch

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c2fb8a87dccb..fbae5564a1f3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -141,6 +141,7 @@  config X86
 	select ACPI_LEGACY_TABLES_LOOKUP if ACPI
 	select X86_FEATURE_NAMES if PROC_FS
 	select SRCU
+	select PRINTK_NMI if X86_LOCAL_APIC
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 6873ab925d00..8bc00476011d 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -30,40 +30,16 @@  u64 hw_nmi_get_sample_period(int watchdog_thresh)
 #ifdef arch_trigger_all_cpu_backtrace
 /* For reliability, we're prepared to waste bits here. */
 static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
-static cpumask_t printtrace_mask;
-
-#define NMI_BUF_SIZE		4096
-
-struct nmi_seq_buf {
-	unsigned char		buffer[NMI_BUF_SIZE];
-	struct seq_buf		seq;
-};
-
-/* Safe printing in NMI context */
-static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
-
-/* "in progress" flag of arch_trigger_all_cpu_backtrace */
-static unsigned long backtrace_flag;
-
-static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
-{
-	const char *buf = s->buffer + start;
-
-	printk("%.*s", (end - start) + 1, buf);
-}
 
 void arch_trigger_all_cpu_backtrace(bool include_self)
 {
-	struct nmi_seq_buf *s;
-	int len;
-	int cpu;
 	int i;
 	int this_cpu = get_cpu();
 
-	if (test_and_set_bit(0, &backtrace_flag)) {
+	if (0 != printk_nmi_prepare()) {
 		/*
-		 * If there is already a trigger_all_cpu_backtrace() in progress
-		 * (backtrace_flag == 1), don't output double cpu dump infos.
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
 		 */
 		put_cpu();
 		return;
@@ -73,16 +49,6 @@  void arch_trigger_all_cpu_backtrace(bool include_self)
 	if (!include_self)
 		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
 
-	cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask));
-	/*
-	 * Set up per_cpu seq_buf buffers that the NMIs running on the other
-	 * CPUs will write to.
-	 */
-	for_each_cpu(cpu, to_cpumask(backtrace_mask)) {
-		s = &per_cpu(nmi_print_seq, cpu);
-		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
-	}
-
 	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
 		pr_info("sending NMI to %s CPUs:\n",
 			(include_self ? "all" : "other"));
@@ -97,73 +63,20 @@  void arch_trigger_all_cpu_backtrace(bool include_self)
 		touch_softlockup_watchdog();
 	}
 
-	/*
-	 * Now that all the NMIs have triggered, we can dump out their
-	 * back traces safely to the console.
-	 */
-	for_each_cpu(cpu, &printtrace_mask) {
-		int last_i = 0;
-
-		s = &per_cpu(nmi_print_seq, cpu);
-		len = seq_buf_used(&s->seq);
-		if (!len)
-			continue;
-
-		/* Print line by line. */
-		for (i = 0; i < len; i++) {
-			if (s->buffer[i] == '\n') {
-				print_seq_line(s, last_i, i);
-				last_i = i + 1;
-			}
-		}
-		/* Check if there was a partial line. */
-		if (last_i < len) {
-			print_seq_line(s, last_i, len - 1);
-			pr_cont("\n");
-		}
-	}
-
-	clear_bit(0, &backtrace_flag);
-	smp_mb__after_atomic();
+	printk_nmi_complete();
 	put_cpu();
 }
 
-/*
- * It is not safe to call printk() directly from NMI handlers.
- * It may be fine if the NMI detected a lock up and we have no choice
- * but to do so, but doing a NMI on all other CPUs to get a back trace
- * can be done with a sysrq-l. We don't want that to lock up, which
- * can happen if the NMI interrupts a printk in progress.
- *
- * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
- * the content into a per cpu seq_buf buffer. Then when the NMIs are
- * all done, we can safely dump the contents of the seq_buf to a printk()
- * from a non NMI context.
- */
-static int nmi_vprintk(const char *fmt, va_list args)
-{
-	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
-	unsigned int len = seq_buf_used(&s->seq);
-
-	seq_buf_vprintf(&s->seq, fmt, args);
-	return seq_buf_used(&s->seq) - len;
-}
-
 static int
 arch_trigger_all_cpu_backtrace_handler(unsigned int cmd, struct pt_regs *regs)
 {
-	int cpu;
-
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
 	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-		printk_func_t printk_func_save = this_cpu_read(printk_func);
-
-		/* Replace printk to write into the NMI seq */
-		this_cpu_write(printk_func, nmi_vprintk);
+		printk_nmi_this_cpu_begin();
 		printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu);
 		show_regs(regs);
-		this_cpu_write(printk_func, printk_func_save);
+		printk_nmi_this_cpu_end();
 
 		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
 		return NMI_HANDLED;