* [PATCH 1/3] powerpc/watchdog: Tweak watchdog printks
@ 2017-10-12 4:44 Michael Ellerman
2017-10-12 4:44 ` [PATCH 2/3] powerpc/watchdog: regs can't be null in soft_nmi_interrupt() Michael Ellerman
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-10-12 4:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] powerpc/watchdog: regs can't be null in soft_nmi_interrupt()
2017-10-12 4:44 [PATCH 1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
@ 2017-10-12 4:44 ` Michael Ellerman
2017-10-12 4:44 ` [PATCH 3/3] powerpc/watchdog: Print the NIP " Michael Ellerman
2018-01-29 4:13 ` [1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-10-12 4:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] powerpc/watchdog: Print the NIP in soft_nmi_interrupt()
2017-10-12 4:44 [PATCH 1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
2017-10-12 4:44 ` [PATCH 2/3] powerpc/watchdog: regs can't be null in soft_nmi_interrupt() Michael Ellerman
@ 2017-10-12 4:44 ` Michael Ellerman
2017-10-12 8:18 ` Nicholas Piggin
2018-01-29 4:13 ` [1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
2 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2017-10-12 4:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] powerpc/watchdog: Print the NIP in soft_nmi_interrupt()
2017-10-12 4:44 ` [PATCH 3/3] powerpc/watchdog: Print the NIP " Michael Ellerman
@ 2017-10-12 8:18 ` Nicholas Piggin
2017-10-12 12:01 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2017-10-12 8:18 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
On Thu, 12 Oct 2017 15:44:34 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> 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?
Thanks,
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] powerpc/watchdog: Print the NIP in soft_nmi_interrupt()
2017-10-12 8:18 ` Nicholas Piggin
@ 2017-10-12 12:01 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2017-10-12 12:01 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
Nicholas Piggin <npiggin@gmail.com> writes:
> On Thu, 12 Oct 2017 15:44:34 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [1/3] powerpc/watchdog: Tweak watchdog printks
2017-10-12 4:44 [PATCH 1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
2017-10-12 4:44 ` [PATCH 2/3] powerpc/watchdog: regs can't be null in soft_nmi_interrupt() Michael Ellerman
2017-10-12 4:44 ` [PATCH 3/3] powerpc/watchdog: Print the NIP " Michael Ellerman
@ 2018-01-29 4:13 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2018-01-29 4:13 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-29 4:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-12 4:44 [PATCH 1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
2017-10-12 4:44 ` [PATCH 2/3] powerpc/watchdog: regs can't be null in soft_nmi_interrupt() Michael Ellerman
2017-10-12 4:44 ` [PATCH 3/3] powerpc/watchdog: Print the NIP " Michael Ellerman
2017-10-12 8:18 ` Nicholas Piggin
2017-10-12 12:01 ` Michael Ellerman
2018-01-29 4:13 ` [1/3] powerpc/watchdog: Tweak watchdog printks Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).