Date: Wed, 9 Nov 2016 15:25:15 +0100
From: Robert Richter <rric@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Liang, Kan" <kan.liang@intel.com>, Andi Kleen <andi@firstfloor.org>,
        Jiri Olsa <jolsa@kernel.org>, Vince Weaver <vince@deater.net>,
        lkml <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] perf/x86: Fix overlap counter scheduling bug
Message-ID: <20161109142515.GY25086@rric.localdomain>
References: <1478015068-14052-1-git-send-email-jolsa@kernel.org>
 <20161108122039.GP3142@twins.programming.kicks-ass.net>
 <20161108150949.GM26852@two.firstfloor.org>
 <37D7C6CF3E00A74B8858931C1DB2F07750C8D9D0@SHSMSX103.ccr.corp.intel.com>
 <20161108165749.GJ3117@twins.programming.kicks-ass.net>
 <37D7C6CF3E00A74B8858931C1DB2F07750C8DA4F@SHSMSX103.ccr.corp.intel.com>
 <20161108182739.GO3117@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161108182739.GO3117@twins.programming.kicks-ass.net>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1796
Lines: 45

On 08.11.16 19:27:39, Peter Zijlstra wrote:
> The comment with EVENT_CONSTRAINT_OVERLAP states: "This is the case if
> the counter mask of such an event is not a subset of any other counter
> mask of a constraint with an equal or higher weight".
> 
> Esp. that latter part is of interest here I think, our overlapping mask
> is 0x0e, that has 3 bits set and is the highest weight mask in on the
> PMU, therefore it will be placed last. Can we still create a scenario
> where we would need to rewind that?
> 
> The scenario for AMD Fam15h is we're having masks like:
> 
> 	0x3F -- 111111
> 	0x38 -- 111000
> 	0x07 -- 000111
> 
> 	0x09 -- 001001
> 
> And we mark 0x09 as overlapping, because it is not a direct subset of
> 0x38 or 0x07 and has less weight than either of those. This means we'll
> first try and place the 0x09 event, then try and place 0x38/0x07 events.
> Now imagine we have:
> 
> 	3 * 0x07 + 0x09
> 
> and the initial pick for the 0x09 event is counter 0, then we'll fail to
> place all 0x07 events. So we'll pop back, try counter 4 for the 0x09
> event, and then re-try all 0x07 events, which will now work.
> 
> 
> 
> But given, that in the uncore case, the overlapping event is the
> heaviest mask, I don't think this can happen. Or did I overlook
> something.... takes a bit to page all this back in.

Right, IMO 0xE mask may not be marked as overlapping. It is placed
last and if there is no space left we are lost. There is no other
combination or state we could try then. So the fix is to remove the
overlapping bit for that counter, the state is then never saved.

This assumes there are no other counters than 0x3 and 0xc that overlap
with 0xe. It becomes a bit tricky if there is another counter with the
same or higher weight that overlaps with 0xe, e.g. 0x7.

-Robert