From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751009AbdE3R0G (ORCPT ); Tue, 30 May 2017 13:26:06 -0400 Received: from merlin.infradead.org ([205.233.59.134]:51278 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750821AbdE3R0F (ORCPT ); Tue, 30 May 2017 13:26:05 -0400 Date: Tue, 30 May 2017 19:25:56 +0200 From: Peter Zijlstra To: Vince Weaver Cc: 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 Message-ID: <20170530172555.5ya3ilfw3sowokjz@hirez.programming.kicks-ass.net> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Looking at the layout it would be slightly awkward to do (or we need to ref the layout, which will undoubtedly be painful too). But for groups the time fields at least are all shared. At the very least we need index and offset, ideally pmc_width would be the same for all counters (can we assume that?). 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? --- 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.