Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

[Peter Zijlstra]

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@xxxxxxxxx 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.