Skip to contentSkip to conversation

disable NMI watchdog by default

PATCH In review Ingo Molnar 6 participants 8 messages Mar 5, 2007 – Mar 7, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 disable NMI watchdog by default

Linus,

Andrew sent the patch below (which is now months old and has been in -mm for some time) towards Andi's tree 4 weeks ago, but apparently it fell into a black hole there - the patch is still not upstream!

This is a must-have for v2.6.21, because the problem still triggers even with the latest upstream tree, if i boot a KVM guest.

Frankly, i find it ridiculous that i had to write more than 10 emails about this stupid topic already, while i'm the original author of this feature to begin with. The "do not enable by default debug features that break certain systems" concept is obvious to me.

This category of regressions has been introduced by Andi when he made the NMI watchdog the default in certain scenarios at around v2.6.18 - over my repeated objections. Andi Cc:-ed :-)

Ingo

----- Forwarded message from Ingo Molnar <[email protected]> -----

From: Ingo Molnar <[email protected]>
Subject: [patch] disable NMI watchdog by default

there's a new NMI watchdog related problem: KVM crashes on certain bzImages because ... we enable the NMI watchdog by default (even if the user does not ask for it) , and no other OS on this planet does that so KVM doesnt have emulation for that yet. So KVM injects a #GP, which crashes the Linux guest:

 general protection fault: 0000 [#1]
 PREEMPT SMP
 Modules linked in:
 CPU:    0
 EIP:    0060:[<c011a8ae>]    Not tainted VLI
 EFLAGS: 00000246   (2.6.20-rc5-rt0 #3)
 EIP is at setup_apic_nmi_watchdog+0x26d/0x3d3

and no, i did /not/ request an nmi_watchdog on the boot command line!

Solution: turn off that darn thing! It's a debug tool, not a 'make life harder' tool!!

with this patch the KVM guest boots up just fine.

And with this my laptop (Lenovo T60) also stopped its sporadic hard hanging (sometimes in acpi_init(), sometimes later during bootup, sometimes much later during actual use) as well. It hung with both nmi_watchdog=1 and nmi_watchdog=2, so it's generally the fact of NMI injection that is causing problems, not the NMI watchdog variant, nor any particular bootup code.

The patch is unintrusive.

include/asm-i386/nmi.h +1 -1
Index: linux/include/asm-i386/nmi.h
===================================================================
@@ -33,7 +33,7 @@ extern int nmi_watchdog_tick (struct pt_
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT     -1
+#define NMI_DEFAULT     0
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2
include/asm-x86_64/nmi.h +1 -1
Index: linux/include/asm-x86_64/nmi.h
===================================================================
@@ -63,7 +63,7 @@ extern int setup_nmi_watchdog(char *);
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT	-1
+#define NMI_DEFAULT	0
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2
#1 lore raw
Quoted 8 lines · Ingo Molnar

Linus,

Andrew sent the patch below (which is now months old and has been in -mm for some time) towards Andi's tree 4 weeks ago, but apparently it fell into a black hole there - the patch is still not upstream!

This is a must-have for v2.6.21

I agree.
There are multiple machines not booting because of nmi_watchdog. Some of them are documented here:
http://bugzilla.kernel.org/show_bug.cgi?id=7839

We used to think this was the "nolapic" bug,
but it is actually the "nmi_watchdog=0" bug.

-Len

Quoted 81 lines

Frankly, i find it ridiculous that i had to write more than 10 emails about this stupid topic already, while i'm the original author of this feature to begin with. The "do not enable by default debug features that break certain systems" concept is obvious to me.

This category of regressions has been introduced by Andi when he made the NMI watchdog the default in certain scenarios at around v2.6.18 - over my repeated objections. Andi Cc:-ed :-)

Ingo

----- Forwarded message from Ingo Molnar <[email protected]> -----

From: Ingo Molnar <[email protected]>
Subject: [patch] disable NMI watchdog by default

there's a new NMI watchdog related problem: KVM crashes on certain bzImages because ... we enable the NMI watchdog by default (even if the user does not ask for it) , and no other OS on this planet does that so KVM doesnt have emulation for that yet. So KVM injects a #GP, which crashes the Linux guest:

 general protection fault: 0000 [#1]
 PREEMPT SMP
 Modules linked in:
 CPU:    0
 EIP:    0060:[<c011a8ae>]    Not tainted VLI
 EFLAGS: 00000246   (2.6.20-rc5-rt0 #3)
 EIP is at setup_apic_nmi_watchdog+0x26d/0x3d3

and no, i did /not/ request an nmi_watchdog on the boot command line!

Solution: turn off that darn thing! It's a debug tool, not a 'make life harder' tool!!

with this patch the KVM guest boots up just fine.

And with this my laptop (Lenovo T60) also stopped its sporadic hard hanging (sometimes in acpi_init(), sometimes later during bootup, sometimes much later during actual use) as well. It hung with both nmi_watchdog=1 and nmi_watchdog=2, so it's generally the fact of NMI injection that is causing problems, not the NMI watchdog variant, nor any particular bootup code.

The patch is unintrusive.

Index: linux/include/asm-i386/nmi.h
===================================================================

include/asm-i386/nmi.h +1 -1
@@ -33,7 +33,7 @@ extern int nmi_watchdog_tick (struct pt_
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT     -1
+#define NMI_DEFAULT     0
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2

Index: linux/include/asm-x86_64/nmi.h
===================================================================

include/asm-x86_64/nmi.h +1 -2
@@ -63,7 +63,7 @@ extern int setup_nmi_watchdog(char *);
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT	-1
+#define NMI_DEFAULT	0
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2
 
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
Quoted 10 lines · Len Brown
Quoted 8 lines · Ingo Molnar

Linus,

Andrew sent the patch below (which is now months old and has been in -mm for some time) towards Andi's tree 4 weeks ago, but apparently it fell into a black hole there - the patch is still not upstream!

This is a must-have for v2.6.21

Quoted 7 lines

I agree.
There are multiple machines not booting because of nmi_watchdog. Some of them are documented here:
http://bugzilla.kernel.org/show_bug.cgi?id=7839

We used to think this was the "nolapic" bug,
but it is actually the "nmi_watchdog=0" bug.

I thought that one was worked around by Ingo's patch to not do nmi watchdog during ACPI methods, wasn't it?

-Andi

#3 lore raw
Quoted 9 lines · Andi Kleen
Quoted 6 lines

There are multiple machines not booting because of nmi_watchdog. Some of them are documented here:
http://bugzilla.kernel.org/show_bug.cgi?id=7839

We used to think this was the "nolapic" bug,
but it is actually the "nmi_watchdog=0" bug.

I thought that one was worked around by Ingo's patch to not do nmi watchdog during ACPI methods, wasn't it?

unfortunately that only made the lockups on my laptop rarer, it didnt totally solve it. My workaround was only done for init acpi methods (bootup) - it was getting really ugly when i tried to extend it to all ACPI execution. I'd guess the situation on those other systems is similar.

Maybe we could take only the 32-bit side of my patch, because that's what is most affected by legacies. Although i suspect Windows still doesnt inject NMIs in 64-bit mode either, so i dont think there's any fundamental difference in terms of breakage in the future, it's just that 64-bit systems and 64-bit testing is 1:5 - 1:10 rarer than 32-bit testing.

dunno. A distro can still patch the NMI watchdog on, easily. If you think it's a better approach i can make this a .config option - just like CONFIG_DETECT_SOFTLOCKUP: CONFIG_DETECT_HARDLOCKUP, which would default to off?

Ingo

#4 lore raw
Quoted 7 lines · Ingo Molnar

Maybe we could take only the 32-bit side of my patch, because that's what is most affected by legacies. Although i suspect Windows still doesnt inject NMIs in 64-bit mode either, so i dont think there's any fundamental difference in terms of breakage in the future, it's just that 64-bit systems and 64-bit testing is 1:5 - 1:10 rarer than 32-bit testing.

I really don't see the point of having NMI on by default. Debugging that can cause problems should be disabled. I thought we disabled this a long time ago - but anyway, now it *really* is (ie I applied your patch).

Anybody who thinks that debugging code is always good is deluded. Debugging is only good if it has zero downsides and doesn't introduce problems of its own. And the NMI watchdog clearly isn't in that camp.

If you *actively* debug stuff and/or have actually seen lockups, use the NMI watchdog. But last I saw, if the machine was running X, the NMI watchdog wouldn't help *anyway*, so enabling it by default

- has almost zero upsides in the wild _anyway_

- clearly has risks.

so this was a no-brainer. I don't understand why it's even discussed, or why I hadn't gotten the patch the last time this was discussed.

Linus

Acked-by on v1 1/1
#5 lore raw
In reply to Ingo Molnar
Quoted 5 lines

Frankly, i find it ridiculous that i had to write more than 10 emails about this stupid topic already, while i'm the original author of this feature to begin with. The "do not enable by default debug features that break certain systems" concept is obvious to me.

NMI breaks on some systems, esp in combination with SMM so

64 bit

#6 lore raw
In reply to Ingo Molnar

include/asm-x86_64/nmi.h extern int setup_nmi_watchdog(char *); · line 66

Quoted 8 lines
include/asm-x86_64/nmi.h +1 -1
@@ -63,7 +63,7 @@ extern int setup_nmi_watchdog(char *);
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT	-1
+#define NMI_DEFAULT	0

Maybe I'm missing something obvious, but this patch doesn't seem correct to me. The sentiment of disabling the NMI watchdog by default is fine, and I agree with it, but I don't think this patch does what it says. First of all, I have a system running a kernel with this patch applied (v2.6.21-rc2-gc3442e2), and I see NMIs in /proc/interrupts and "testing NMI watchdog ... OK." in the log.

And second, looking at the NMI code, it seems that this change actually makes it impossible to turn off the NMI watchdog! In arch/x86_64/kernel/nmi.c, we have:

void nmi_watchdog_default(void)

{
	if (nmi_watchdog != NMI_DEFAULT)
		return;
	if (nmi_known_cpu())
		nmi_watchdog = NMI_LOCAL_APIC;
	else
		nmi_watchdog = NMI_IO_APIC;
}

so it seems changing the value of NMI_DEFAULT has no effect on this logic, really: if nmi_watchdog is left at the default, then the kernel chooses LAPIC or IO-APIC. And if someone passes "nmi_watchdog=0" on the command line, nmi_watchdog is still NMI_DEFAULT and so the same logic triggers.

Ingo, I assume you tested this, so what am I missing?

- R.

#7 lore raw
Quoted 4 lines · Roland Dreier
Quoted 4 lines
include/asm-x86_64/nmi.h +0 -0
@@ -63,7 +63,7 @@ extern int setup_nmi_watchdog(char *);
 
Quoted 11 lines

include/asm-x86_64/nmi.h extern int setup_nmi_watchdog(char *); · line 66

Quoted 4 lines
include/asm-x86_64/nmi.h +1 -1
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT	-1
+#define NMI_DEFAULT	0

Maybe I'm missing something obvious, but this patch doesn't seem correct to me. The sentiment of disabling the NMI watchdog by default is fine, and I agree with it, but I don't think this patch does what it says. First of all, I have a system running a kernel with this patch applied (v2.6.21-rc2-gc3442e2), and I see NMIs in /proc/interrupts and "testing NMI watchdog ... OK." in the log.

Yes the patch looks quite broken.

-Andi