Re: [PATCH] NMI notifiers for 2.5

Re: [PATCH] NMI notifiers for 2.5

[Andi Kleen]

Stephen Hemminger <shemminger@osdl.org> writes:

> The following generalizes the NMI callback's needed by things like crash
> dump and debuggers in the same way that panic has notifiers. 
> 
> Please apply this since it makes writing and maintaining RAS extensions
> easier. Since there is already a panic_notifier callback, this follows
> the same model. 

> +			
> +			notifier_call_chain(&nmi_notifier_list, 0, regs);
> +

Most debuggers/crash dumpers etc. need a way to veto normal processing of NMIs 
and other exceptions. For NMI the usual case is to turn off the nmi watchdog 
while you do something slow with interrupts disabled, that requires
doing the hook very early. Without veta NMI notification is not very useful.

You want something like:

	if (notifier_call_chain(&nmi_notifier_list, 0, regs) == NOTIFY_BAD)
		goto ignore;

For a more comprehensive variant see include/asm-x86_64/kdebug.h	
The x86-64 variant cannot be 1:1 copied because it's still incomplete
and e.g. does not implement veto for all places where it's needed.


-Andi

Re: [PATCH] NMI notifiers for 2.5

[Stephen Hemminger]

> For a more comprehensive variant see include/asm-x86_64/kdebug.h	
> The x86-64 variant cannot be 1:1 copied because it's still incomplete
> and e.g. does not implement veto for all places where it's needed.
> 

Didn't look in x86_64 code.  Would it just make more sense to turn this
into an architecture independent mechanism and provide sample versions
for x86_64 and i386?

Re: [PATCH] NMI notifiers for 2.5

[Andi Kleen]

On Wed, Dec 04, 2002 at 01:54:13PM -0800, Stephen Hemminger wrote:
> 
> > For a more comprehensive variant see include/asm-x86_64/kdebug.h	
> > The x86-64 variant cannot be 1:1 copied because it's still incomplete
> > and e.g. does not implement veto for all places where it's needed.
> > 
> 
> Didn't look in x86_64 code.  Would it just make more sense to turn this
> into an architecture independent mechanism and provide sample versions
> for x86_64 and i386?

Would seem like overkill to me.

notifiers are already architecture independent, that should be enough.

My experience so far is that one has to be very careful how to design
such hooks and the first versions of it usually don't survive 
the actual implementation of an debugger or crash dumper.

-Andi

RE: [PATCH] NMI notifiers for 2.5

[Stephen Hemminger]

On Wed, 2002-12-04 at 11:20, Larry Sendlosky wrote:
> Why not just use the panic notifier callback? After all, isn't
> an "NMI timeout" just another type of panic? Is there code
> that needs to take different action on an NMI timeout than a panic?
> 
> larry

That is a possibility but the arguments available are different.
panic has the string and NMI has the registers available.

Could also include oops handler as well.

RE: [PATCH] NMI notifiers for 2.5

[Larry Sendlosky]

Why not just use the panic notifier callback? After all, isn't
an "NMI timeout" just another type of panic? Is there code
that needs to take different action on an NMI timeout than a panic?

larry

-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@osdl.org]
Sent: Wednesday, December 04, 2002 1:39 PM
To: Linus Torvalds
Cc: Kernel List
Subject: [PATCH] NMI notifiers for 2.5


The following generalizes the NMI callback's needed by things like crash
dump and debuggers in the same way that panic has notifiers. 

Please apply this since it makes writing and maintaining RAS extensions
easier. Since there is already a panic_notifier callback, this follows
the same model. 

For architectures without NMI, it is a nop 

diff -urN -X dontdiff -x drivers/dump linux-2.5/include/linux/nmi.h linux-2.5-lkcd/include/linux/nmi.h
--- linux-2.5/include/linux/nmi.h	2002-12-02 09:38:58.000000000 -0800
+++ linux-2.5-lkcd/include/linux/nmi.h	2002-12-02 09:20:42.000000000 -0800
@@ -4,6 +4,8 @@
 #ifndef LINUX_NMI_H
 #define LINUX_NMI_H
 
+#include <linux/notifier.h>
+
 #include <asm/irq.h>
 
 /**
@@ -14,9 +16,28 @@
  * disables interrupts for a long time. This call is stateless.
  */
 #ifdef ARCH_HAS_NMI_WATCHDOG
+
+/* 
+ * Registration for maintance routines that want to do something
+ * on NMI.
+ */
+extern struct notifier_block *nmi_notifier_list;
+static inline int register_nmi_notifier(struct notifier_block *nb) {
+	return notifier_chain_register(&nmi_notifier_list, nb);
+}
+
+static inline int unregister_nmi_notifier(struct notifier_block * nb)
+{
+	return notifier_chain_unregister(&nmi_notifier_list, nb);
+}
+
 extern void touch_nmi_watchdog(void);
+
 #else
-# define touch_nmi_watchdog() do { } while(0)
+# define touch_nmi_watchdog() 		do { } while(0)
+# define register_nmi_notifier(x)	do { } while(0)
+# define unregister_nmi_notifier(x)	do { } while(0)
+
 #endif
 
 #endif
diff -urN -X dontdiff -x drivers/dump linux-2.5/arch/i386/kernel/nmi.c linux-2.5-lkcd/arch/i386/kernel/nmi.c
--- linux-2.5/arch/i386/kernel/nmi.c	2002-12-02 09:38:13.000000000 -0800
+++ linux-2.5-lkcd/arch/i386/kernel/nmi.c	2002-12-02 09:20:38.000000000 -0800
@@ -20,6 +20,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/nmi.h>
 
 #include <asm/smp.h>
 #include <asm/mtrr.h>
@@ -29,6 +30,8 @@
 static unsigned int nmi_hz = HZ;
 unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
 extern void show_registers(struct pt_regs *regs);
+struct notifier_block *nmi_notifier_list;
+
 
 #define K7_EVNTSEL_ENABLE	(1 << 22)
 #define K7_EVNTSEL_INT		(1 << 20)
@@ -377,6 +380,9 @@
 			bust_spinlocks(1);
 			printk("NMI Watchdog detected LOCKUP on CPU%d, eip %08lx, registers:\n", cpu, regs->eip);
 			show_registers(regs);
+			
+			notifier_call_chain(&nmi_notifier_list, 0, regs);
+
 			printk("console shuts up ...\n");
 			console_silent();
 			spin_unlock(&nmi_print_lock);
@@ -387,6 +393,7 @@
 		last_irq_sums[cpu] = sum;
 		alert_counter[cpu] = 0;
 	}
+
 	if (nmi_perfctr_msr) {
 		if (nmi_perfctr_msr == MSR_P4_IQ_COUNTER0) {
 			/*


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

Re: [PATCH] NMI notifiers for 2.5

[John Levon]

On Wed, Dec 04, 2002 at 10:39:02AM -0800, Stephen Hemminger wrote:

> +/* 
> + * Registration for maintance routines that want to do something
> + * on NMI.
> + */
> +extern struct notifier_block *nmi_notifier_list;
> +static inline int register_nmi_notifier(struct notifier_block *nb) {
> +	return notifier_chain_register(&nmi_notifier_list, nb);
> +}

Easy way to confuse the hell out of people...

> +			
> +			notifier_call_chain(&nmi_notifier_list, 0, regs);
> +

... because it's really a lockup detect notifier. Please rethink the
naming

(oh and maintenance not maintance)

regards
john

-- 
"Trolls like content too."
	- Bob Abooey, /.

[PATCH] NMI notifiers for 2.5

[Stephen Hemminger]

The following generalizes the NMI callback's needed by things like crash
dump and debuggers in the same way that panic has notifiers. 

Please apply this since it makes writing and maintaining RAS extensions
easier. Since there is already a panic_notifier callback, this follows
the same model. 

For architectures without NMI, it is a nop 

diff -urN -X dontdiff -x drivers/dump linux-2.5/include/linux/nmi.h linux-2.5-lkcd/include/linux/nmi.h
--- linux-2.5/include/linux/nmi.h	2002-12-02 09:38:58.000000000 -0800
+++ linux-2.5-lkcd/include/linux/nmi.h	2002-12-02 09:20:42.000000000 -0800
@@ -4,6 +4,8 @@
 #ifndef LINUX_NMI_H
 #define LINUX_NMI_H
 
+#include <linux/notifier.h>
+
 #include <asm/irq.h>
 
 /**
@@ -14,9 +16,28 @@
  * disables interrupts for a long time. This call is stateless.
  */
 #ifdef ARCH_HAS_NMI_WATCHDOG
+
+/* 
+ * Registration for maintance routines that want to do something
+ * on NMI.
+ */
+extern struct notifier_block *nmi_notifier_list;
+static inline int register_nmi_notifier(struct notifier_block *nb) {
+	return notifier_chain_register(&nmi_notifier_list, nb);
+}
+
+static inline int unregister_nmi_notifier(struct notifier_block * nb)
+{
+	return notifier_chain_unregister(&nmi_notifier_list, nb);
+}
+
 extern void touch_nmi_watchdog(void);
+
 #else
-# define touch_nmi_watchdog() do { } while(0)
+# define touch_nmi_watchdog() 		do { } while(0)
+# define register_nmi_notifier(x)	do { } while(0)
+# define unregister_nmi_notifier(x)	do { } while(0)
+
 #endif
 
 #endif
diff -urN -X dontdiff -x drivers/dump linux-2.5/arch/i386/kernel/nmi.c linux-2.5-lkcd/arch/i386/kernel/nmi.c
--- linux-2.5/arch/i386/kernel/nmi.c	2002-12-02 09:38:13.000000000 -0800
+++ linux-2.5-lkcd/arch/i386/kernel/nmi.c	2002-12-02 09:20:38.000000000 -0800
@@ -20,6 +20,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <linux/kernel_stat.h>
+#include <linux/nmi.h>
 
 #include <asm/smp.h>
 #include <asm/mtrr.h>
@@ -29,6 +30,8 @@
 static unsigned int nmi_hz = HZ;
 unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
 extern void show_registers(struct pt_regs *regs);
+struct notifier_block *nmi_notifier_list;
+
 
 #define K7_EVNTSEL_ENABLE	(1 << 22)
 #define K7_EVNTSEL_INT		(1 << 20)
@@ -377,6 +380,9 @@
 			bust_spinlocks(1);
 			printk("NMI Watchdog detected LOCKUP on CPU%d, eip %08lx, registers:\n", cpu, regs->eip);
 			show_registers(regs);
+			
+			notifier_call_chain(&nmi_notifier_list, 0, regs);
+
 			printk("console shuts up ...\n");
 			console_silent();
 			spin_unlock(&nmi_print_lock);
@@ -387,6 +393,7 @@
 		last_irq_sums[cpu] = sum;
 		alert_counter[cpu] = 0;
 	}
+
 	if (nmi_perfctr_msr) {
 		if (nmi_perfctr_msr == MSR_P4_IQ_COUNTER0) {
 			/*