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

Andi Kleen
2008-11-10T17:37:52Z

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