Date: Tue, 12 Jun 2007 21:07:30 +0200
From: =?iso-8859-1?Q?Bj=F6rn?= Steinbrink <B.Steinbrink@gmx.de>
To: Stephane Eranian <eranian@hpl.hp.com>
Cc: oprofile-list@lists.sourceforge.net, wcohen@redhat.com, ak@suse.de,
       perfmon@napali.hpl.hp.com, linux-kernel@vger.kernel.org,
       levon@movementarian.org
Subject: Re: OProfile issues
Message-ID: <20070612190730.GA6978@atjola.homenet>
Mail-Followup-To: =?iso-8859-1?Q?Bj=F6rn?= Steinbrink <B.Steinbrink@gmx.de>,
	Stephane Eranian <eranian@hpl.hp.com>,
	oprofile-list@lists.sourceforge.net, wcohen@redhat.com, ak@suse.de,
	perfmon@napali.hpl.hp.com, linux-kernel@vger.kernel.org,
	levon@movementarian.org
References: <20070612150246.GJ32163@frankl.hpl.hp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20070612150246.GJ32163@frankl.hpl.hp.com>
User-Agent: Mutt/1.5.13 (2006-08-11)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2488
Lines: 53

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/