Re: [PATCH 2/6] perf/x86: Fix data source decoding for Skylake

[Andi Kleen]

On Tue, Jun 06, 2017 at 06:21:08PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 06, 2017 at 06:51:20AM -0700, Andi Kleen wrote:
> > > Not too happy about that..
> > > 
> > >   P(LVLX, L4) | P(LVLX, REMOTE)
> > > 
> > > reads like something that should be PERF_MEM_LVL_REM_CCE1 or something
> > 
> > CCE1? You mean L4?
> 
> #define PERF_MEM_LVL_REM_CCE1   0x400 /* Remote Cache (1 hop) */
> 
> It says 'cache' which is irrespective of level.

But remote L4 is far more useful than remote something.

(even though it currently doesn't exist, so it's not too important)

> 
> > The two bits seem cleaner to me than enumerating all cases.  But ok.
> 
> I tend to agree that a separate remote,distance,type fields would have
> been nicer, but we seem to be stuck with this REM_* crud..

Obviously the old ones cannot be changed, but I don't see any reason
not to do better for new encodings.

> 
> > > This new generic 'REMOTE' has too much overlap with the existing things.
> > 
> > So you want a REM_NA ?
> 
> Not sure... What's the point of a REM_NA vs regular NA ? "'something'
> happened 'not here'" vs "'something' happened".

It's a very big difference in latency. That's useful information.

> I hope Stephane has better ideas, he seems to be the one that introduced
> this in the first place.

The original bits were pretty much a direct mapping from the Nehalem 
Intel bits.  But even Intel has out grown it to some degree. 

-Andi