Messages in this thread | | | Date | Thu, 8 Jun 2017 22:03:02 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 2/4] perf/x86: Fix data source decoding for Skylake |
| |
On Thu, Jun 08, 2017 at 12:40:59PM -0700, Stephane Eranian wrote: > Hi, > > On Thu, Jun 8, 2017 at 1:15 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Wed, Jun 07, 2017 at 04:22:24PM -0700, Andi Kleen wrote: > > > > > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > > > index b1c0b187acfe..95daade294d7 100644 > > > --- a/include/uapi/linux/perf_event.h > > > +++ b/include/uapi/linux/perf_event.h > > > @@ -931,14 +931,18 @@ union perf_mem_data_src { > > > mem_snoop:5, /* snoop mode */ > > > mem_lock:2, /* lock instr */ > > > mem_dtlb:7, /* tlb access */ > > > - mem_rsvd:31; > > > + mem_lvlx:8, /* memory hierarchy level, ext */ > > > + mem_snoopx:2, /* snoop mode, ext */ > > > + mem_rsvd:21; > > > }; > > > }; > > > #elif defined(__BIG_ENDIAN_BITFIELD) > > > union perf_mem_data_src { > > > __u64 val; > > > struct { > > > - __u64 mem_rsvd:31, > > > + __u64 mem_rsvd:21, > > > + mem_snoopx:2, /* snoop mode, ext */ > > > + mem_lvlx:8, /* memory hierarchy level, ext */ > > > mem_dtlb:7, /* tlb access */ > > > mem_lock:2, /* lock instr */ > > > mem_snoop:5, /* snoop mode */ > > > > So one thing we could do is add a mem_hops field and always set that, > > even for the old stuff. The old stuff will not know about that field and > > ignore the bits, but new stuff will then not need as many LVL bits. > > > That would be better than lvlx I think. I am guessing you're suggesting > an integer count here and not a bitmask. Right?
Yah, 0 hops = local, etc..
> Then I wonder why it > would need 8 bits or 255 possible levels!
I thing we still need lvlx, simply because the current lvl doesn't have room to encode L4.
But having a mem_hops field avoids having to have local/remote/remote2 variants of everything.
That said, I'm afraid SGI can actually fill a mem_hops:8 or something like that ;-)
| |