Skip to contentSkip to conversation

Panic on softdog timeout

PATCH In review Anithra P Janakiraman 3 participants 5 messages Jan 18, 2011 – Jan 25, 2011

Subsystems (2)
Watchdog device drivers kexec
lore
Share & tools
lore

Anithra P Janakiraman

author
#0 lore raw

cover Panic on softdog timeout

Hi.

We currently have no way of determining the reason for failure when a softdog timeout occurs. At the minimum a snapshot of the system would help to determine the cause. The attached patch invokes panic on softdog timeout iff kdump is configured, if kdump is not configured it works as usual.

Signed-off-by Anithra P J <[email protected]>
---
 drivers/watchdog/softdog.c |   14 ++++++++++----
 kernel/kexec.c             |    1 +
 2 files changed, 11 insertions(+), 4 deletions(-)
drivers/watchdog/softdog.c +10 -4
Index: linux-2.6.37-rc4/drivers/watchdog/softdog.c
===================================================================
@@ -48,6 +48,7 @@
 #include <linux/init.h>
 #include <linux/jiffies.h>
 #include <linux/uaccess.h>
+#include <linux/kexec.h>
 
 #define PFX "SoftDog: "
 

@@ -99,10 +100,15 @@
 	if (soft_noboot)
 		printk(KERN_CRIT PFX "Triggered - Reboot ignored.\n");
 	else {
-		printk(KERN_CRIT PFX "Initiating system reboot.\n");
-		emergency_restart();
-		printk(KERN_CRIT PFX "Reboot didn't ?????\n");
-	}
+		if (kexec_crash_image) {
+			printk(KERN_CRIT PFX "Initiating kdump. \n");
+			panic("Watchdog timer expired.");
+		} else {
+			printk(KERN_CRIT PFX "Initiating system reboot. \n");
+			emergency_restart();
+			printk(KERN_CRIT PFX "Reboot didn't ?????\n");
+		}
+	      }
 }
 
 /*
kernel/kexec.c +1 -0
Index: linux-2.6.37-rc4/kernel/kexec.c
===================================================================
@@ -935,6 +935,7 @@
  */
 struct kimage *kexec_image;
 struct kimage *kexec_crash_image;
+EXPORT_SYMBOL(kexec_crash_image);
 
 static DEFINE_MUTEX(kexec_mutex);
#1 lore raw
Quoted 9 lines · Anithra P Janakiraman

Hi.

We currently have no way of determining the reason for failure when a softdog timeout occurs. At the minimum a snapshot of the system would help to determine the cause.
The attached patch invokes panic on softdog timeout iff kdump is configured, if kdump is not configured it works as usual.

We don't do it in this way, check softlockup_panic, we have a boot parameter, i.e. "softlockup_panic=". :)

author
#2 lore raw
Quoted 11 lines · Américo Wang
Quoted 9 lines · Anithra P Janakiraman

Hi.

We currently have no way of determining the reason for failure when a softdog timeout occurs. At the minimum a snapshot of the system would help to determine the cause.
The attached patch invokes panic on softdog timeout iff kdump is configured, if kdump is not configured it works as usual.

Quoted 2 lines

We don't do it in this way, check softlockup_panic, we have a boot parameter, i.e. "softlockup_panic=". :)

Some softdog specific scenarios cannot be handled by a softlockup detector. We use softdog to watch for critical application failures, where it is possible that the application has failed but there isn't a softlockup as such.
For e.g. when doing high availability tests on applications, softdog is setup so that the timer is reset by an application thread. In case of the application failing the timer expires and causes a reboot. In such scenarios some information on what caused the failure would be useful and i don't see how softlockup can be used. The patch i had sent would be useful in these cases. If I am missing something please do let me know. I will make the modifications as suggested by Dave Hansen and post the patch shortly.

Anithra.

#3 lore raw
In reply to Anithra P Janakiraman
Quoted 5 lines · Anithra P Janakiraman

We currently have no way of determining the reason for failure when a softdog timeout occurs. At the minimum a snapshot of the system would help to determine the cause.
The attached patch invokes panic on softdog timeout iff kdump is configured, if kdump is not configured it works as usual.

This sounds like a decent idea. But, is it something that should be a bit more optional? We currently have boot options for when to reboot or panic for other things, and this is really the first use of kexec_crash_image outside of kexec itself. Is it really the best switch to use?

Will this break anyone who expects a quick, clean, reboot and instead gets a kdump? Should we make _all_ emergency_restart()s use kdump?

You might have noticed, but your subject is a little wonky. It should probably just omit the 1/1 stuff when you only have a single patch series. The subject is pretty short and doesn't really explain what's going on. Could you beef it up a bit?

Quoted 16 lines
drivers/watchdog/softdog.c +1 -4
@@ -48,6 +48,7 @@
 #include <linux/init.h>
 #include <linux/jiffies.h>
 #include <linux/uaccess.h>
+#include <linux/kexec.h>
 
 #define PFX "SoftDog: "
 

@@ -99,10 +100,15 @@
        if (soft_noboot)
                printk(KERN_CRIT PFX "Triggered - Reboot ignored.\n");
        else {
-               printk(KERN_CRIT PFX "Initiating system reboot.\n");
-               emergency_restart();
-               printk(KERN_CRIT PFX "Reboot didn't ?????\n");
-       }

drivers/watchdog/softdog.c line 112

Quoted 10 lines
drivers/watchdog/softdog.c +9 -0
+               if (kexec_crash_image) {
+                       printk(KERN_CRIT PFX "Initiating kdump. \n");
+                       panic("Watchdog timer expired.");
+               } else {
+                       printk(KERN_CRIT PFX "Initiating system reboot. \n");
+                       emergency_restart();
+                       printk(KERN_CRIT PFX "Reboot didn't ?????\n");
+               }
+             }
 }

The whitespace here is a bit damaged. You might want to double-check what your editor did to it.

Also, it's a bit more conventional to append patches to emails rather than actually attach them.

Please also find some maintainers of this code or people you expect to accept it, and cc them. People are likely to miss this on LKML.

kernel/kexec.c line 938

Quoted 3 lines
kernel/kexec.c +1 -0
 struct kimage *kexec_image;
 struct kimage *kexec_crash_image;
+EXPORT_SYMBOL(kexec_crash_image);

EXPORT_SYMBOL_GPL(), perhaps?

It also isn't _immediately_ obvious why you're doing this. A quick little blurb in the patch description would help.

-- Dave

author
#4 lore raw
Quoted 14 lines · Dave Hansen
Quoted 6 lines · Anithra P Janakiraman

We currently have no way of determining the reason for failure when a softdog timeout occurs. At the minimum a snapshot of the system would help to determine the cause.
The attached patch invokes panic on softdog timeout iff kdump is configured, if kdump is not configured it works as usual.

This sounds like a decent idea. But, is it something that should be a bit more optional? We currently have boot options for when to reboot or panic for other things, and this is really the first use of kexec_crash_image outside of kexec itself. Is it really the best switch to use?

Quoted 10 lines

Will this break anyone who expects a quick, clean, reboot and instead gets a kdump? Should we make _all_ emergency_restart()s use kdump?

You might have noticed, but your subject is a little wonky. It should probably just omit the 1/1 stuff when you only have a single patch series. The subject is pretty short and doesn't really explain what's going on. Could you beef it up a bit?

Thanks for looking at it and for the comments. I've sent a version 2 of the patch that hopefully addresses all your comments.

link:
http://permalink.gmane.org/gmane.linux.kernel/1091282

For some strange reason i'm unable to find a link to my mail on lkml.org. I see emails only upto the 23rd of Jan.

Regards,
Anithra.