Re: Patch to 2.6.8.1-mm2 to allow multiple NMI handlers to be registered

[Mikael Pettersson <mikpe@csd.uu.se>]

Corey Minyard writes:
 > +static int k7_watchdog_reset(int handled)
 > +{
 > +	unsigned int low, high;
 > +	int          source;
 > +
 > +	rdmsr(MSR_K7_PERFCTR0, low, high);

Please use rdpmc() instead of rdmsr() when reading counter registers.
Ditto in the other places.
(I know oprofile doesn't, but that's no excuse.)

 > +	/* 
 > +	 * If the timer has overflowed, this is certainly a watchdog
 > +	 * source
 > +	 */
 > +	source = (low & (1 << 31)) == 0;
 > +	if (source)

Why not "if ((int)low >= 0)"?

 > +	/*
 > +	 * The only thing that SHOULD be before us is the oprofile
 > +	 * code.  If it has handled an NMI, then we shouldn't.  This
 > +	 * is a rather unnatural relationship, it would much better to
 > +	 * build a perf-counter handler and then tie both the
 > +	 * watchdog and oprofile code to it.  Then this ugliness
 > +	 * could go away.
 > +	 */

Depending on the value of nmi_watchdog and how oprofile was
set up, neither, just one, or both of them can cause NMIs.
Only one of them can do it via the performance counters, however.

How do you handle multiple simultaneous NMIs from different sources?