x86: nmi - fix incorrect NMI watchdog used by default

PATCH Applied Cyrill Gorcunov 3 participants 7 messages May 28, 2008 – May 31, 2008 x86

lore

Share & tools

lore

Cyrill Gorcunov

author

2008-05-28 19:01 UTC #0 lore raw

PATCH 1/1 x86: nmi - fix incorrect NMI watchdog used by default

The commit

	commit 4b82b277707a39b97271439c475f186f63ec4692
	Author: Cyrill Gorcunov <[email protected]>
	Date:   Sat May 24 19:36:35 2008 +0400

set nmi_watchdog to NMI_IO_APIC as by default. This causes hangs on some machines with buggy watchdogs. Fix it - i.e. restore old behaviour.

Thanks to Sitsofe Wheeler and Adrian Bunk for catching the problem and Maciej W. Rozycki for explanation what is going on there.

Signed-off-by Cyrill Gorcunov <[email protected]>

CC Maciej W. Rozycki <[email protected]>

---

The patch is over today tip/x86/nmi

arch/x86/kernel/nmi.c +9 -7

Index: linux-2.6.git/arch/x86/kernel/nmi.c
====================================================================
@@ -84,20 +84,15 @@ static inline unsigned int get_timer_irq
 #endif
 }
 
+#ifdef CONFIG_X86_64
 /* Run after command line and cpu_init init, but before all other checks */
 void nmi_watchdog_default(void)
 {
 	if (nmi_watchdog != NMI_DEFAULT)
 		return;
-#ifdef CONFIG_X86_64
 	nmi_watchdog = NMI_NONE;
-#else
-	if (lapic_watchdog_ok())
-		nmi_watchdog = NMI_LOCAL_APIC;
-	else
-		nmi_watchdog = NMI_IO_APIC;
-#endif
 }
+#endif
 
 #ifdef CONFIG_SMP
 /*

@@ -488,8 +483,15 @@ int proc_nmi_enabled(struct ctl_table *t
 		return -EIO;
 	}
 
+#ifdef CONFIG_X86_64
 	/* if nmi_watchdog is not set yet, then set it */
 	nmi_watchdog_default();
+#else
+	if (lapic_watchdog_ok())
+		nmi_watchdog = NMI_LOCAL_APIC;
+	else
+		nmi_watchdog = NMI_IO_APIC;
+#endif
 
 	if (nmi_watchdog == NMI_LOCAL_APIC) {
 		if (nmi_watchdog_enabled)

include/asm-x86/nmi.h +3 -1

Index: linux-2.6.git/include/asm-x86/nmi.h
====================================================================
@@ -38,9 +38,11 @@ static inline void unset_nmi_pm_callback
 
 #ifdef CONFIG_X86_64
 extern void default_do_nmi(struct pt_regs *);
+extern void nmi_watchdog_default(void);
+#else
+#define nmi_watchdog_default(void) do {} while (0)
 #endif
 
-extern void nmi_watchdog_default(void);
 extern void die_nmi(char *str, struct pt_regs *regs, int do_panic);
 extern int check_nmi_watchdog(void);
 extern int nmi_watchdog_enabled;

Ingo Molnar

Applied

2008-05-28 19:12 UTC #1 lore raw

Quoted 6 lines · Cyrill Gorcunov

The commit

	commit 4b82b277707a39b97271439c475f186f63ec4692
	Author: Cyrill Gorcunov <[email protected]>
	Date:   Sat May 24 19:36:35 2008 +0400

Quoted 6 lines

set nmi_watchdog to NMI_IO_APIC as by default. This causes hangs on some machines with buggy watchdogs. Fix it - i.e. restore old behaviour.
Thanks to Sitsofe Wheeler and Adrian Bunk for catching the problem and Maciej W. Rozycki for explanation what is going on there.

applied, thanks Cyrill.

Ingo

Ingo Molnar

2008-05-30 12:54 UTC #2 lore raw

In reply to Cyrill Gorcunov

include/asm-x86/nmi.h static inline void unset_nmi_pm_callback · line 43

Quoted 3 lines · Cyrill Gorcunov

include/asm-x86/nmi.h +3 -0

+extern void nmi_watchdog_default(void);
+#else
+#define nmi_watchdog_default(void) do {} while (0)

                                  ^------

that doesnt work too well in a macro ... :-)

Ingo

Cyrill Gorcunov

author

2008-05-30 14:45 UTC #3 lore raw

[Ingo Molnar - Fri, May 30, 2008 at 02:54:00PM +0200]

| 
| * Cyrill Gorcunov <[email protected]> wrote:
| 
| > +extern void nmi_watchdog_default(void);
| > +#else
| > +#define nmi_watchdog_default(void) do {} while (0)
|                                   ^------
| 
| that doesnt work too well in a macro ... :-)
| 
| 	Ingo
|

Oh my.... maybe two prerocessor passes would help? ;-) here is an updated version, please reapply, and thanks a lot for catching this Ingo! Sorry for inconvenience.

- Cyrill -
---

Subject: [PATCH] x86: nmi - fix incorrect NMI watchdog used by default

Commit

commit 4b82b277707a39b97271439c475f186f63ec4692

Author: Cyrill Gorcunov <[email protected]>
Date:   Sat May 24 19:36:35 2008 +0400

sets nmi_watchdog to NMI_IO_APIC as by default value even not being asking to. That causes hangs on some machines with buggy watchdogs. Fix it - i.e. restore old behaviour.

Thanks to Sitsofe Wheeler and Adrian Bunk for catching the problem and Maciej W. Rozycki for explanation what is going on there.

Signed-off-by Cyrill Gorcunov <[email protected]>

CC Maciej W. Rozycki <[email protected]>

---

arch/x86/kernel/nmi.c +9 -7

Index: linux-2.6.git/arch/x86/kernel/nmi.c
====================================================================
@@ -84,20 +84,15 @@ static inline unsigned int get_timer_irq
 #endif
 }
 
+#ifdef CONFIG_X86_64
 /* Run after command line and cpu_init init, but before all other checks */
 void nmi_watchdog_default(void)
 {
 	if (nmi_watchdog != NMI_DEFAULT)
 		return;
-#ifdef CONFIG_X86_64
 	nmi_watchdog = NMI_NONE;
-#else
-	if (lapic_watchdog_ok())
-		nmi_watchdog = NMI_LOCAL_APIC;
-	else
-		nmi_watchdog = NMI_IO_APIC;
-#endif
 }
+#endif
 
 #ifdef CONFIG_SMP
 /*

@@ -488,8 +483,15 @@ int proc_nmi_enabled(struct ctl_table *t
 		return -EIO;
 	}
 
+#ifdef CONFIG_X86_64
 	/* if nmi_watchdog is not set yet, then set it */
 	nmi_watchdog_default();
+#else
+	if (lapic_watchdog_ok())
+		nmi_watchdog = NMI_LOCAL_APIC;
+	else
+		nmi_watchdog = NMI_IO_APIC;
+#endif
 
 	if (nmi_watchdog == NMI_LOCAL_APIC) {
 		if (nmi_watchdog_enabled)

include/asm-x86/nmi.h +3 -1

Index: linux-2.6.git/include/asm-x86/nmi.h
====================================================================
@@ -38,9 +38,11 @@ static inline void unset_nmi_pm_callback
 
 #ifdef CONFIG_X86_64
 extern void default_do_nmi(struct pt_regs *);
+extern void nmi_watchdog_default(void);
+#else
+#define nmi_watchdog_default() do {} while (0)
 #endif
 
-extern void nmi_watchdog_default(void);
 extern void die_nmi(char *str, struct pt_regs *regs, int do_panic);
 extern int check_nmi_watchdog(void);
 extern int nmi_watchdog_enabled;

Ingo Molnar

2008-05-30 15:10 UTC #4 lore raw

Quoted 9 lines · Cyrill Gorcunov

| > +#else
| > +#define nmi_watchdog_default(void) do {} while (0)
|                                   ^------
| 
| that doesnt work too well in a macro ... :-)
| 
| 	Ingo
|

Quoted 3 lines

Oh my.... maybe two prerocessor passes would help? ;-) here is an updated version, please reapply, and thanks a lot for catching this Ingo! Sorry for inconvenience.

no problem, already fixed in latest -tip :)

Ingo

Sitsofe Wheeler

2008-05-31 07:45 UTC #5 lore raw

In reply to Cyrill Gorcunov

Quoted 8 lines · Cyrill Gorcunov

The commit
commit 4b82b277707a39b97271439c475f186f63ec4692
Author: Cyrill Gorcunov <[email protected]>
Date:   Sat May 24 19:36:35 2008 +0400
set nmi_watchdog to NMI_IO_APIC as by default. This causes hangs on some machines with buggy watchdogs. Fix it - i.e. restore old behaviour.

Thanks for fixing this. Just for the record, the problem also seemed to depend on the ACPI Video option. Removing that also stopped the problem... However I can confirm that I don't have a boot hang with next-0530.

Signature · 1 line

Sitsofe | http://sucs.org/~sits/

Cyrill Gorcunov

author

2008-05-31 09:17 UTC #6 lore raw

[Sitsofe Wheeler - Sat, May 31, 2008 at 08:42:06AM +0100]

| Cyrill Gorcunov wrote:
| 
| > The commit
| > 
| > commit 4b82b277707a39b97271439c475f186f63ec4692
| > Author: Cyrill Gorcunov <[email protected]>
| > Date:   Sat May 24 19:36:35 2008 +0400
| > 
| > set nmi_watchdog to NMI_IO_APIC as by default. This causes hangs on some
| > machines with buggy watchdogs. Fix it - i.e. restore old behaviour.
| 
| Thanks for fixing this. Just for the record, the problem also seemed to
| depend on the ACPI Video option. Removing that also stopped the problem...
| However I can confirm that I don't have a boot hang with next-0530.
| 
| -- 
| Sitsofe | http://sucs.org/~sits/
|

ok, thanks for testing and additional info

- Cyrill -