Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751594AbdFHUDH (ORCPT ); Thu, 8 Jun 2017 16:03:07 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:52217 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751416AbdFHUDG (ORCPT ); Thu, 8 Jun 2017 16:03:06 -0400 Date: Thu, 8 Jun 2017 22:03:02 +0200 From: Peter Zijlstra To: Stephane Eranian Cc: Andi Kleen , Arnaldo Carvalho de Melo , LKML , Jiri Olsa , Andi Kleen , Sukadev Bhattiprolu , Madhavan Srinivasan Subject: Re: [PATCH v2 2/4] perf/x86: Fix data source decoding for Skylake Message-ID: <20170608200302.GC8337@worktop.programming.kicks-ass.net> References: <20170607232226.26365-1-andi@firstfloor.org> <20170607232226.26365-3-andi@firstfloor.org> <20170608081531.iv27xaghntniwmm6@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2188 Lines: 53 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 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 ;-)