From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758842AbdEVMXE (ORCPT ); Mon, 22 May 2017 08:23:04 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:38396 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751800AbdEVMXB (ORCPT ); Mon, 22 May 2017 08:23:01 -0400 Date: Mon, 22 May 2017 14:22:53 +0200 From: Peter Zijlstra To: kan.liang@intel.com Cc: mingo@redhat.com, eranian@google.com, linux-kernel@vger.kernel.org, alexander.shishkin@linux.intel.com, acme@redhat.com, jolsa@redhat.com, torvalds@linux-foundation.org, tglx@linutronix.de, vincent.weaver@maine.edu, ak@linux.intel.com Subject: Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Message-ID: <20170522122253.z2dhuopiajseqvu7@hirez.programming.kicks-ass.net> References: <1495213582-3635-1-git-send-email-kan.liang@intel.com> <20170522091916.3gydvflk4fnqkzw5@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170522091916.3gydvflk4fnqkzw5@hirez.programming.kicks-ass.net> 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 Mon, May 22, 2017 at 11:19:16AM +0200, Peter Zijlstra wrote: > On Fri, May 19, 2017 at 10:06:21AM -0700, kan.liang@intel.com wrote: > > @@ -934,6 +938,21 @@ int x86_schedule_events(struct cpu_hw_events *cpuc, int n, int *assign) > > for (i = 0; i < n; i++) { > > e = cpuc->event_list[i]; > > e->hw.flags |= PERF_X86_EVENT_COMMITTED; > > + > > + /* > > + * 0x0300 is pseudo-encoding for REF_CPU_CYCLES. > > + * It indicates that fixed counter 2 should be used. > > + * > > + * If fixed counter 2 is occupied and a GP counter > > + * is assigned, an alternative event which can be > > + * counted in GP counter will be used to replace > > + * the pseudo-encoding REF_CPU_CYCLES event. > > + */ > > + if (((e->hw.config & X86_RAW_EVENT_MASK) == 0x0300) && > > + (assign[i] < INTEL_PMC_IDX_FIXED) && > > + x86_pmu.ref_cycles_rep) > > + x86_pmu.ref_cycles_rep(e); > > + > > if (x86_pmu.commit_scheduling) > > x86_pmu.commit_scheduling(cpuc, i, assign[i]); > > } > > This looks dodgy, this is the branch were we managed to schedule all > events. Why would we need to consider anything here? > > I was expecting a retry if there are still unscheduled events and one of > the events was our 0x0300 event. In that case you have to reset the > event and retry the whole scheduling thing. Ah, I see what you've done. That Changelog could use a lot of help, it's barely readable.