Skip to contentSkip to conversation

IDE: Touch NMI watchdog during resume from STR

PATCH In review Michal Schmidt 4 participants 7 messages Jul 13, 2006 – Jul 24, 2006

lore
Share & tools
lore

Michal Schmidt

author
#0 lore raw

PATCH 1/1 IDE: Touch NMI watchdog during resume from STR

When resuming from suspend-to-RAM, the NMI watchdog detects a lockup in ide_wait_not_busy. Here's a screenshot of the trace taken by a digital camera: http://www.uamt.feec.vutbr.cz/rizeni/pom/DSC03510-2.JPG

Let's touch the NMI watchdog in ide_wait_not_busy. The system then resumes correctly from STR.

Signed-off-by Michal Schmidt <[email protected]>
drivers/ide/ide-iops.c +0 -0
index 6571652..77703ac 100644
@@ -23,6 +23,7 @@ #include <linux/delay.h>

#include <linux/hdreg.h>
#include <linux/ide.h>
#include <linux/bitops.h>
+#include <linux/nmi.h>

#include <asm/byteorder.h>
#include <asm/irq.h>
@@ -1243,6 +1244,7 @@ int ide_wait_not_busy(ide_hwif_t *hwif,

		if (stat == 0xff)
			return -ENODEV;
		touch_softlockup_watchdog();
+		touch_nmi_watchdog();
	}
	return -EBUSY;
}
#1 lore raw

Review

Quoted 4 lines · Michal Schmidt

if (stat == 0xff)

(quoted diff) +1 -0
 			return -ENODEV;
 		touch_softlockup_watchdog();
+		touch_nmi_watchdog();

You can remove the touch_softlock_watchdog then. It's implied in touch_nmi_watchdog

-Andi

author
#2 lore raw
Quoted 6 lines · Andi Kleen

Review

Quoted 4 lines · Michal Schmidt

if (stat == 0xff)

(quoted diff) +1 -0
 			return -ENODEV;
 		touch_softlockup_watchdog();
+		touch_nmi_watchdog();

You can remove the touch_softlock_watchdog then. It's implied in touch_nmi_watchdog

I don't think that's always true. There are architectures where touch_nmi_watchdog is a NOP. This is in include/linux/nmi.h:

#ifdef ARCH_HAS_NMI_WATCHDOG
extern void touch_nmi_watchdog(void);
#else

# define touch_nmi_watchdog() do { } while(0)
#endif

Michal

#3 lore raw
Quoted 9 lines · Michal Schmidt
Quoted 7 lines · Andi Kleen

Review

Quoted 4 lines · Michal Schmidt

if (stat == 0xff)

(quoted diff) +1 -0
 			return -ENODEV;
 		touch_softlockup_watchdog();
+		touch_nmi_watchdog();

You can remove the touch_softlock_watchdog then. It's implied in touch_nmi_watchdog

Quoted 8 lines

I don't think that's always true. There are architectures where touch_nmi_watchdog is a NOP. This is in include/linux/nmi.h:

#ifdef ARCH_HAS_NMI_WATCHDOG
extern void touch_nmi_watchdog(void);
#else

# define touch_nmi_watchdog() do { } while(0)
#endif

That's broken code then. It should be defined to touch_softlock_watchdog for the !ARCH_HAS_NMI_WATCHDOG then.

-Andi

author
#4 lore raw

PATCH 1/1 Make touch_nmi_watchdog imply touch_softlockup_watchdog on all archs

touch_nmi_watchdog() calls touch_softlockup_watchdog() on both architectures that implement it (i386 and x86_64). On other architectures it does nothing at all. touch_nmi_watchdog() should imply touch_softlockup_watchdog() on all architectures.
Suggested by Andi Kleen.

Signed-off-by Michal Schmidt <[email protected]>
include/linux/nmi.h +0 -0
index c8f4d2f..e16904e 100644
@@ -4,6 +4,7 @@

#ifndef LINUX_NMI_H
#define LINUX_NMI_H

+#include <linux/sched.h>
#include <asm/irq.h>

/**
@@ -16,7 +17,7 @@ #include <asm/irq.h>
#ifdef ARCH_HAS_NMI_WATCHDOG
extern void touch_nmi_watchdog(void);
#else
-# define touch_nmi_watchdog() do { } while(0)
+# define touch_nmi_watchdog() touch_softlockup_watchdog() #endif

#endif

Acked-by on v1 1/1
#5 lore raw

Re: [PATCH] Make touch_nmi_watchdog imply touch_softlockup_watchdog on all archs

Quoted 7 lines · Michal Schmidt

touch_nmi_watchdog() calls touch_softlockup_watchdog() on both architectures that implement it (i386 and x86_64). On other architectures it does nothing at all. touch_nmi_watchdog() should imply touch_softlockup_watchdog() on all architectures. Suggested by Andi Kleen.

Signed-off-by Michal Schmidt <[email protected]>
Acked-by Ingo Molnar <[email protected]>

although i suspect this could be cleaned up by consolidating both under touch_watchdog(), right?

Ingo

#6 lore raw
In reply to Michal Schmidt

Hi!

Quoted 11 lines

When resuming from suspend-to-RAM, the NMI watchdog detects a lockup in ide_wait_not_busy. Here's a screenshot of the trace taken by a digital camera:
http://www.uamt.feec.vutbr.cz/rizeni/pom/DSC03510-2.JPG

Let's touch the NMI watchdog in ide_wait_not_busy. The system then resumes correctly from STR.

Signed-off-by: Michal Schmidt
<[email protected]>

ACK.

Quoted 14 lines
drivers/ide/ide-iops.c +1 -0
index 6571652..77703ac 100644
@@ -23,6 +23,7 @@ #include <linux/delay.h>
 #include <linux/hdreg.h>
 #include <linux/ide.h>
 #include <linux/bitops.h>
+#include <linux/nmi.h>
 
 #include <asm/byteorder.h>
 #include <asm/irq.h>

drivers/ide/ide-iops.c int ide_wait_not_busy(ide_hwif_t · line 1247

Quoted 4 lines
drivers/ide/ide-iops.c +1 -0
@@ -1243,6 +1244,7 @@ int ide_wait_not_busy(ide_hwif_t 
 *hwif, if (stat == 0xff)
 			return -ENODEV;
 		touch_softlockup_watchdog();
+		touch_nmi_watchdog();

Perhaps we need touch_all_watchdogs()?

Pavel

Signature · 1 line
Thanks for all the (sleeping) penguins.