sreset driven panic/oops message printing fixes
Subsystems (2)
Nicholas Piggin
reviewercover sreset driven panic/oops message printing fixes
Here's a couple of fixes that should go into 4.15 and possibly stable kernels as well. Most of the time when you hit the system reset button, either you're a developer with xmon enabled or customer with a crash dumper enabled. But if neither is the case, then the result is currently pretty useless. After these patches we get nice oops trace on each CPU.
Nicholas Piggin (2):
powerpc: System reset avoid interleaving oops using die
synchronisation
powerpc/pseries,ps3: panic flush kernel messages before halting system
arch/powerpc/include/asm/bug.h | 3 ++- arch/powerpc/kernel/traps.c | 26 +++++++++++++++++++++++++- arch/powerpc/platforms/powernv/opal.c | 18 ++++-------------- arch/powerpc/platforms/ps3/setup.c | 1 + arch/powerpc/platforms/pseries/setup.c | 8 +++++++- 5 files changed, 39 insertions(+), 17 deletions(-)
Signature · 1 line
2.15.0
Nicholas Piggin
reviewerPATCH 1/2 powerpc: System reset avoid interleaving oops using die synchronisation
The die() oops path contains a serializing lock to prevent oops messages from being interleaved. In the case of a system reset initiated oops (e.g., qemu nmi command), __die was being called which lacks that synchronisation and oops reports could be interleaved across CPUs.
A recent patch 4388c9b3a6ee7 ("powerpc: Do not send system reset request through the oops path") changed this to __die to avoid the debugger() call, but there is no real harm to calling it twice if the first time fell through. So go back to using die() here. This was observed to fix the problem.
--- arch/powerpc/kernel/traps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/kernel/traps.c +1 -1
@@ -337,7 +337,7 @@ void system_reset_exception(struct pt_regs *regs)* No debugger or crash dump registered, print logs then * panic. */ -__die ("System Reset" , regs,SIGABRT ); +die ("System Reset" , regs,SIGABRT );mdelay (2 *MSEC_PER_SEC );/* Wait a little while for others to print */ add_taint (TAINT_DIE ,LOCKDEP_NOW_UNRELIABLE );
Signature · 1 line
2.15.0
Re: [PATCH 1/2] powerpc: System reset avoid interleaving oops using die synchronisation
Quoted 6 lines · Nicholas Piggin
The
die()oops path contains a serializing lock to prevent oops messages from being interleaved. In the case of a system reset initiated oops (e.g., qemu nmi command),__diewas being called which lacks that synchronisation and oops reports could be interleaved across CPUs.
Quoted 8 lines
A recent patch
4388c9b3a6ee7("powerpc: Do not send system reset request through the oops path") changed this to__dieto avoid thedebugger()call, but there is no real harm to calling it twice if the first time fell through. So go back to usingdie()here. This was observed to fix the problem.Fixes 4388c9b3a6ee7 ("powerpc: Do not send system reset request through the oops path")Signed-off-by Nicholas Piggin <[email protected]>
Quoted 17 lines
--- arch/powerpc/kernel/traps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/powerpc/kernel/traps.c +1
-1@@ -337,7 +337,7 @@ void system_reset_exception(struct pt_regs *regs)* No debugger or crash dump registered, print logs then * panic. */ -__die ("System Reset" , regs,SIGABRT ); +die ("System Reset" , regs,SIGABRT );mdelay (2 *MSEC_PER_SEC );/* Wait a little while for others to print */ add_taint (TAINT_DIE ,LOCKDEP_NOW_UNRELIABLE );
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Michael Ellerman
AppliedRe: [1/2] powerpc: System reset avoid interleaving oops using die synchronisation
In reply to Nicholas PigginQuoted 12 lines · Nicholas Piggin
The
die()oops path contains a serializing lock to prevent oops messages from being interleaved. In the case of a system reset initiated oops (e.g., qemu nmi command),__diewas being called which lacks that synchronisation and oops reports could be interleaved across CPUs.A recent patch
4388c9b3a6ee7("powerpc: Do not send system reset request through the oops path") changed this to__dieto avoid thedebugger()call, but there is no real harm to calling it twice if the first time fell through. So go back to usingdie()here. This was observed to fix the problem.
Quoted 3 lines
Fixes 4388c9b3a6ee7 ("powerpc: Do not send system reset request through the oops path")Signed-off-by Nicholas Piggin <[email protected]>Reviewed-by David Gibson <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/4552d128c26e0f0f27a5bd2fadc240
cheers
Nicholas Piggin
reviewerPATCH 2/2 powerpc/pseries, ps3: panic flush kernel messages before halting system
In reply to Nicholas PigginPlatforms with a panic handler that halts the system can have problems getting kernel messages out, because the panic notifiers are called before kernel/panic.c does its flushing of printk buffers an console etc.
This was attempted to be solved with commit a3b2cb30f252 ("powerpc: Do not call ppc_md.panic in fadump panic notifier"), but that wasn't the right approach and caused other problems, and was reverted by commit ab9dbf771ff9.
Instead, the powernv shutdown paths have already had a similar problem, fixed by taking the message flushing sequence from kernel/panic.c. That's a little bit ugly, but while we have the code duplicated, it will work for this case as well. So have ppc panic handlers do the same flushing before they terminate.
Without this patch, a qemu pseries_le_defconfig guest stops silently when issued the nmi command when xmon is off and no crash dumpers enabled. Afterwards, an oops is printed by each CPU as expected.
--- arch/powerpc/include/asm/bug.h | 3 ++- arch/powerpc/kernel/traps.c | 24 ++++++++++++++++++++++++ arch/powerpc/platforms/powernv/opal.c | 18 ++++-------------- arch/powerpc/platforms/ps3/setup.c | 1 + arch/powerpc/platforms/pseries/setup.c | 8 +++++++- 5 files changed, 38 insertions(+), 16 deletions(-)
arch/powerpc/include/asm/bug.h +2 -1
@@ -135,7 +135,8 @@ extern void bad_page_fault(struct pt_regs *, unsigned long, int);extern void _exception (int ,struct pt_regs *,int ,unsigned long );extern void die (const char *,struct pt_regs *,long );extern bool die_will_crash (void ); - +extern void panic_flush_kmsg_start (void ); +extern void panic_flush_kmsg_end (void );#endif /* !__ASSEMBLY__ */ #endif /* __KERNEL__ */
arch/powerpc/kernel/traps.c +24 -0
@@ -38,6 +38,8 @@#include <linux/ratelimit.h> #include <linux/context_tracking.h> #include <linux/smp.h> +#include <linux/console.h> +#include <linux/kmsg_dump.h> #include <asm/emulated_ops.h> #include <asm/pgtable.h>
@@ -142,6 +144,28 @@ static int die_owner = -1;static unsigned int die_nest_count;static int die_counter; +extern void panic_flush_kmsg_start (void ) +{ +/* +* These are mostly taken from kernel/panic.c, but tries to do +* relatively minimal work. Don't use delay functions (TB may +* be broken), don't crash dump (need to set a firmware log), +* don't run notifiers. We do want to get some information to +* Linux console. +*/ +console_verbose (); +bust_spinlocks (1 ); +} + +extern void panic_flush_kmsg_end (void ) +{ +printk_safe_flush_on_panic (); +kmsg_dump (KMSG_DUMP_PANIC ); +bust_spinlocks (0 ); +debug_locks_off (); +console_flush_on_panic (); +} +static unsigned long oops_begin (struct pt_regs *regs) {int cpu;
arch/powerpc/platforms/powernv/opal.c +4 -14
@@ -461,24 +461,14 @@ static int opal_recover_mce(struct pt_regs *regs,void pnv_platform_error_reboot (struct pt_regs *regs,const char *msg) { -/* -* This is mostly taken from kernel/panic.c, but tries to do -* relatively minimal work. Don't use delay functions (TB may -* be broken), don't crash dump (need to set a firmware log), -* don't run notifiers. We do want to get some information to -* Linux console. -*/ -console_verbose (); -bust_spinlocks (1 ); +panic_flush_kmsg_start (); +pr_emerg ("Hardware platform error: %s\n" , msg);if (regs)show_regs (regs);smp_send_stop (); -printk_safe_flush_on_panic (); -kmsg_dump (KMSG_DUMP_PANIC ); -bust_spinlocks (0 ); -debug_locks_off (); -console_flush_on_panic (); + +panic_flush_kmsg_end ();/* * Don't bother to shut things down because this will
arch/powerpc/platforms/ps3/setup.c +1 -0
@@ -113,6 +113,7 @@ static void ps3_panic(char *str)printk (" System does not reboot automatically.\n" );printk (" Please press POWER button.\n" );printk ("\n" ); +panic_flush_kmsg_end ();while (1 )lv1_pause (1 );
arch/powerpc/platforms/pseries/setup.c +7 -1
@@ -498,6 +498,12 @@ static void __init pSeries_setup_arch(void) ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; } +static void pSeries_panic (char *str) +{ +panic_flush_kmsg_end (); +rtas_os_term (str); +} +static int __init pSeries_init_panel (void ) {/* Manually leave the kernel version on the panel. */
@@ -726,7 +732,7 @@ define_machine(pseries) { .pcibios_fixup = pSeries_final_fixup, .restart = rtas_restart, .halt = rtas_halt, - .panic = rtas_os_term, + .panic = pSeries_panic, .get_boot_time = rtas_get_boot_time, .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time,
Signature · 1 line
2.15.0
Re: [PATCH 2/2] powerpc/pseries,ps3: panic flush kernel messages before halting system
Quoted 16 lines · Nicholas Piggin
Platforms with a panic handler that halts the system can have problems getting kernel messages out, because the panic notifiers are called before
kernel/panic.cdoes its flushing of printk buffers an console etc.This was attempted to be solved with commit
a3b2cb30f252("powerpc: Do not call ppc_md.panic in fadump panic notifier"), but that wasn't the right approach and caused other problems, and was reverted by commitab9dbf771ff9.Instead, the powernv shutdown paths have already had a similar problem, fixed by taking the message flushing sequence from
kernel/panic.c. That's a little bit ugly, but while we have the code duplicated, it will work for this case as well. So have ppc panic handlers do the same flushing before they terminate.
Quoted 6 lines
Without this patch, a qemu
pseries_le_defconfigguest stops silently when issued the nmi command when xmon is off and no crash dumpers enabled. Afterwards, an oops is printed by each CPU as expected.Fixes ab9dbf771ff9 ("Revert "powerpc: Do not call ppc_md.panic in fadump panic notifier"")Signed-off-by Nicholas Piggin <[email protected]>
Quoted 135 lines
--- arch/powerpc/include/asm/bug.h | 3 ++- arch/powerpc/kernel/traps.c | 24 ++++++++++++++++++++++++ arch/powerpc/platforms/powernv/opal.c | 18 ++++-------------- arch/powerpc/platforms/ps3/setup.c | 1 + arch/powerpc/platforms/pseries/setup.c | 8 +++++++- 5 files changed, 38 insertions(+), 16 deletions(-)arch/powerpc/include/asm/bug.h +2
-1@@ -135,7 +135,8 @@ extern void bad_page_fault(struct pt_regs *, unsigned long, int);extern void _exception (int ,struct pt_regs *,int ,unsigned long );extern void die (const char *,struct pt_regs *,long );extern bool die_will_crash (void ); - +extern void panic_flush_kmsg_start (void ); +extern void panic_flush_kmsg_end (void );#endif /* !__ASSEMBLY__ */ #endif /* __KERNEL__ */ arch/powerpc/kernel/traps.c +24
-0@@ -38,6 +38,8 @@#include <linux/ratelimit.h> #include <linux/context_tracking.h> #include <linux/smp.h> +#include <linux/console.h> +#include <linux/kmsg_dump.h> #include <asm/emulated_ops.h> #include <asm/pgtable.h> @@ -142,6 +144,28 @@ static int die_owner = -1;static unsigned int die_nest_count;static int die_counter; +extern void panic_flush_kmsg_start (void ) +{ +/* +* These are mostly taken from kernel/panic.c, but tries to do +* relatively minimal work. Don't use delay functions (TB may +* be broken), don't crash dump (need to set a firmware log), +* don't run notifiers. We do want to get some information to +* Linux console. +*/ +console_verbose (); +bust_spinlocks (1 ); +} + +extern void panic_flush_kmsg_end (void ) +{ +printk_safe_flush_on_panic (); +kmsg_dump (KMSG_DUMP_PANIC ); +bust_spinlocks (0 ); +debug_locks_off (); +console_flush_on_panic (); +} +static unsigned long oops_begin (struct pt_regs *regs) {int cpu;arch/powerpc/platforms/powernv/opal.c +4
-14@@ -461,24 +461,14 @@ static int opal_recover_mce(struct pt_regs *regs,void pnv_platform_error_reboot (struct pt_regs *regs,const char *msg) { -/* -* This is mostly taken from kernel/panic.c, but tries to do -* relatively minimal work. Don't use delay functions (TB may -* be broken), don't crash dump (need to set a firmware log), -* don't run notifiers. We do want to get some information to -* Linux console. -*/ -console_verbose (); -bust_spinlocks (1 ); +panic_flush_kmsg_start (); +pr_emerg ("Hardware platform error: %s\n" , msg);if (regs)show_regs (regs);smp_send_stop (); -printk_safe_flush_on_panic (); -kmsg_dump (KMSG_DUMP_PANIC ); -bust_spinlocks (0 ); -debug_locks_off (); -console_flush_on_panic (); + +panic_flush_kmsg_end ();/* * Don't bother to shut things down because this will arch/powerpc/platforms/ps3/setup.c +1
-0@@ -113,6 +113,7 @@ static void ps3_panic(char *str)printk (" System does not reboot automatically.\n" );printk (" Please press POWER button.\n" );printk ("\n" ); +panic_flush_kmsg_end ();while (1 )lv1_pause (1 );arch/powerpc/platforms/pseries/setup.c +7
-1@@ -498,6 +498,12 @@ static void __init pSeries_setup_arch(void) ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; } +static void pSeries_panic (char *str) +{ +panic_flush_kmsg_end (); +rtas_os_term (str); +} +static int __init pSeries_init_panel (void ) {/* Manually leave the kernel version on the panel. */ @@ -726,7 +732,7 @@ define_machine(pseries) { .pcibios_fixup = pSeries_final_fixup, .restart = rtas_restart, .halt = rtas_halt, - .panic = rtas_os_term, + .panic = pSeries_panic, .get_boot_time = rtas_get_boot_time, .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time,
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Michael Ellerman
AppliedRe: [2/2] powerpc/pseries, ps3: panic flush kernel messages before halting system
In reply to Nicholas PigginQuoted 16 lines · Nicholas Piggin
Platforms with a panic handler that halts the system can have problems getting kernel messages out, because the panic notifiers are called before
kernel/panic.cdoes its flushing of printk buffers an console etc.This was attempted to be solved with commit
a3b2cb30f252("powerpc: Do not call ppc_md.panic in fadump panic notifier"), but that wasn't the right approach and caused other problems, and was reverted by commitab9dbf771ff9.Instead, the powernv shutdown paths have already had a similar problem, fixed by taking the message flushing sequence from
kernel/panic.c. That's a little bit ugly, but while we have the code duplicated, it will work for this case as well. So have ppc panic handlers do the same flushing before they terminate.
Quoted 7 lines
Without this patch, a qemu
pseries_le_defconfigguest stops silently when issued the nmi command when xmon is off and no crash dumpers enabled. Afterwards, an oops is printed by each CPU as expected.Fixes ab9dbf771ff9 ("Revert "powerpc: Do not call ppc_md.panic in fadump panic notifier"")Signed-off-by Nicholas Piggin <[email protected]>Reviewed-by David Gibson <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/35adacd6fc48d658419522f192a3c8
cheers