Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

[Andi Kleen <andi@firstfloor.org>]

On Mon, Nov 10, 2008 at 04:50:00PM +0100, Eric Dumazet wrote:
> Andi Kleen a écrit 


Can you send the /proc/cpuinfo output privately please?
I can ask around here.

> >> 			oprofile_add_sample(regs, i);
> >>+			/*
> >>+			 * We need to unmask the apic vector *before*
> >>+			 * writing reset_value to msr counter
> >>+			 */
> >>+			apic_write(APIC_LVTPC, APIC_DM_NMI);
> >> 			wrmsrl(msrs->counters[i].addr, -reset_value[i]);
> >> 		}
> >> 	}
> >> 
> >>-	/* Only P6 based Pentium M need to re-unmask the apic vector but it
> >>-	 * doesn't hurt other P6 variant */
> >>-	apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
> >
> >Did you also test if it really needs to be inside the if () or 
> >just before the wrmsrl? 
> 
> 
> In my testings, the "if (CTR_OVERFLOWED())" condition is always true.
> 
> (I am profiling one event only)

There can be other valid (but rare) cases of NMIs. The code handles this
by not having any overflow events.

It might be that the case where your NMI gets stuck is such
a stray NMI?

That is in theory it should be safe to unmask  that vector
multiple times (the other NMIs should come from other vectors),
but local APIC is an fragile area.

> 
> I felt uncomfortable issuing the apic_write() before oprofile_add_sample()
> 
> Do you mean doing the apic_write() right at the beginning of the 
> ppro_check_ctrs()
> function ?

Better once, otherwise you'll do it multiple times worst case
(multiple counters can overflow)

-Andi

-- 
ak@linux.intel.com