Skip to contentSkip to conversation

kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

PATCH In review Andrew Cooper 3 participants 5 messages Sep 7, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

This issue came up when debugging pcpu linked list corruption (patches for that issue to follow).

Signature · 2 lines
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

# HG changeset patch
# Parent 1126b3079bef37e1bb5a97b90c14a51d4e1c91c3 kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

The noreboot option is for debugging, to be able to see the dying output. To that end, continuing and possibly booting into the crash kernel defeats one of the purposes of noreboot.

xen/drivers/char/console.c +3 -4
diff -r 1126b3079bef xen/drivers/char/console.c
@@ -963,16 +963,15 @@ void panic(const char *fmt, ...)
 
     debugger_trap_immediate();
 
-#ifdef CONFIG_KEXEC
-    kexec_crash();
-#endif
-
     if ( opt_noreboot )
     {
         machine_halt();
     }
     else
     {
+#ifdef CONFIG_KEXEC
+        kexec_crash();
+#endif
         watchdog_disable();
         machine_restart(5000);
     }

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#1 lore raw
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Andrew Cooper

This issue came up when debugging pcpu linked list corruption (patches for that issue to follow).

Hmm, that's a matter of taste of course. Nor is running kdump really a (direct) reboot action. Both ways have their reasoning imo, so I'm not sure whether keeping things as is or applying the patch is the better thing.

Jan

#2 lore raw
Quoted 9 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Andrew Cooper

This issue came up when debugging pcpu linked list corruption (patches for that issue to follow).

Hmm, that's a matter of taste of course. Nor is running kdump really a (direct) reboot action. Both ways have their reasoning imo, so I'm not sure whether keeping things as is or applying the patch is the better thing.

Jan

Hmm yes - I guess its not as clean cut as I thought, but I would still argue that this patch is the lest surprising alternative.

One tweak I am thinking of working on soon is a "pause-on-reboot" feature which is a bit like noreboot, but will accept a passphrase and continue with the reboot, so you can 'pause', see the dying panic then reboot without needing to invoke the server power management.

(This may have been as a result of being given details for the wrong server power management interface IP address when debugging a server in a data center 7 time zones away, and getting rather confused when it didn't make any attempt to reboot. I imagine someone else might get equally irate if their server was rebooting without warning.)

~Andrew

Quoted 6 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Signature · 2 lines
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com
#3 lore raw
In reply to Jan Beulich
Quoted 8 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Andrew Cooper

This issue came up when debugging pcpu linked list corruption (patches for that issue to follow).

Hmm, that's a matter of taste of course. Nor is running kdump really a (direct) reboot action. Both ways have their reasoning imo, so I'm not sure whether keeping things as is or applying the patch is the better thing.

I note though that kexec_crash() should be stubbed out in the header file when !CONFIG_KEXEC, rather than ifdef at the caller. By the by.

-- Keir

Quoted 2 lines

Jan

#4 lore raw
Quoted 5 lines · Keir Fraser
Quoted 3 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Andrew Cooper

This issue came up when debugging pcpu linked list corruption (patches for that issue to follow).

Quoted 8 lines
Quoted 4 lines

Hmm, that's a matter of taste of course. Nor is running kdump really a (direct) reboot action. Both ways have their reasoning imo, so I'm not sure whether keeping things as is or applying the patch is the better thing.

I note though that kexec_crash() should be stubbed out in the header file when !CONFIG_KEXEC, rather than ifdef at the caller. By the by.

-- Keir

Agreed - I will put it on my todo list to fix. (I think its elsewhere in the codebase as well)

~Andrew

Quoted 4 lines
Quoted 3 lines

Jan

Signature · 2 lines
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com