[PATCH 1/3] powerpc/watchdog: Tweak watchdog printks

Use pr_fmt() in the watchdog code, so we don't have to say "Watchdog"
so many times.

Rather than "CPU:%d" just spell it "CPU %d", "Hard" doesn't need a
capital in the middle of a sentence, and "LOCKUP other CPUS" should be
"LOCKUP on other CPUS".

Also make it clear when a CPU self detects a lockup by spelling it
out.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/watchdog.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 15e209a37c2d..937cb92374c7 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c

@@ -5,6 +5,9 @@
  *
  * This uses code from arch/sparc/kernel/nmi.c and kernel/watchdog.c
  */
+
+#define pr_fmt(fmt) "watchdog: " fmt
+
 #include <linux/kernel.h>
 #include <linux/param.h>
 #include <linux/init.h>

@@ -89,7 +92,7 @@ static inline void wd_smp_unlock(unsigned long *flags)
 
 static void wd_lockup_ipi(struct pt_regs *regs)
 {
-	pr_emerg("Watchdog CPU:%d Hard LOCKUP\n", raw_smp_processor_id());
+	pr_emerg("CPU %d Hard LOCKUP\n", raw_smp_processor_id());
 	print_modules();
 	print_irqtrace_events(current);
 	if (regs)

@@ -130,8 +133,8 @@ static void watchdog_smp_panic(int cpu, u64 tb)
 	if (cpumask_weight(&wd_smp_cpus_pending) == 0)
 		goto out;
 
-	pr_emerg("Watchdog CPU:%d detected Hard LOCKUP other CPUS:%*pbl\n",
-			cpu, cpumask_pr_args(&wd_smp_cpus_pending));
+	pr_emerg("CPU %d detected hard LOCKUP on other CPUs %*pbl\n",
+		 cpu, cpumask_pr_args(&wd_smp_cpus_pending));
 
 	if (!sysctl_hardlockup_all_cpu_backtrace) {
 		/*

@@ -174,7 +177,7 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
 		if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) {
 			unsigned long flags;
 
-			pr_emerg("Watchdog CPU:%d became unstuck\n", cpu);
+			pr_emerg("CPU %d became unstuck\n", cpu);
 			wd_smp_lock(&flags);
 			cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
 			wd_smp_unlock(&flags);

@@ -232,7 +235,7 @@ void soft_nmi_interrupt(struct pt_regs *regs)
 		}
 		set_cpu_stuck(cpu, tb);
 
-		pr_emerg("Watchdog CPU:%d Hard LOCKUP\n", cpu);
+		pr_emerg("CPU %d self-detected hard LOCKUP\n", cpu);
 		print_modules();
 		print_irqtrace_events(current);
 		if (regs)

@@ -388,7 +391,7 @@ int __init watchdog_nmi_probe(void)
 					"powerpc/watchdog:online",
 					start_wd_on_cpu, stop_wd_on_cpu);
 	if (err < 0) {
-		pr_warn("Watchdog could not be initialized");
+		pr_warn("could not be initialized");
 		return err;
 	}
 	return 0;

-- 
2.7.4

soft_nmi_interrupt() is called directly from the asm exception
handling code, which passes regs as a pointer to the stack. So regs
can't be NULL, it may be full of junk, but that's a separate problem.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/watchdog.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 937cb92374c7..2494cbe34132 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c

@@ -238,10 +238,7 @@ void soft_nmi_interrupt(struct pt_regs *regs)
 		pr_emerg("CPU %d self-detected hard LOCKUP\n", cpu);
 		print_modules();
 		print_irqtrace_events(current);
-		if (regs)
-			show_regs(regs);
-		else
-			dump_stack();
+		show_regs(regs);
 
 		wd_smp_unlock(&flags);
 

-- 
2.7.4

When a CPU detects its locked up via soft_nmi_interrupt() we have
pt_regs, so print the regs->nip, which points to where we took the
soft-NMI.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/watchdog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 2494cbe34132..4594ba0979e4 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c

@@ -235,7 +235,7 @@ void soft_nmi_interrupt(struct pt_regs *regs)
 		}
 		set_cpu_stuck(cpu, tb);
 
-		pr_emerg("CPU %d self-detected hard LOCKUP\n", cpu);
+		pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n", cpu, (void *)regs->nip);
 		print_modules();
 		print_irqtrace_events(current);
 		show_regs(regs);

-- 
2.7.4

On Thu, 12 Oct 2017 15:44:34 +1100
Michael Ellerman [off-list ref] wrote:

These patches all look fine to me, but we should be printing nip
with show_regs, so why here too?

Thanks,
Nick

Nicholas Piggin [off-list ref] writes:

On Thu, 12 Oct 2017 15:44:34 +1100
Michael Ellerman [off-list ref] wrote:

quoted

When a CPU detects its locked up via soft_nmi_interrupt() we have
pt_regs, so print the regs->nip, which points to where we took the
soft-NMI.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/watchdog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 2494cbe34132..4594ba0979e4 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c

@@ -235,7 +235,7 @@ void soft_nmi_interrupt(struct pt_regs *regs)
 		}
 		set_cpu_stuck(cpu, tb);
 
-		pr_emerg("CPU %d self-detected hard LOCKUP\n", cpu);
+		pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n", cpu, (void *)regs->nip);
 		print_modules();
 		print_irqtrace_events(current);
 		show_regs(regs);

These patches all look fine to me, but we should be printing nip
with show_regs, so why here too?

Because we can? :)

But maybe it's overkill.

We also have issues with loglevels, where sometimes you don't see the
regs, and just the EMERG lines, so it helps then.

But possibly we should fix that by making show_regs() EMERG, though
other arches don't seem to.

cheers

On Thu, 2017-10-12 at 04:44:32 UTC, Michael Ellerman wrote:

Use pr_fmt() in the watchdog code, so we don't have to say "Watchdog"
so many times.

Rather than "CPU:%d" just spell it "CPU %d", "Hard" doesn't need a
capital in the middle of a sentence, and "LOCKUP other CPUS" should be
"LOCKUP on other CPUS".

Also make it clear when a CPU self detects a lockup by spelling it
out.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Series applied to powerpc next.

https://git.kernel.org/powerpc/c/d8fa82e0e2b1ad9c66aa76810615ad

cheers