| Message ID | 20211104161057.1255659-6-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: watchdog fixes | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
Le 04/11/2021 à 17:10, Nicholas Piggin a écrit : > The watchdog unstuck message can't be serialised with other watchdog > messages because that might prevent watchdog reporting. This removes > the big backtrace from the unstuck message, which can get mixed with > other messages and confuse logs, and just prints a single line. I'm not sure that's a good idea to remove the registers and backtrace here. I agree that this output may interleaved (and usually it does), but it is also providing some good information about the culprit block of code. Usually, it's pointing the IRQ release code, and so the IRQ blocking one which are really useful. I don't have a good way to prevent trace interleaving here, but I think interleaved traces are better here than nothing. Thanks, Laurent. > Signed-of-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/kernel/watchdog.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c > index 2444cd10b61a..5f69ba4de1f3 100644 > --- a/arch/powerpc/kernel/watchdog.c > +++ b/arch/powerpc/kernel/watchdog.c > @@ -242,16 +242,10 @@ static void wd_smp_clear_cpu_pending(int cpu) > { > if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending)) { > if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) { > - struct pt_regs *regs = get_irq_regs(); > unsigned long flags; > > pr_emerg("CPU %d became unstuck TB:%lld\n", > cpu, get_tb()); > - print_irqtrace_events(current); > - if (regs) > - show_regs(regs); > - else > - dump_stack(); > > wd_smp_lock(&flags); > cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck); >
Excerpts from Laurent Dufour's message of November 5, 2021 2:48 am: > Le 04/11/2021 à 17:10, Nicholas Piggin a écrit : >> The watchdog unstuck message can't be serialised with other watchdog >> messages because that might prevent watchdog reporting. This removes >> the big backtrace from the unstuck message, which can get mixed with >> other messages and confuse logs, and just prints a single line. > > I'm not sure that's a good idea to remove the registers and backtrace here. > I agree that this output may interleaved (and usually it does), but it is also > providing some good information about the culprit block of code. Usually, it's > pointing the IRQ release code, and so the IRQ blocking one which are really useful. Okay, I was thinking that be inferred from the context usually, but sometimes it's not that easy which I guess is why I added it in the first place. > I don't have a good way to prevent trace interleaving here, but I think > interleaved traces are better here than nothing. Okay we can leave this patch off. Thanks, Nick
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c index 2444cd10b61a..5f69ba4de1f3 100644 --- a/arch/powerpc/kernel/watchdog.c +++ b/arch/powerpc/kernel/watchdog.c @@ -242,16 +242,10 @@ static void wd_smp_clear_cpu_pending(int cpu) { if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending)) { if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) { - struct pt_regs *regs = get_irq_regs(); unsigned long flags; pr_emerg("CPU %d became unstuck TB:%lld\n", cpu, get_tb()); - print_irqtrace_events(current); - if (regs) - show_regs(regs); - else - dump_stack(); wd_smp_lock(&flags); cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);