Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751782AbdFHUKL (ORCPT ); Thu, 8 Jun 2017 16:10:11 -0400 Received: from one.firstfloor.org ([193.170.194.197]:38560 "EHLO one.firstfloor.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751535AbdFHUKK (ORCPT ); Thu, 8 Jun 2017 16:10:10 -0400 Date: Thu, 8 Jun 2017 13:10:07 -0700 From: Andi Kleen To: Stephane Eranian Cc: Peter Zijlstra , 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: <20170608201007.GH8096@two.firstfloor.org> 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.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2759 Lines: 64 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. Note that Skylake cannot fill it in, it doesn't report the hops. And for the old parts the existing bits work. So I don't think there's a motivation to add a new hops field, unless some other architecture needs it. > > > That would be better than lvlx I think. I am guessing you're suggesting > an integer count here and not a bitmask. Right? Then I wonder why it > would need 8 bits or 255 possible levels! Sure counts are better than bits. At least it's a far more efficient encoding. Ok so add a new level_num field, and use one count for PMEM and L4, and also fill in for the others. > > > +#define PERF_MEM_LVLX_PMEM 0x08 /* Persistent Memory */ > > > +#define PERF_MEM_LVLX_REM_PMEM 0x10 /* Remote Persistent Memory */ > > > +#define PERF_MEM_LVLX_REM_NA 0x20 /* Remote N/A level */ > > > > Still wondering what the point of REM_NA is.. can you explain? It's a remote cache, but the hardware doesn't report which one, and also doesn't report the hops. Ok PERF_MEM_LVL_REM_CCE would be a better name I guess, fitting with the existing ones. -Andi