[patch] IDE: Touch NMI watchdog during resume from STR

[patch] IDE: Touch NMI watchdog during resume from STR

[Michal Schmidt]

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 <xschmi00@stud.feec.vutbr.cz>

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 6571652..77703ac 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -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;
 }

Re: [patch] IDE: Touch NMI watchdog during resume from STR

[Andi Kleen]

Michal Schmidt <xschmi00@stud.feec.vutbr.cz> writes:
> if (stat == 0xff)
>  			return -ENODEV;
>  		touch_softlockup_watchdog();
> +		touch_nmi_watchdog();
You can remove the touch_softlock_watchdog then. It's implied in 
touch_nmi_watchdog

-Andi

Re: [patch] IDE: Touch NMI watchdog during resume from STR

[Michal Schmidt]

Andi Kleen wrote:
> Michal Schmidt <xschmi00@stud.feec.vutbr.cz> writes:
>> if (stat == 0xff)
>>  			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

Re: [patch] IDE: Touch NMI watchdog during resume from STR

[Andi Kleen]

On Thursday 13 July 2006 19:33, Michal Schmidt wrote:
> Andi Kleen wrote:
> > Michal Schmidt <xschmi00@stud.feec.vutbr.cz> writes:
> >> if (stat == 0xff)
> >>  			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

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

-Andi

Re: [patch] IDE: Touch NMI watchdog during resume from STR

[Pavel Machek]

Hi!

> 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 
> <xschmi00@stud.feec.vutbr.cz>

ACK.

> diff --git a/drivers/ide/ide-iops.c 
> b/drivers/ide/ide-iops.c
> index 6571652..77703ac 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -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();

Perhaps we need touch_all_watchdogs()?

						Pavel

-- 
Thanks for all the (sleeping) penguins.

[PATCH] Make touch_nmi_watchdog imply touch_softlockup_watchdog on all archs

[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 <xschmi00@stud.feec.vutbr.cz>

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index c8f4d2f..e16904e 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -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

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

[Ingo Molnar]

* Michal Schmidt <xschmi00@stud.feec.vutbr.cz> wrote:

> 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 <xschmi00@stud.feec.vutbr.cz>

Acked-by: Ingo Molnar <mingo@elte.hu>

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

	Ingo