[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <37D7C6CF3E00A74B8858931C1DB2F077536F07BC@SHSMSX103.ccr.corp.intel.com>
Date: Mon, 22 May 2017 16:59:32 +0000
From: "Liang, Kan" <kan.liang@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: "mingo@...hat.com" <mingo@...hat.com>,
"eranian@...gle.com" <eranian@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"alexander.shishkin@...ux.intel.com"
<alexander.shishkin@...ux.intel.com>,
"acme@...hat.com" <acme@...hat.com>,
"jolsa@...hat.com" <jolsa@...hat.com>,
"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"vincent.weaver@...ne.edu" <vincent.weaver@...ne.edu>,
"ak@...ux.intel.com" <ak@...ux.intel.com>
Subject: RE: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter
> 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@...el.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.
Thanks for the suggestions.
I will modify the changelog and make it clearer why we need the patch.
Thanks,
Kan
Powered by blists - more mailing lists