Re: OProfile issues

["Björn Steinbrink" <B.Steinbrink@gmx.de>]

On 2007.06.12 08:02:46 -0700, Stephane Eranian wrote:
> Hello,
> 
> I am working on perfmon2 to allow Oprofile and perfmon2 to co-exist
> as suggested by Andi Kleen. I looked at the Oprofile init/shutdown
> code and I am puzzled by several things which you might be able to
> explain for me. I am looking at 2.6.22-rc3.
> 
> Here are the issues:
> 
>  * model->fill_in_addresses is called once for all CPUs
>    on X86, it does more than just filling in the addresses,
>    it also coordinates with the NMI watchdog by reserving
>    registers via the reserve_*nmi() interface.
> 
>    The problem is that the release of the registers is done
>    in model->shutdown() which happens to be executed on every
>    CPU. So you end up releasing the registers too many times.
>    This is *not* harmless once you start sharing the PMU with
>    other subsystems given the way the allocator is designed.

Hm, currently it should be ok to move the call to model->shutdown() into
nmi_shutdown(), but you might want to instead set addr to 0 when the
register is released to still allow for per cpu actions in shutdown().

>  * allocate_msrs() allocates two tables per CPU. One for the
>    counters, the other for the eventsel registers. But then
>    nmi_setup() copies the cpu_msrs[0] into cpu_msrs[] of all
>    other cpus. This operation overrides the cpu_msrs[].counters
>    and cpu_msrs[].controls pointers for all CPUs but CPU0.
>    But free_msrs() will free the same tables multiple times. This
>    causes a kernel dump when you enable certain kernel debugging
>    features. The fix is to copy the content of the counters and
>    controls array, not the pointers.

This was fixed in commit 0939c17c7bcf1.

>  * the fill_in_addresses() callback for X86 invokes the NMI watchdog
>    reserve_*_nmi() register allocation routines. This is done regardless
>    of whether the NMI watchdog is active. When the NMI watchdog is not
>    active, the allocator will satisfy the allocation for the first MSR
>    of each type (counter or control), but then it will reject any
>    request for the others. You end up working with a single
>    counter/control register.

Hm, ouch. I'll try to move the reservation parts into a separate system.

Björn