NMI handling rework for x86
Linus,
John Levon suggested I send this to you. It's a cleanup of the NMI handling to make it into a request/release mechanism (instead of hard-coding everything into traps.c). It renames "nmi.c" to "nmi_watchdog.c" (as it should be named) and moves the real NMI handling code from traps.c to nmi.c. It's been posted and reworked on lkml, and it seems to have finally met approval. The "cc-ed" people have reviewed the patch (or at least made helpful suggestions :-).
Since a lot of things are hacking into this code (lkcd, kdb, oprofile, nmi watchdog, and now my IPMI watchdog pretimeout), it would be very nice to get their junk out of this code and allow them to bind in nicely, and allow binding from modules.
(And this time it's a -p1 diff)
Thanks,
-Corey
arch/i386/kernel/Makefile +2 -2
@@ -8,7 +8,7 @@ obj-y := process.o semaphore.o signal.o entry.o traps.o irq.o vm86.o \ ptrace.o i8259.o ioport.o ldt.o setup.o time.o sys_i386.o \ - pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o + pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o nmi.o obj-y += cpu/ obj-y += timers/
@@ -22,7 +22,7 @@ obj-$(CONFIG_ACPI_SLEEP) += acpi_wakeup.o obj-$(CONFIG_X86_SMP) += smp.o smpboot.o trampoline.o obj-$(CONFIG_X86_MPPARSE) += mpparse.o -obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi.o +obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi_watchdog.o obj-$(CONFIG_X86_IO_APIC) += io_apic.o obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o obj-$(CONFIG_X86_NUMAQ) += numaq.o
arch/i386/kernel/i386_ksyms.c +3 -2
@@ -92,6 +92,9 @@EXPORT_SYMBOL (cpu_khz);EXPORT_SYMBOL (apm_info); +EXPORT_SYMBOL (request_nmi); +EXPORT_SYMBOL (release_nmi); +#ifdef CONFIG_DEBUG_IOVIRT EXPORT_SYMBOL (__io_virt_debug);#endif
@@ -185,8 +188,6 @@EXPORT_SYMBOL_GPL (register_profile_notifier);EXPORT_SYMBOL_GPL (unregister_profile_notifier); -EXPORT_SYMBOL_GPL (set_nmi_callback); -EXPORT_SYMBOL_GPL (unset_nmi_callback);#undef memcpy#undef memset
arch/i386/kernel/irq.c +4 -0
@@ -131,6 +131,8 @@* Generic, controller-independent functions: */ +extern void nmi_append_user_names (struct seq_file *p); +int show_interrupts (struct seq_file *p,void *v) {int i, j;
@@ -166,6 +168,8 @@for (j =0 ; j <NR_CPUS ; j++)if (cpu_online (j)) p +=seq_printf (p,"%10u " ,nmi_count (j)); +seq_printf (p," " ); +nmi_append_user_names (p);seq_putc (p,'\n' );#if CONFIG_X86_LOCAL_APIC seq_printf (p,"LOC: " );
arch/i386/kernel/nmi.c +181 -340
585 lines not on this page · raw message
arch/i386/kernel/nmi_watchdog.c +481 -0
481 lines not on this page · raw message
arch/i386/kernel/traps.c +3 -107
132 lines not on this page · raw message
arch/i386/oprofile/nmi_int.c +27 -5
@@ -54,12 +54,24 @@// FIXME: kernel_only -static int nmi_callback (struct pt_regs * regs,int cpu) +static int nmi_callback (void *dev_id,struct pt_regs *regs,int cpu,int handled) { -return (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)); +if (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)) +return NOTIFY_OK ; + +return NOTIFY_DONE ; } - +static struct nmi_handler nmi_handler = +{ + .link =LIST_HEAD_INIT (nmi_handler.link), + .dev_name ="oprofile" , + .dev_id =NULL , + .handler = nmi_callback, + .priority =1023 ,/* Very high priority. */ +}; + +static void nmi_save_registers (struct op_msrs * msrs) {unsigned int const nr_ctrs = model->num_counters;
@@ -96,8 +108,12 @@ } +static void nmi_cpu_shutdown (void * dummy); +static int nmi_setup (void ) { +int rv; +/* We walk a thin line between law and rape here. * We need to be careful to install our NMI handler * without actually triggering any NMIs as this will
@@ -105,7 +121,13 @@*/ smp_call_function (nmi_cpu_setup,NULL ,0 ,1 );nmi_cpu_setup (0 ); -set_nmi_callback (nmi_callback); + rv =request_nmi (&nmi_handler); +if (rv) { +smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 ); +nmi_cpu_shutdown (0 ); +return rv; + } + oprofile_pmdev =set_nmi_pm_callback (oprofile_pm_callback);return 0 ; }
@@ -155,7 +177,7 @@static void nmi_shutdown (void ) {unset_nmi_pm_callback (oprofile_pmdev); -unset_nmi_callback (); +release_nmi (&nmi_handler);smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 );nmi_cpu_shutdown (0 ); }
kernel/sched.c +1 -1
@@ -17,7 +17,7 @@*/ #include <linux/mm.h> -#include <linux/nmi.h> +#include <linux/nmi_watchdog.h> #include <linux/init.h> #include <asm/uaccess.h> #include <linux/highmem.h>
include/linux/nmi_watchdog.h +22 -0
@@ -0,0 +1,22 @@ +/* +* linux/include/linux/nmi.h +*/ +#ifndef LINUX_NMI_WATCHDOG_H +#define LINUX_NMI_WATCHDOG_H + +#include <asm/irq.h> + +/** +* touch_nmi_watchdog - restart NMI watchdog timeout. +* +* If the architecture supports the NMI watchdog, touch_nmi_watchdog() +* may be used to reset the timeout - for code which intentionally +* disables interrupts for a long time. This call is stateless. +*/ +#ifdef ARCH_HAS_NMI_WATCHDOG +extern void touch_nmi_watchdog (void ); +#else +# define touch_nmi_watchdog () do { } while(0) +#endif + +#endif
include/linux/nmi.h +7 -18
@@ -1,22 +1,11 @@/* -* linux/include/linux/nmi.h +* linux/include/linux/nmi.h +* +* (C) 2002 Corey Minyard <[email protected]> +* +* Include file for NMI handling. */ -#ifndef LINUX_NMI_H -#define LINUX_NMI_H - -#include <asm/irq.h> - -/** -* touch_nmi_watchdog - restart NMI watchdog timeout. -* -* If the architecture supports the NMI watchdog, touch_nmi_watchdog() -* may be used to reset the timeout - for code which intentionally -* disables interrupts for a long time. This call is stateless. -*/ -#ifdef ARCH_HAS_NMI_WATCHDOG -extern void touch_nmi_watchdog (void ); -#else -# define touch_nmi_watchdog () do { } while(0) -#endif +#if defined(__i386__) +#include <asm/nmi.h> #endif
include/asm-i386/apic.h +0 -1
@@ -79,7 +79,6 @@extern void setup_boot_APIC_clock (void );extern void setup_secondary_APIC_clock (void );extern void setup_apic_nmi_watchdog (void ); -extern inline void nmi_watchdog_tick (struct pt_regs * regs);extern int APIC_init_uniprocessor (void );extern void disable_APIC_timer (void );extern void enable_APIC_timer (void );
include/asm-i386/nmi.h +31 -17
62 lines not on this page · raw message
Quoted 7 lines · Corey Minyard
John Levon suggested I send this to you. It's a cleanup of the NMI handling to make it into a request/release mechanism (instead of hard-coding everything into
traps.c). It renames "nmi.c" to "nmi_watchdog.c" (as it should be named) and moves the real NMI handling code fromtraps.ctonmi.c. It's been posted and reworked on lkml, and it seems to have finally met approval. The "cc-ed" people have reviewed the patch (or at least made helpful suggestions :-).
What interrupt rate have you tested this at? SMP? Adding handlers at runtime? I'm still skeptical on how RCU protects you, but i'm RCU clueless...
Zwane
Signature · 1 line
function.linuxpower.ca
Quoted 4 lines · Corey Minyard
Since a lot of things are hacking into this code (lkcd, kdb, oprofile, nmi watchdog, and now my IPMI watchdog pretimeout), it would be very nice to get their junk out of this code and allow them to bind in nicely, and allow binding from modules.
I've just noticed you haven't fixed the watchdog vs. oprofile case. You pass in the handled flag to the NMI watchdog handler, but you ignore the value and always do the perfctr reset. You /must/ only do the reset if handled == false, or you'll screw up oprofile when it's running.
also, the diff would be much easier to read as a separate "mv nmi.c nmi_watchdog.c" then diff against that
regards
john
Quoted 2 lines · John Levon
also, the diff would be much easier to read as a separate "mv
nmi.cnmi_watchdog.c" then diff against that
Oh, and I suppose I agree with Zwane - stuff like this really needs hammering wrt testing. Have you actually tried oprofile against it yourself Corey ? It's probably the best source of huge amounts of NMIs :)
If you can fix up the patch I'll try and make some time to test it properly
regards
john
Signature · 2 lines
Khendon's Law: If the same point is made twice by the same person, the thread is over.
On Thu, 14 Nov 2002, Corey Minyard wrote:
| Linus, | | John Levon suggested I send this to you. It's a cleanup of the NMI | handling to make it into a request/release mechanism (instead of | hard-coding everything into traps.c). It renames "nmi.c" to | "nmi_watchdog.c" (as it should be named) and moves the real NMI handling | code from traps.c to nmi.c. It's been posted and reworked on lkml, and | it seems to have finally met approval. The "cc-ed" people have reviewed | the patch (or at least made helpful suggestions :-). | | Since a lot of things are hacking into this code (lkcd, kdb, oprofile, | nmi watchdog, and now my IPMI watchdog pretimeout), it would be very | nice to get their junk out of this code and allow them to bind in | nicely, and allow binding from modules.
Switching topics:
Where is the watchdog pretimeout interface ("API") defined? Has there been any discussion of it?
| (And this time it's a -p1 diff)
Looks like a -p0 diff to me.
Signature · 1 line
~Randy
Quoted 2 lines · Zwane Mwaikambo
What interrupt rate have you tested this at? SMP? Adding handlers at runtime? I'm still skeptical on how RCU protects you, but i'm RCU clueless...
The RCU part is fairly simple - you want to avoid having to acquire a lock for every NMI event to walk the handler so you do it lockfree. If a process running in a different CPU tries to
free an nmi handler, it removes it from the list, issues an
rcu callback (to be invoked after all CPUs have gone through
a context switch or executed user-level code ensuring that the deleted nmi handler can't be running) and waits for completion of the callback. The rcu callback handler wakes it up.
It is all hidden under list_add_rcu()/list_del_rcu() and __list_for_each_rcu().
Thanks
Signature · 2 lines
Dipankar Sarma <[email protected]> http://lse.sourceforge.net Linux Technology Center, IBM Software Lab, Bangalore, India.
Quoted 7 lines · Dipankar Sarma
The RCU part is fairly simple - you want to avoid having to acquire a lock for every NMI event to walk the handler so you do it lockfree. If a process running in a different CPU tries to free an nmi handler, it removes it from the list, issues an rcu callback (to be invoked after all CPUs have gone through a context switch or executed user-level code ensuring that the deleted nmi handler can't be running) and waits for completion of
How are you so sure the handler isn't running? You can get an NMI after any cpu instruction inbetween all of that happening, not to mention that it can happen on multiple processors means since its a shared nmi handler list, you're almost never going to find that list not being traversed at some stage by a processor. Try synchronising the cpus for a removal when they're all handling an NMI every millisecond.
Quoted 2 lines
the callback. The rcu callback handler wakes it up.
It is all hidden under list_add_rcu()/list_del_rcu() and__list_for_each_rcu().
I don't think you can rely on completion() to ensure this. Its hardly an atomic operation in this context, whats wrong with spin_trylock(nmi_handler_lock) and do an early bailout on failure?
Zwane
Signature · 1 line
function.linuxpower.ca
Quoted 9 lines · Zwane Mwaikambo
Quoted 7 lines · Dipankar Sarma
The RCU part is fairly simple - you want to avoid having to acquire a lock for every NMI event to walk the handler so you do it lockfree. If a process running in a different CPU tries to free an nmi handler, it removes it from the list, issues an rcu callback (to be invoked after all CPUs have gone through a context switch or executed user-level code ensuring that the deleted nmi handler can't be running) and waits for completion of
Quoted 6 lines
How are you so sure the handler isn't running? You can get an NMI after any cpu instruction inbetween all of that happening, not to mention that it can happen on multiple processors means since its a shared nmi handler list, you're almost never going to find that list not being traversed at some stage by a processor. Try synchronising the cpus for a removal when they're all handling an NMI every millisecond.
Once you remove a handler from the list, any subsequent NMI is *not* going to see the handler. So even if another CPU is executing the same handler, if you wait for the RCU callback, you can guarantee that no-one is executing the deleted handler. RCU will wait for all the CPUs to context switch or execute user-level code atleast once. That meant any other CPU which might have been executing the NMI handler at the time of deletion has now exited from the handler.
Quoted 1 line
I don't think you can rely on
completion()to ensure this. Its hardly an
Corey's code doesn't rely on completion() to ensure this, it relies on RCU to make sure that nobody is running the handler. The key is that once the pointers between the prev and the next of the deleted NMI handler are set, subsequent NMIs aren't going to see that handler. context switch/user-level means that CPU must have exited any NMI handler it may have been executing at the time of deletion.
Quoted 2 lines
atomic operation in this context, whats wrong with
spin_trylock(nmi_handler_lock)and do an early bailout on failure?
spin_trylock modifies the lock cacheline, so cacheline bouncing.
Thanks
Signature · 2 lines
Dipankar Sarma <[email protected]> http://lse.sourceforge.net Linux Technology Center, IBM Software Lab, Bangalore, India.
Quoted 5 lines · Dipankar Sarma
Once you remove a handler from the list, any subsequent NMI is *not* going to see the handler. So even if another CPU is executing the same handler, if you wait for the RCU callback, you can guarantee that no-one is executing the deleted handler. RCU will wait for all the CPUs to context switch or execute user-level code atleast once.
I think you're confusing NMI handling, they aren't like your normal interrupts. You're not going to see that context switch.
Quoted 8 lines
That meant any other CPU which might have been executing the NMI handler at the time of deletion has now exited from the handler.
Quoted 1 line
I don't think you can rely on
completion()to ensure this. Its hardly anCorey's code doesn't rely on
completion()to ensure this, it relies on RCU to make sure that nobody is running the handler. The key is that once the pointers between the prev and the next of the deleted
Can you change prev and next atomically?
Quoted 3 lines
NMI handler are set, subsequent NMIs aren't going to see that handler. context switch/user-level means that CPU must have exited any NMI handler it may have been executing at the time of deletion.
Again, are you mistaking this for a normal interrupt?
Quoted 4 lines
Quoted 2 lines
atomic operation in this context, whats wrong with
spin_trylock(nmi_handler_lock)and do an early bailout on failure?
spin_trylockmodifies the lock cacheline, so cacheline bouncing.
At a fair interrupt rate i'd rather have that fill my caches, less time spent in the NMI handler means more overall system time.
Zwane
Signature · 1 line
function.linuxpower.ca
Quoted 10 lines · Zwane Mwaikambo
Quoted 5 lines · Dipankar Sarma
Once you remove a handler from the list, any subsequent NMI is *not* going to see the handler. So even if another CPU is executing the same handler, if you wait for the RCU callback, you can guarantee that no-one is executing the deleted handler. RCU will wait for all the CPUs to context switch or execute user-level code atleast once.
I think you're confusing NMI handling, they aren't like your normal interrupts. You're not going to see that context switch.
Let us examine the race -
CPU #0 CPU #1 CPU #2
(free_nmi P) execute NMI P syscall
delete from list
call_rcu NMI (doesn't see P)
wait for completion process in kernel process in kernel
(context switch)
context switch context switch
----------- RCU complete NMI handler P must be complete here --------------
RCU handler tasklet
callback: complete()
nmi freeing task
wakes up and proceeds.
Quoted 5 lines
Quoted 3 lines
Corey's code doesn't rely on
completion()to ensure this, it relies on RCU to make sure that nobody is running the handler. The key is that once the pointers between the prev and the next of the deletedCan you change prev and next atomically?
You don't have to, the traversal during __list_for_each_rcu() is done in only one direction. So writing out the next pointer is sufficiently atomic for subsequent NMIs not to see the deleted handler. Either you see the deleted handler or you don't.
Quoted 4 lines
Quoted 1 line
spin_trylockmodifies the lock cacheline, so cacheline bouncing.At a fair interrupt rate i'd rather have that fill my caches, less time spent in the NMI handler means more overall system time.
It isn't going to fill your caches, it is going to bounce around from CPU to CPU on every NMI since every NMI will modify the cache line. So you hurt performance.
Thanks
Signature · 2 lines
Dipankar Sarma <[email protected]> http://lse.sourceforge.net Linux Technology Center, IBM Software Lab, Bangalore, India.
Quoted 3 lines · Corey Minyard
diff -urN
linux.orig/arch/i386/kernel/nmi_watchdog.clinux/arch/i386/kernel/nmi_watchdog.carch/i386/kernel/nmi_watchdog.c +0
-0
...
arch/i386/kernel/nmi_watchdog.c line 268
Quoted 11 lines
arch/i386/kernel/nmi_watchdog.c +11
-0+static int k7_watchdog_reset (int handled) +{ +unsigned int low, high; +int source; + +rdmsr (MSR_K7_PERFCTR0 , low, high); + source = (low & (1 <<31 )) ==0 ; +if (source) +wrmsr (MSR_K7_PERFCTR0 , -(cpu_khz/nmi_hz*1000 ),-1 ); +return source; +}
and similar code in p6 and p4 watchdog_reset.
- Why are you reading the perfctrs with RDMSR instead of RDPMC? RDMSR is noticeably slower on most post-P5 CPUs.
- "(low & (1 << 31)) == 0" looks like a convoluted and inefficient way of computing "(int)low >= 0".
- The p6 and k7 watchdog_reset procedures are identicial, except for the actual MSR number used. The original nmi.c makes the number a parameter and shares the code, causing less code bloat.
/Mikael
Quoted 4 lines · John Levon
On Thu, Nov 14, 2002 at 10:30:16PM -0600, Corey Minyard wrote:
Quoted 9 lines
Quoted 3 lines
Since a lot of things are hacking into this code (lkcd, kdb, oprofile, nmi watchdog, and now my IPMI watchdog pretimeout), it would be very nice to get their junk out of this code and allow them to bind in nicely, and allow binding from modules.
I've just noticed you haven't fixed the watchdog vs. oprofile case. You pass in the handled flag to the NMI watchdog handler, but you ignore the value and always do the perfctr reset. You /must/ only do the reset if handled == false, or you'll screw up oprofile when it's running.
I don't think that's a good idea for two reasons:
* If the oprofile code is only using the counter that the NMI watchdog is not using, it will silently cause the NMI watchdog to stop working. I know that's not the case now, but it could be in the future.
* The oprofile code will always reset the counter, so the NMI watchdog will never see the timeout, so it doesn't matter.
It's currently kind of an unnatural relationship. IMHO, it would be better to have a separate handler for the perf counters that they both use. But that's beyond the scope of this right now.
Quoted 4 lines
also, the diff would be much easier to read as a separate "mv
nmi.cnmi_watchdog.c" then diff against that
Yes, but harder to apply (a little :-). I'll attach that patch. As John says, you need to do a "mv nmi.c nmi_watchdog.c" in the arch/i386/kernel.c directory before you apply this patch.
-Corey
arch/i386/kernel/Makefile +2 -2
@@ -8,7 +8,7 @@ obj-y := process.o semaphore.o signal.o entry.o traps.o irq.o vm86.o \ ptrace.o i8259.o ioport.o ldt.o setup.o time.o sys_i386.o \ - pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o + pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o nmi.o obj-y += cpu/ obj-y += timers/
@@ -22,7 +22,7 @@ obj-$(CONFIG_ACPI_SLEEP) += acpi_wakeup.o obj-$(CONFIG_X86_SMP) += smp.o smpboot.o trampoline.o obj-$(CONFIG_X86_MPPARSE) += mpparse.o -obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi.o +obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi_watchdog.o obj-$(CONFIG_X86_IO_APIC) += io_apic.o obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o obj-$(CONFIG_X86_NUMAQ) += numaq.o
arch/i386/kernel/i386_ksyms.c +3 -2
@@ -92,6 +92,9 @@EXPORT_SYMBOL (cpu_khz);EXPORT_SYMBOL (apm_info); +EXPORT_SYMBOL (request_nmi); +EXPORT_SYMBOL (release_nmi); +#ifdef CONFIG_DEBUG_IOVIRT EXPORT_SYMBOL (__io_virt_debug);#endif
@@ -185,8 +188,6 @@EXPORT_SYMBOL_GPL (register_profile_notifier);EXPORT_SYMBOL_GPL (unregister_profile_notifier); -EXPORT_SYMBOL_GPL (set_nmi_callback); -EXPORT_SYMBOL_GPL (unset_nmi_callback);#undef memcpy#undef memset
arch/i386/kernel/irq.c +4 -0
@@ -131,6 +131,8 @@* Generic, controller-independent functions: */ +extern void nmi_append_user_names (struct seq_file *p); +int show_interrupts (struct seq_file *p,void *v) {int i, j;
@@ -166,6 +168,8 @@for (j =0 ; j <NR_CPUS ; j++)if (cpu_online (j)) p +=seq_printf (p,"%10u " ,nmi_count (j)); +seq_printf (p," " ); +nmi_append_user_names (p);seq_putc (p,'\n' );#if CONFIG_X86_LOCAL_APIC seq_printf (p,"LOC: " );
arch/i386/kernel/nmi.c +245 -0
245 lines not on this page · raw message
arch/i386/kernel/nmi_watchdog.c +99 -22
204 lines not on this page · raw message
arch/i386/kernel/traps.c +3 -107
132 lines not on this page · raw message
arch/i386/oprofile/nmi_int.c +27 -5
@@ -54,12 +54,24 @@// FIXME: kernel_only -static int nmi_callback (struct pt_regs * regs,int cpu) +static int nmi_callback (void *dev_id,struct pt_regs *regs,int cpu,int handled) { -return (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)); +if (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)) +return NOTIFY_OK ; + +return NOTIFY_DONE ; } - +static struct nmi_handler nmi_handler = +{ + .link =LIST_HEAD_INIT (nmi_handler.link), + .dev_name ="oprofile" , + .dev_id =NULL , + .handler = nmi_callback, + .priority =1023 ,/* Very high priority. */ +}; + +static void nmi_save_registers (struct op_msrs * msrs) {unsigned int const nr_ctrs = model->num_counters;
@@ -96,8 +108,12 @@ } +static void nmi_cpu_shutdown (void * dummy); +static int nmi_setup (void ) { +int rv; +/* We walk a thin line between law and rape here. * We need to be careful to install our NMI handler * without actually triggering any NMIs as this will
@@ -105,7 +121,13 @@*/ smp_call_function (nmi_cpu_setup,NULL ,0 ,1 );nmi_cpu_setup (0 ); -set_nmi_callback (nmi_callback); + rv =request_nmi (&nmi_handler); +if (rv) { +smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 ); +nmi_cpu_shutdown (0 ); +return rv; + } + oprofile_pmdev =set_nmi_pm_callback (oprofile_pm_callback);return 0 ; }
@@ -155,7 +177,7 @@static void nmi_shutdown (void ) {unset_nmi_pm_callback (oprofile_pmdev); -unset_nmi_callback (); +release_nmi (&nmi_handler);smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 );nmi_cpu_shutdown (0 ); }
include/asm-i386/apic.h +0 -1
@@ -79,7 +79,6 @@extern void setup_boot_APIC_clock (void );extern void setup_secondary_APIC_clock (void );extern void setup_apic_nmi_watchdog (void ); -extern inline void nmi_watchdog_tick (struct pt_regs * regs);extern int APIC_init_uniprocessor (void );extern void disable_APIC_timer (void );extern void enable_APIC_timer (void );
include/asm-i386/nmi.h +31 -17
62 lines not on this page · raw message
include/linux/nmi.h +7 -18
@@ -1,22 +1,11 @@/* -* linux/include/linux/nmi.h +* linux/include/linux/nmi.h +* +* (C) 2002 Corey Minyard <[email protected]> +* +* Include file for NMI handling. */ -#ifndef LINUX_NMI_H -#define LINUX_NMI_H - -#include <asm/irq.h> - -/** -* touch_nmi_watchdog - restart NMI watchdog timeout. -* -* If the architecture supports the NMI watchdog, touch_nmi_watchdog() -* may be used to reset the timeout - for code which intentionally -* disables interrupts for a long time. This call is stateless. -*/ -#ifdef ARCH_HAS_NMI_WATCHDOG -extern void touch_nmi_watchdog (void ); -#else -# define touch_nmi_watchdog () do { } while(0) -#endif +#if defined(__i386__) +#include <asm/nmi.h> #endif
include/linux/nmi_watchdog.h +22 -0
@@ -0,0 +1,22 @@ +/* +* linux/include/linux/nmi.h +*/ +#ifndef LINUX_NMI_WATCHDOG_H +#define LINUX_NMI_WATCHDOG_H + +#include <asm/irq.h> + +/** +* touch_nmi_watchdog - restart NMI watchdog timeout. +* +* If the architecture supports the NMI watchdog, touch_nmi_watchdog() +* may be used to reset the timeout - for code which intentionally +* disables interrupts for a long time. This call is stateless. +*/ +#ifdef ARCH_HAS_NMI_WATCHDOG +extern void touch_nmi_watchdog (void ); +#else +# define touch_nmi_watchdog () do { } while(0) +#endif + +#endif
kernel/sched.c +1 -1
@@ -17,7 +17,7 @@*/ #include <linux/mm.h> -#include <linux/nmi.h> +#include <linux/nmi_watchdog.h> #include <linux/init.h> #include <asm/uaccess.h> #include <linux/highmem.h>
Quoted 8 lines · Zwane Mwaikambo
Quoted 5 lines
Once you remove a handler from the list, any subsequent NMI is *not* going to see the handler. So even if another CPU is executing the same handler, if you wait for the RCU callback, you can guarantee that no-one is executing the deleted handler. RCU will wait for all the CPUs to context switch or execute user-level code atleast once.
I think you're confusing NMI handling, they aren't like your normal interrupts. You're not going to see that context switch.
The dangerous part is when a particular NMI interrupt is holding a reference to the removed item on the list. Now, after *every* CPU has been through a normal schedule, none of them can still be running /that particular/ NMI interrupt: the fact they can be running other NMIs constantly is neither here nor there, as newly generated NMIs can't see the deleted element anyway.
Quoted 5 lines
Quoted 3 lines
Corey's code doesn't rely on
completion()to ensure this, it relies on RCU to make sure that nobody is running the handler. The key is that once the pointers between the prev and the next of the deletedCan you change prev and next atomically?
As long as they're naturally aligned.
Quoted 5 lines
Quoted 3 lines
NMI handler are set, subsequent NMIs aren't going to see that handler. context switch/user-level means that CPU must have exited any NMI handler it may have been executing at the time of deletion.
Again, are you mistaking this for a normal interrupt?
Zwane you're going to have describe a race if you think you can see one
- neither I nor Dipankar follow your point
Quoted 2 lines
At a fair interrupt rate i'd rather have that fill my caches, less time spent in the NMI handler means more overall system time.
-EPARSE. You will spend more time in the NMI handlers bouncing the line across.
regards
john
Signature · 2 lines
Khendon's Law: If the same point is made twice by the same person, the thread is over.
Quoted 6 lines · Corey Minyard
I don't think that's a good idea for two reasons:
* If the oprofile code is only using the counter that the NMI watchdog is not using, it will silently cause the NMI watchdog to stop working. I know that's not the case now, but it could be in the future.
Uh, this is fine. We always call the NMI watchdog handler, so it will see apic irqs get stuck, and work anyway.
Quoted 2 lines
* The oprofile code will always reset the counter, so the NMI watchdog will never see the timeout, so it doesn't matter.
wrong. If we are using counters 0 and 1, and 1 overflows, oprofile resets that, then 0 overflows, the NMI watchdog will see it and incorrectly reset it. You HAVE to avoid the reset - you can test it if you don't believe me.
Quoted 3 lines
It's currently kind of an unnatural relationship. IMHO, it would be better to have a separate handler for the perf counters that they both use. But that's beyond the scope of this right now.
Yes.
arch/i386/kernel/nmi_watchdog.c line 38
Quoted 6 lines
arch/i386/kernel/nmi_watchdog.c +6
-0+/* This is for I/O APIC, until we can figure out how to tell if it's from the +I/O APIC. If the NMI was not handled before now, we handle it. */ +static int dummy_watchdog_reset (int handled) +{ +return !handled; +}
And if it was handled previously, you reset it to not handled ? Uh ?
regards
john
Quoted 6 lines · John Levon
Quoted 2 lines · Corey Minyard
I don't think that's a good idea for two reasons:
Quoted 9 lines
Quoted 6 lines
* If the oprofile code is only using the counter that the NMI watchdog is not using, it will silently cause the NMI watchdog to stop working. I know that's not the case now, but it could be in the future.
Uh, this is fine. We always call the NMI watchdog handler, so it will see apic irqs get stuck, and work anyway.
If the NMI watchdog doesn't handle an interrupt, it will not re-enable the watchdog (in local APIC mode). If there is no other source of NMIs, then it will never run. It will get hit when the timer wraps around, I guess, but that could be a LONG time.
Quoted 9 lines
Quoted 4 lines
* The oprofile code will always reset the counter, so the NMI watchdog will never see the timeout, so it doesn't matter.
wrong. If we are using counters 0 and 1, and 1 overflows, oprofile resets that, then 0 overflows, the NMI watchdog will see it and incorrectly reset it. You HAVE to avoid the reset - you can test it if you don't believe me.
Ok, I see that.
Quoted 11 lines
Review
Quoted 8 lines
(quoted diff) +6
-0+/* This is for I/O APIC, until we can figure out how to tell if it's from the + I/O APIC. If the NMI was not handled before now, we handle it. */ +static int dummy_watchdog_reset(int handled) +{ + return !handled; +}And if it was handled previously, you reset it to not handled ? Uh ?
Oops. Fixed this.
I have attached another patch, this one fixes my stupid bug in dummy_watchdog_reset and also adds code to the NMI watchdog to not handle the NMI if it has already been handled. Again, you must do a "cd arch/i386/kernel; mv nmi.c nmi_watchdog.c" before applying this patch.
I have also created a kernel module that loops requesting and releasing the NMI, and counting the number of NMIs that actually get hit by the handler that is installed.. This is on a dual 2.8GHZ Pentium 4 machine with hyperthreading (so 4 processors, sort of). I have six processes doing the request/release and some other processes eating CPU on each processor. This has been running for almost three hours, about 10,000,000 NMIs have occurred (around 1000/sec). Around 4700 NMIs have been caught by the handler, meaning that it was a close race between the removal and the NMI occurring. So it looks good.
-Corey
arch/i386/kernel/Makefile +2 -2
@@ -8,7 +8,7 @@ obj-y := process.o semaphore.o signal.o entry.o traps.o irq.o vm86.o \ ptrace.o i8259.o ioport.o ldt.o setup.o time.o sys_i386.o \ - pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o + pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o nmi.o obj-y += cpu/ obj-y += timers/
@@ -22,7 +22,7 @@ obj-$(CONFIG_ACPI_SLEEP) += acpi_wakeup.o obj-$(CONFIG_X86_SMP) += smp.o smpboot.o trampoline.o obj-$(CONFIG_X86_MPPARSE) += mpparse.o -obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi.o +obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi_watchdog.o obj-$(CONFIG_X86_IO_APIC) += io_apic.o obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o obj-$(CONFIG_X86_NUMAQ) += numaq.o
arch/i386/kernel/i386_ksyms.c +3 -2
@@ -92,6 +92,9 @@EXPORT_SYMBOL (cpu_khz);EXPORT_SYMBOL (apm_info); +EXPORT_SYMBOL (request_nmi); +EXPORT_SYMBOL (release_nmi); +#ifdef CONFIG_DEBUG_IOVIRT EXPORT_SYMBOL (__io_virt_debug);#endif
@@ -185,8 +188,6 @@EXPORT_SYMBOL_GPL (register_profile_notifier);EXPORT_SYMBOL_GPL (unregister_profile_notifier); -EXPORT_SYMBOL_GPL (set_nmi_callback); -EXPORT_SYMBOL_GPL (unset_nmi_callback);#undef memcpy#undef memset
arch/i386/kernel/irq.c +4 -0
@@ -131,6 +131,8 @@* Generic, controller-independent functions: */ +extern void nmi_append_user_names (struct seq_file *p); +int show_interrupts (struct seq_file *p,void *v) {int i, j;
@@ -166,6 +168,8 @@for (j =0 ; j <NR_CPUS ; j++)if (cpu_online (j)) p +=seq_printf (p,"%10u " ,nmi_count (j)); +seq_printf (p," " ); +nmi_append_user_names (p);seq_putc (p,'\n' );#if CONFIG_X86_LOCAL_APIC seq_printf (p,"LOC: " );
arch/i386/kernel/nmi.c +245 -0
245 lines not on this page · raw message
arch/i386/kernel/nmi_watchdog.c +112 -22
217 lines not on this page · raw message
arch/i386/kernel/traps.c +3 -107
132 lines not on this page · raw message
arch/i386/oprofile/nmi_int.c +27 -5
@@ -54,12 +54,24 @@// FIXME: kernel_only -static int nmi_callback (struct pt_regs * regs,int cpu) +static int nmi_callback (void *dev_id,struct pt_regs *regs,int cpu,int handled) { -return (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)); +if (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)) +return NOTIFY_OK ; + +return NOTIFY_DONE ; } - +static struct nmi_handler nmi_handler = +{ + .link =LIST_HEAD_INIT (nmi_handler.link), + .dev_name ="oprofile" , + .dev_id =NULL , + .handler = nmi_callback, + .priority =1023 ,/* Very high priority. */ +}; + +static void nmi_save_registers (struct op_msrs * msrs) {unsigned int const nr_ctrs = model->num_counters;
@@ -96,8 +108,12 @@ } +static void nmi_cpu_shutdown (void * dummy); +static int nmi_setup (void ) { +int rv; +/* We walk a thin line between law and rape here. * We need to be careful to install our NMI handler * without actually triggering any NMIs as this will
@@ -105,7 +121,13 @@*/ smp_call_function (nmi_cpu_setup,NULL ,0 ,1 );nmi_cpu_setup (0 ); -set_nmi_callback (nmi_callback); + rv =request_nmi (&nmi_handler); +if (rv) { +smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 ); +nmi_cpu_shutdown (0 ); +return rv; + } + oprofile_pmdev =set_nmi_pm_callback (oprofile_pm_callback);return 0 ; }
@@ -155,7 +177,7 @@static void nmi_shutdown (void ) {unset_nmi_pm_callback (oprofile_pmdev); -unset_nmi_callback (); +release_nmi (&nmi_handler);smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 );nmi_cpu_shutdown (0 ); }
include/asm-i386/apic.h +0 -1
@@ -79,7 +79,6 @@extern void setup_boot_APIC_clock (void );extern void setup_secondary_APIC_clock (void );extern void setup_apic_nmi_watchdog (void ); -extern inline void nmi_watchdog_tick (struct pt_regs * regs);extern int APIC_init_uniprocessor (void );extern void disable_APIC_timer (void );extern void enable_APIC_timer (void );
include/asm-i386/nmi.h +31 -17
62 lines not on this page · raw message
include/linux/nmi.h +7 -18
@@ -1,22 +1,11 @@/* -* linux/include/linux/nmi.h +* linux/include/linux/nmi.h +* +* (C) 2002 Corey Minyard <[email protected]> +* +* Include file for NMI handling. */ -#ifndef LINUX_NMI_H -#define LINUX_NMI_H - -#include <asm/irq.h> - -/** -* touch_nmi_watchdog - restart NMI watchdog timeout. -* -* If the architecture supports the NMI watchdog, touch_nmi_watchdog() -* may be used to reset the timeout - for code which intentionally -* disables interrupts for a long time. This call is stateless. -*/ -#ifdef ARCH_HAS_NMI_WATCHDOG -extern void touch_nmi_watchdog (void ); -#else -# define touch_nmi_watchdog () do { } while(0) -#endif +#if defined(__i386__) +#include <asm/nmi.h> #endif
include/linux/nmi_watchdog.h +22 -0
@@ -0,0 +1,22 @@ +/* +* linux/include/linux/nmi.h +*/ +#ifndef LINUX_NMI_WATCHDOG_H +#define LINUX_NMI_WATCHDOG_H + +#include <asm/irq.h> + +/** +* touch_nmi_watchdog - restart NMI watchdog timeout. +* +* If the architecture supports the NMI watchdog, touch_nmi_watchdog() +* may be used to reset the timeout - for code which intentionally +* disables interrupts for a long time. This call is stateless. +*/ +#ifdef ARCH_HAS_NMI_WATCHDOG +extern void touch_nmi_watchdog (void ); +#else +# define touch_nmi_watchdog () do { } while(0) +#endif + +#endif
kernel/sched.c +1 -1
@@ -17,7 +17,7 @@*/ #include <linux/mm.h> -#include <linux/nmi.h> +#include <linux/nmi_watchdog.h> #include <linux/init.h> #include <asm/uaccess.h> #include <linux/highmem.h>
[cc trimmed]
Quoted 4 lines · Corey Minyard
If the NMI watchdog doesn't handle an interrupt, it will not re-enable the watchdog (in local APIC mode). If there is no other source of NMIs, then it will never run. It will get hit when the timer wraps around, I guess, but that could be a LONG time.
oprofile enabled: source of NMIs, check for apic irqs gets triggered, all is OK
oprofile disabled: oprofile disable restores the perfctr settings, so NMI watchdog is as before, all is OK
So this patch actually allows the NMI watchdog to work when oprofile is running, which it didn't before, which is neat.
Quoted 2 lines
I have attached another patch, this one fixes my stupid bug in
dummy_watchdog_resetand also adds code to the NMI watchdog to not
OK great.
Quoted 9 lines
I have also created a kernel module that loops requesting and releasing the NMI, and counting the number of NMIs that actually get hit by the handler that is installed.. This is on a dual 2.8GHZ Pentium 4 machine with hyperthreading (so 4 processors, sort of). I have six processes doing the request/release and some other processes eating CPU on each processor. This has been running for almost three hours, about 10,000,000 NMIs have occurred (around 1000/sec). Around 4700 NMIs have been caught by the handler, meaning that it was a close race between the removal and the NMI occurring. So it looks good.
Can you send me the test module so I don't have to bother writing one myself ?
I'll try to test this weekend
regards
john
Quoted 9 lines · John Levon
Quoted 3 lines
I have also created a kernel module that loops requesting and releasing the NMI, and counting the number of NMIs that actually get hit by the handler that is installed.. This is on a dual 2.8GHZ Pentium 4 machine with hyperthreading (so 4 processors, sort of). I have six processes doing the request/release and some other processes eating CPU on each processor. This has been running for almost three hours, about 10,000,000 NMIs have occurred (around 1000/sec). Around 4700 NMIs have been caught by the handler, meaning that it was a close race between the removal and the NMI occurring. So it looks good.
Can you send me the test module so I don't have to bother writing one myself ?
I'll try to test this weekend
Certainly. It's attached.
Thanks,
-Corey
#include <linux/config.h>
#include <linux/module.h>
#include <linux/miscdevice.h>
#include <linux/init.h>
#include <linux/errno.h>
#include <linux/nmi.h>
#include <linux/notifier.h>
#include <asm/atomic.h>
static atomic_t nmi_count = ATOMIC_INIT(0);
static atomic_t request_count = ATOMIC_INIT(0);
static intdo_nmi(void *dev_id, struct pt_regs *regs, int cpu, int handled)
{
atomic_inc(&nmi_count);
return NOTIFY_DONE;
}static ssize_t do_read(struct file *file,
char *buf,
size_t count,
loff_t *ppos)
{
int rv;
long last_jiffies = jiffies;
struct nmi_handler nmi_handler =
{
.link = LIST_HEAD_INIT(nmi_handler.link),
.dev_name = "nmi_test",
.dev_id = NULL,
.handler = do_nmi,
.priority = 0, /* Call us last. */
};
printk("NMI test: start test\n");
for (;;) {
if (signal_pending(current))
return -ERESTARTSYS;
rv = request_nmi(&nmi_handler);
if (rv) {
printk(KERN_WARNING
"NMI test: Can't register nmi handler\n");
return rv;
}// set_current_state(TASK_INTERRUPTIBLE);
// schedule_timeout(1);
release_nmi(&nmi_handler);
atomic_inc(&request_count);
if ((jiffies - last_jiffies) >= HZ) {
last_jiffies = jiffies;
printk("NMIs = %d, requests=%d\n",
atomic_read(&nmi_count),
atomic_read(&request_count));
}
}
return 0;
}static int do_open(struct inode *ino, struct file *filep)
{
return 0;
}static int do_close(struct inode *ino, struct file *filep)
{
printk("NMI test: in close\n");
return 0;
}static struct file_operations nmi_test_fops = {
.owner = THIS_MODULE, .read = do_read, .write = NULL, .ioctl = NULL, .open = do_open,
.release = do_close,
};
static struct miscdevice nmi_test_miscdev = {
130,
"nmi_test",
&nmi_test_fops
};static int __init nmi_test_init(void)
{
int rv;
rv = misc_register(&nmi_test_miscdev);
if (rv < 0) {
printk("NMI test: Unable to register misc device\n");
return rv;
}
printk(KERN_INFO "NMI test by "
"Corey Minyard ([email protected])\n");
return 0;
}static void __exit nmi_test_exit(void)
{
/* Make sure no one can call us any more. */
misc_deregister(&nmi_test_miscdev);
}module_exit(nmi_test_exit);module_init(nmi_test_init);MODULE_LICENSE("GPL");
Quoted 2 lines · John Levon
Zwane you're going to have describe a race if you think you can see one
- neither I nor Dipankar follow your point
My apologies gentlemen, i stand corrected, my statements were based on my misunderstanding of the RCU code.
Dipankar thanks for taking the time to drill it in.
Regards,
Zwane
Signature · 1 line
function.linuxpower.ca
Quoted 4 lines · Corey Minyard
I have attached another patch, this one fixes my stupid bug in
dummy_watchdog_resetand also adds code to the NMI watchdog to not handle the NMI if it has already been handled. Again, you must do a "cdarch/i386/kernel; mvnmi.cnmi_watchdog.c" before applying this patch.
I have tested this patch running oprofile on a dual box at 150,000 ints/sec and more, with nmi_watchdog=0,1,2, and couldn't reproduce any problems.
One thing: since we have the unnatural relationship between the watchdog and oprofile, I would much prefer that be obvious in the priority. e.g MAX_NMI_PRIORITY, which oprofile uses, then watchdog is MAX_NMI_PRIORITY -1. Currently the gap between the two values you use indicates it's OK to have another handler inbetween, which it definitely isn't.
regards
john
Quoted 5 lines · John Levon
One thing: since we have the unnatural relationship between the watchdog and oprofile, I would much prefer that be obvious in the priority. e.g
MAX_NMI_PRIORITY, which oprofile uses, then watchdog isMAX_NMI_PRIORITY-1. Currently the gap between the two values you use indicates it's OK to have another handler inbetween, which it definitely isn't.
Hmm how about when the machine really is in trouble, we really wouldn't want some things to be running when we want the watchdog to trigger. How do you propose we handle this? nmi_watchdog_tick is pretty light so it has a lesser chance of blowing up in various code when the machine is on the brink of death.
150,000? Nice Corey, again i stand corrected on that front.
Zwane
Signature · 1 line
function.linuxpower.ca
Quoted 12 lines · John Levon
Quoted 3 lines · Corey Minyard
I have attached another patch, this one fixes my stupid bug in
dummy_watchdog_resetand also adds code to the NMI watchdog to not handle the NMI if it has already been handled. Again, you must do a "cdarch/i386/kernel; mvnmi.cnmi_watchdog.c" before applying this patch.I have tested this patch running oprofile on a dual box at 150,000 ints/sec and more, with nmi_watchdog=0,1,2, and couldn't reproduce any problems.
Quoted 6 lines
One thing: since we have the unnatural relationship between the watchdog and oprofile, I would much prefer that be obvious in the priority. e.g
MAX_NMI_PRIORITY, which oprofile uses, then watchdog isMAX_NMI_PRIORITY-1. Currently the gap between the two values you use indicates it's OK to have another handler inbetween, which it definitely isn't.
I agree, the patch is attached, along with other changes that fix stylistic things that John pointed out and removes the comments about the notify stop mask. I have tested this with my test program.
-Corey
arch/i386/kernel/Makefile +2 -2
@@ -8,7 +8,7 @@ obj-y := process.o semaphore.o signal.o entry.o traps.o irq.o vm86.o \ ptrace.o i8259.o ioport.o ldt.o setup.o time.o sys_i386.o \ - pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o + pci-dma.o i386_ksyms.o i387.o dmi_scan.o bootflag.o nmi.o obj-y += cpu/ obj-y += timers/
@@ -22,7 +22,7 @@ obj-$(CONFIG_ACPI_SLEEP) += acpi_wakeup.o obj-$(CONFIG_X86_SMP) += smp.o smpboot.o trampoline.o obj-$(CONFIG_X86_MPPARSE) += mpparse.o -obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi.o +obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi_watchdog.o obj-$(CONFIG_X86_IO_APIC) += io_apic.o obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o obj-$(CONFIG_X86_NUMAQ) += numaq.o
arch/i386/kernel/i386_ksyms.c +3 -2
@@ -92,6 +92,9 @@EXPORT_SYMBOL (cpu_khz);EXPORT_SYMBOL (apm_info); +EXPORT_SYMBOL (request_nmi); +EXPORT_SYMBOL (release_nmi); +#ifdef CONFIG_DEBUG_IOVIRT EXPORT_SYMBOL (__io_virt_debug);#endif
@@ -185,8 +188,6 @@EXPORT_SYMBOL_GPL (register_profile_notifier);EXPORT_SYMBOL_GPL (unregister_profile_notifier); -EXPORT_SYMBOL_GPL (set_nmi_callback); -EXPORT_SYMBOL_GPL (unset_nmi_callback);#undef memcpy#undef memset
arch/i386/kernel/irq.c +4 -0
@@ -131,6 +131,8 @@* Generic, controller-independent functions: */ +extern void nmi_append_user_names (struct seq_file *p); +int show_interrupts (struct seq_file *p,void *v) {int i, j;
@@ -166,6 +168,8 @@for (j =0 ; j <NR_CPUS ; j++)if (cpu_online (j)) p +=seq_printf (p,"%10u " ,nmi_count (j)); +seq_printf (p," " ); +nmi_append_user_names (p);seq_putc (p,'\n' );#if CONFIG_X86_LOCAL_APIC seq_printf (p,"LOC: " );
arch/i386/kernel/nmi.c +245 -0
245 lines not on this page · raw message
arch/i386/kernel/nmi_watchdog.c +116 -23
228 lines not on this page · raw message
arch/i386/kernel/traps.c +3 -107
132 lines not on this page · raw message
arch/i386/oprofile/nmi_int.c +27 -5
@@ -54,12 +54,24 @@// FIXME: kernel_only -static int nmi_callback (struct pt_regs * regs,int cpu) +static int nmi_callback (void * dev_id,struct pt_regs * regs,int cpu,int handled) { -return (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)); +if (model->check_ctrs (cpu, &cpu_msrs[cpu], regs)) +return NOTIFY_OK ; + +return NOTIFY_DONE ; } - +static struct nmi_handler nmi_handler = +{ + .link =LIST_HEAD_INIT (nmi_handler.link), + .dev_name ="oprofile" , + .dev_id =NULL , + .handler = nmi_callback, + .priority =NMI_HANDLER_MAX_PRIORITY /* Highest possible priority */ +}; + +static void nmi_save_registers (struct op_msrs * msrs) {unsigned int const nr_ctrs = model->num_counters;
@@ -96,8 +108,12 @@ } +static void nmi_cpu_shutdown (void * dummy); +static int nmi_setup (void ) { +int rv; +/* We walk a thin line between law and rape here. * We need to be careful to install our NMI handler * without actually triggering any NMIs as this will
@@ -105,7 +121,13 @@*/ smp_call_function (nmi_cpu_setup,NULL ,0 ,1 );nmi_cpu_setup (0 ); -set_nmi_callback (nmi_callback); + rv =request_nmi (&nmi_handler); +if (rv) { +smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 ); +nmi_cpu_shutdown (0 ); +return rv; + } + oprofile_pmdev =set_nmi_pm_callback (oprofile_pm_callback);return 0 ; }
@@ -155,7 +177,7 @@static void nmi_shutdown (void ) {unset_nmi_pm_callback (oprofile_pmdev); -unset_nmi_callback (); +release_nmi (&nmi_handler);smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 );nmi_cpu_shutdown (0 ); }
include/asm-i386/apic.h +0 -1
@@ -79,7 +79,6 @@extern void setup_boot_APIC_clock (void );extern void setup_secondary_APIC_clock (void );extern void setup_apic_nmi_watchdog (void ); -extern inline void nmi_watchdog_tick (struct pt_regs * regs);extern int APIC_init_uniprocessor (void );extern void disable_APIC_timer (void );extern void enable_APIC_timer (void );
include/asm-i386/nmi.h +32 -17
63 lines not on this page · raw message
include/linux/nmi.h +7 -18
@@ -1,22 +1,11 @@/* -* linux/include/linux/nmi.h +* linux/include/linux/nmi.h +* +* (C) 2002 Corey Minyard <[email protected]> +* +* Include file for NMI handling. */ -#ifndef LINUX_NMI_H -#define LINUX_NMI_H - -#include <asm/irq.h> - -/** -* touch_nmi_watchdog - restart NMI watchdog timeout. -* -* If the architecture supports the NMI watchdog, touch_nmi_watchdog() -* may be used to reset the timeout - for code which intentionally -* disables interrupts for a long time. This call is stateless. -*/ -#ifdef ARCH_HAS_NMI_WATCHDOG -extern void touch_nmi_watchdog (void ); -#else -# define touch_nmi_watchdog () do { } while(0) -#endif +#if defined(__i386__) +#include <asm/nmi.h> #endif
include/linux/nmi_watchdog.h +22 -0
@@ -0,0 +1,22 @@ +/* +* linux/include/linux/nmi.h +*/ +#ifndef LINUX_NMI_WATCHDOG_H +#define LINUX_NMI_WATCHDOG_H + +#include <asm/irq.h> + +/** +* touch_nmi_watchdog - restart NMI watchdog timeout. +* +* If the architecture supports the NMI watchdog, touch_nmi_watchdog() +* may be used to reset the timeout - for code which intentionally +* disables interrupts for a long time. This call is stateless. +*/ +#ifdef ARCH_HAS_NMI_WATCHDOG +extern void touch_nmi_watchdog (void ); +#else +# define touch_nmi_watchdog () do { } while(0) +#endif + +#endif
kernel/sched.c +1 -1
@@ -17,7 +17,7 @@*/ #include <linux/mm.h> -#include <linux/nmi.h> +#include <linux/nmi_watchdog.h> #include <linux/init.h> #include <asm/uaccess.h> #include <linux/highmem.h>
Quoted 7 lines · Zwane Mwaikambo
On Sun, 17 Nov 2002, John Levon wrote
150,000? Nice Corey, again i stand corrected on that front.
Zwane
Thanks, but I'm not the one who deserves the credit. John Levon and Dipankar Sarma showed me how it works. I just started working on the problem and have been the code monkey :-). I'm not used to working in the ultra-scalability mode, it's different in many ways.
-Corey