Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756728AbbEVKEu (ORCPT ); Fri, 22 May 2015 06:04:50 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:33152 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756632AbbEVKEq (ORCPT ); Fri, 22 May 2015 06:04:46 -0400 MIME-Version: 1.0 In-Reply-To: <20150521111932.704264975@infradead.org> References: <20150521111710.475482798@infradead.org> <20150521111932.704264975@infradead.org> Date: Fri, 22 May 2015 03:04:45 -0700 Message-ID: Subject: Re: [PATCH 02/10] perf/x86: Improve HT workaround GP counter constraint From: Stephane Eranian To: Peter Zijlstra Cc: Ingo Molnar , Vince Weaver , Jiri Olsa , LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4987 Lines: 123 On Thu, May 21, 2015 at 4:17 AM, Peter Zijlstra wrote: > > The (SNB/IVB/HSW) HT bug only affects events that can be programmed > onto GP counters, therefore we should only limit the number of GP > counters that can be used per cpu -- iow we should not constrain the > FP counters. > > This patch changes the code such that we only count GP counters. > > Reported-by: Vince Weaver > Signed-off-by: Peter Zijlstra (Intel) > --- > arch/x86/kernel/cpu/perf_event.c | 15 +++++++++++++++ > arch/x86/kernel/cpu/perf_event.h | 2 -- > arch/x86/kernel/cpu/perf_event_intel.c | 20 -------------------- > 3 files changed, 15 insertions(+), 22 deletions(-) > > --- a/arch/x86/kernel/cpu/perf_event.c > +++ b/arch/x86/kernel/cpu/perf_event.c > @@ -611,6 +611,7 @@ struct sched_state { > int event; /* event index */ > int counter; /* counter index */ > int unassigned; /* number of events to be assigned left */ > + int nr_gp_counters; /* number of GP counters used */ > unsigned long used[BITS_TO_LONGS(X86_PMC_IDX_MAX)]; > }; > > @@ -696,6 +697,20 @@ static bool __perf_sched_find_counter(st > goto done; > } > } > + > + /* > + * Do not allow scheduling of more than half the available generic > + * counters. > + * > + * This helps avoid counter starvation of sibling thread by ensuring at > + * most half the counters cannot be in exclusive mode. There is no > + * designated counters for the limits. Any N/2 counters can be used. > + * This helps with events with specific counter constraints. > + */ > + if (is_ht_workaround_enabled() && > + sched->state.nr_gp_counters++ >= x86_pmu.num_counters / 2) > + return false; > + Has to be > and not >= otherwise: $ perf stat -a -C 0 -e r81d0,r81d0 -I 1000 sleep 100 Gives you 50% multiplexing when it should be 0% based on my testing. > /* Grab the first unused counter starting with idx */ > idx = sched->state.counter; > for_each_set_bit_from(idx, c->idxmsk, INTEL_PMC_IDX_FIXED) { > --- a/arch/x86/kernel/cpu/perf_event.h > +++ b/arch/x86/kernel/cpu/perf_event.h > @@ -134,8 +134,6 @@ enum intel_excl_state_type { > struct intel_excl_states { > enum intel_excl_state_type init_state[X86_PMC_IDX_MAX]; > enum intel_excl_state_type state[X86_PMC_IDX_MAX]; > - int num_alloc_cntrs;/* #counters allocated */ > - int max_alloc_cntrs;/* max #counters allowed */ > bool sched_started; /* true if scheduling has started */ > }; > > --- a/arch/x86/kernel/cpu/perf_event_intel.c > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > @@ -1923,7 +1923,6 @@ intel_start_scheduling(struct cpu_hw_eve > xl = &excl_cntrs->states[tid]; > > xl->sched_started = true; > - xl->num_alloc_cntrs = 0; > /* > * lock shared state until we are done scheduling > * in stop_event_scheduling() > @@ -2008,18 +2007,6 @@ intel_get_excl_constraints(struct cpu_hw > xl = &excl_cntrs->states[tid]; > xlo = &excl_cntrs->states[o_tid]; > > - /* > - * do not allow scheduling of more than max_alloc_cntrs > - * which is set to half the available generic counters. > - * this helps avoid counter starvation of sibling thread > - * by ensuring at most half the counters cannot be in > - * exclusive mode. There is not designated counters for the > - * limits. Any N/2 counters can be used. This helps with > - * events with specifix counter constraints > - */ > - if (xl->num_alloc_cntrs++ == xl->max_alloc_cntrs) > - return &emptyconstraint; > - > cx = c; > > /* > @@ -2632,8 +2619,6 @@ static void intel_pmu_cpu_starting(int c > cpuc->lbr_sel = &cpuc->shared_regs->regs[EXTRA_REG_LBR]; > > if (x86_pmu.flags & PMU_FL_EXCL_CNTRS) { > - int h = x86_pmu.num_counters >> 1; > - > for_each_cpu(i, topology_thread_cpumask(cpu)) { > struct intel_excl_cntrs *c; > > @@ -2647,11 +2632,6 @@ static void intel_pmu_cpu_starting(int c > } > cpuc->excl_cntrs->core_id = core_id; > cpuc->excl_cntrs->refcnt++; > - /* > - * set hard limit to half the number of generic counters > - */ > - cpuc->excl_cntrs->states[0].max_alloc_cntrs = h; > - cpuc->excl_cntrs->states[1].max_alloc_cntrs = h; > } > } > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/