From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751086AbdEaU5Y (ORCPT ); Wed, 31 May 2017 16:57:24 -0400 Received: from mail-it0-f67.google.com ([209.85.214.67]:33273 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750952AbdEaU5W (ORCPT ); Wed, 31 May 2017 16:57:22 -0400 From: Vince Weaver X-Google-Original-From: Vince Weaver Date: Wed, 31 May 2017 16:57:19 -0400 (EDT) X-X-Sender: vince@macbook-air To: Peter Zijlstra cc: Vince Weaver , Andi Kleen , Stephane Eranian , "Liang, Kan" , "mingo@redhat.com" , "linux-kernel@vger.kernel.org" , "alexander.shishkin@linux.intel.com" , "acme@redhat.com" , "jolsa@redhat.com" , "torvalds@linux-foundation.org" , "tglx@linutronix.de" Subject: Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter In-Reply-To: <20170530172555.5ya3ilfw3sowokjz@hirez.programming.kicks-ass.net> Message-ID: References: <1495213582-3635-1-git-send-email-kan.liang@intel.com> <20170522091916.3gydvflk4fnqkzw5@hirez.programming.kicks-ass.net> <37D7C6CF3E00A74B8858931C1DB2F077536F079F@SHSMSX103.ccr.corp.intel.com> <20170522192335.v4gvhz24ix2jeihg@hirez.programming.kicks-ass.net> <20170523063913.363ssgcy7kmeesye@hirez.programming.kicks-ass.net> <20170524154518.GA24144@tassilo.jf.intel.com> <20170530172555.5ya3ilfw3sowokjz@hirez.programming.kicks-ass.net> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 30 May 2017, Peter Zijlstra wrote: > On Wed, May 24, 2017 at 12:01:50PM -0400, Vince Weaver wrote: > > I already have people really grumpy that you have to have one mmap() page > > per event, meaning you sacrifice one TLB entry for each event you are > > measuring. > > So there is space in that page. We could maybe look at having an array > of stuff (max 32 entries?) which would cover a whole event group. > > Then you only need to mmap() the page for the leading event. Yes, I think that was the interface they were hoping for. I've been meaning to run some tests and see if there is a noticible hit with TLB misses but haven't had the chance. > Something like the below for the uapi changes I suppose. I've not > tried to actually implement it yet, but would something like that be > usable? it looks usable from a quick glance. Often the problems only turn up once you try to implement it. Vince > > > --- > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > index b1c0b187acfe..40ff77e52b9d 100644 > --- a/include/uapi/linux/perf_event.h > +++ b/include/uapi/linux/perf_event.h > @@ -345,7 +345,8 @@ struct perf_event_attr { > context_switch : 1, /* context switch data */ > write_backward : 1, /* Write ring buffer from end to beginning */ > namespaces : 1, /* include namespaces data */ > - __reserved_1 : 35; > + group_pmc : 1, /* use group_pmc in perf_event_mmap_page */ > + __reserved_1 : 34; > > union { > __u32 wakeup_events; /* wakeup every n events */ > @@ -469,7 +470,8 @@ struct perf_event_mmap_page { > cap_user_rdpmc : 1, /* The RDPMC instruction can be used to read counts */ > cap_user_time : 1, /* The time_* fields are used */ > cap_user_time_zero : 1, /* The time_zero field is used */ > - cap_____res : 59; > + cap_group_pmc : 1, /* The group_pmc field is used, ignore @index and @offset */ > + cap_____res : 58; > }; > }; > > @@ -530,11 +532,29 @@ struct perf_event_mmap_page { > __u64 time_zero; > __u32 size; /* Header size up to __reserved[] fields. */ > > + __u32 __reserved_4byte_hole; > + > + /* > + * If cap_group_pmc this array replaces @index and @offset. The array > + * will contain an entry for each group member lead by the event belonging > + * to this mmap(). > + * > + * The @id field can be used to identify which sibling event the respective > + * @index and @offset values belong to. Assuming an immutable group, the > + * array index will stay constant for each event. > + */ > + struct { > + __u32 index; > + __u32 __reserved_hole; > + __s64 offset; > + __u64 id; /* event id */ > + } group_pmc[32]; > + > /* > * Hole for extension of the self monitor capabilities > */ > > - __u8 __reserved[118*8+4]; /* align to 1k. */ > + __u8 __reserved[22*8]; /* align to 1k. */ > > /* > * Control data for the mmap() data buffer. >