Re: [PATCH for review] [13/145] x86_64: Add abilty to enable/disablenmi watchdog with sysctl

[Oleg Verych]

Andi Kleen wrote:
> r
>
> From: dzickus <dzickus@xxxxxxxxxx>
>
> Adds a new /proc/sys/kernel/nmi call that will enable/disable the nmi
> watchdog.
>
> Signed-off-by:  Don Zickus <dzickus@xxxxxxxxxx>
> Signed-off-by: Andi Kleen <ak@xxxxxxx>
>
> ---
>  arch/i386/kernel/nmi.c   |   52 +++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86_64/kernel/nmi.c |   48 +++++++++++++++++++++++++++++++++++++++++++
>  include/asm-i386/nmi.h   |    1 
>  include/asm-x86_64/nmi.h |    1 
>  include/linux/sysctl.h   |    1 
>  kernel/sysctl.c          |   11 +++++++++
>  6 files changed, 114 insertions(+)

Hallo, Andi Kleen.

I'm a kernelnewbie, so any *answer* to this will be very appreciated.

Files 'nmi.c' from both archs don't match, obviously. But lets see, how.

cd /tmp/
diff -purN /tmp/ia32.c /tmp/amd64.c
--- /tmp/ia32.c	2006-08-10 21:12:19.292953750 +0200
+++ /tmp/amd64.c	2006-08-10 21:11:49.503092000 +0200
@@ -10,16 +10,12 @@
 +		return 0;
 +
 +	if (atomic_read(&nmi_active) < 0) {
-+		printk(KERN_WARNING "NMI watchdog is permanently disabled\n");
++		printk( KERN_WARNING "NMI watchdog is permanently disabled\n");
 +		return -EINVAL;

Something is wrong;

-+	if (nmi_watchdog == NMI_DEFAULT) {
-+		if (nmi_known_cpu() > 0)
-+			nmi_watchdog = NMI_LOCAL_APIC;
-+		else
-+			nmi_watchdog = NMI_IO_APIC;
-+	}
++	/* if nmi_watchdog is not set yet, then set it */
++	nmi_watchdog_default();

i don't know about nmi, but please drop a word why this is different in both files;

 +
 +	if (nmi_watchdog == NMI_LOCAL_APIC)
 +	{
@@ -32,7 +28,7 @@
 +		 * for some reason these functions don't work
 +		 */
 +		printk("Can not enable/disable NMI on IO APIC\n");
-+		return -EINVAL;
++		return -EIO;

and this;

 +#if 0
 +		if (nmi_watchdog_enabled)
 +			enable_timer_nmi_watchdog();
@@ -40,7 +36,7 @@
 +			disable_timer_nmi_watchdog();
 +#endif
 +	} else {
-+		printk( KERN_WARNING
++		printk(KERN_WARNING

dup.

 +			"NMI watchdog doesn't know what hardware to touch\n");
 +		return -EIO;
 +	}

Maybe this must be one file for both archs ?

Thanks.

--
-o--=O`C
 #oo'L O
<___=E M

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