Date: Wed, 26 Nov 2008 23:54:30 +0100 (CET)
From: Thomas Gleixner <tglx@linutronix.de>
To: Andi Kleen <andi@firstfloor.org>
cc: eranian@googlemail.com, linux-kernel@vger.kernel.org,
       akpm@linux-foundation.org, mingo@elte.hu, x86@kernel.org,
       eranian@gmail.com, sfr@canb.auug.org.au
Subject: Re: [patch 05/24] perfmon: X86 generic code (x86)
In-Reply-To: <20081126140054.GX6703@one.firstfloor.org>
Message-ID: <alpine.LFD.2.00.0811262237140.3325@localhost.localdomain>
References: <492d0be1.09cc660a.0b75.44b7@mx.google.com> <alpine.LFD.2.00.0811261143480.3325@localhost.localdomain> <20081126140054.GX6703@one.firstfloor.org>
User-Agent: Alpine 2.00 (LFD 1167 2008-08-23)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1249
Lines: 36

On Wed, 26 Nov 2008, Andi Kleen wrote:
> On Wed, Nov 26, 2008 at 02:35:18PM +0100, Thomas Gleixner wrote:
> > > +	 */
> > > +	pfm_arch_resend_irq(ctx);
> > 
> > Do we really need this whole NMI business ?
> 
> Without it you cannot profile interrupts off regions well.

Fair enough, but I doubt that this is a real solution.

There is not even an attempt to avoid the obvious wrmrsl races, while
there are several comments which explain how expensive wrmrsl is. In
the NMI handler we enable the NMI right away. This might cause
multiple NMIs for nothing when the NMIs hit between the manipulations
of the counters. Not likely but can happen depending on the counter
settings.

Sending an self-IPI from NMI simply sucks: For every NMI we get an
extra local interrupt and we have an extra of 2 * NR_ACTIVE_COUNTERS
accesses to MSRs.

Designing that code to use lockless buffers instead is not really
rocket science.

This code sucks performance wise in all aspects I have looked at so
far.

Thanks,

	tglx
--
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/