Re: [PATCH] perf, x86: try to handle unknown nmis with running perfctrs

From: Don Zickus <dzickus@redhat.com>
To: Robert Richter <robert.richter@amd.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Lin Ming <ming.m.lin@intel.com>, Ingo Molnar <mingo@elte.hu>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Huang, Ying" <ying.huang@intel.com>,
	Yinghai Lu <yinghai@kernel.org>, Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH] perf, x86: try to handle unknown nmis with running perfctrs
Date: Wed, 11 Aug 2010 08:44:43 -0400	[thread overview]
Message-ID: <20100811124443.GC4879@redhat.com> (raw)
In-Reply-To: <20100811111046.GQ26154@erda.amd.com>

On Wed, Aug 11, 2010 at 01:10:46PM +0200, Robert Richter wrote:
> > > +		 *
> > > +		 * NOTE: We have no way of knowing if a second NMI was
> > > +		 * actually triggered, so we may accidentally skip a valid
> > > +		 * unknown nmi later.
> > > +		 */
> > > +		__get_cpu_var(perfctr_skip) +=1;
> 
> ... but this will not work. You have to mark the *absolute* nmi number
> here. If you only raise a flag, the next unknown nmi will be dropped,
> every. Because, in between there could have been other nmis that
> stopped the chain and thus the 'unknown' path is not executed. The
> trick in my patch is that you *know*, which nmi you want to skip.

I guess I am confused.  The way I read your patch was that you assumed the
next NMI would be the one you skip and if there was another NMI in between
the handled one and the one to skip, you would not skip it (nmi count !=
prev + 1) and it would produce an accidental unknown nmi.

I tried to change that with my patch by setting the skip flag which would
be drained on the next unknown nmi, independent of where it is in the NMI
backlog of NMIs.

Did I misread something?

Cheers,
Don