Messages in this thread Patch in this message | | | Date | Tue, 5 Jun 2012 15:31:18 +0200 | Subject | Re: [PATCH] perf: Fix intel shared extra msr allocation | From | Stephane Eranian <> |
| |
Looks nicer, still missing this little piece + is_fake definition and setting is allocate_fakecpuc())
@@ -1210,7 +1210,7 @@ __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc, * allocated. Also takes care of event which do * not use an extra shared reg */ - if (!reg->alloc) + if (!reg->alloc || cpuc->is_fake) return;
On Tue, Jun 5, 2012 at 3:04 PM, Peter Zijlstra <peterz@infradead.org> wrote: > On Tue, 2012-06-05 at 14:51 +0200, Stephane Eranian wrote: >> So I think if you say in: >> __intel_shared_reg_put_constraints() >> >> if (!cpuc->is_fake) >> reg->alloc = 1; >> >> That should do it given that: >> >> __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc, >> struct hw_perf_event_extra *reg) >> { >> struct er_account *era; >> if (!reg->alloc) >> return; >> >> } >> >> But then, if reg->alloc was already set to 1, you will have a problem >> if you leave >> it as is. So I think in __intel_shared_reg_put_constraints(), you still need: >> >> __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc, >> struct hw_perf_event_extra *reg) >> { >> if (!reg->alloc || cpuc->is_fake) >> return; >> >> Or something like that. > > Right, and then something slightly less hideous for intel_try_alt_er(). > > How does something like this look? > > --- > arch/x86/kernel/cpu/perf_event_intel.c | 43 +++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c > index 166546e..a3b7eb3 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel.c > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > @@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event *event) > return NULL; > } > > -static bool intel_try_alt_er(struct perf_event *event, int orig_idx) > +static int intel_alt_er(int idx) > { > if (!(x86_pmu.er_flags & ERF_HAS_RSP_1)) > - return false; > + return idx; > > - if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) { > + if (idx == EXTRA_REG_RSP_0) > + return EXTRA_REG_RSP_1; > + > + if (idx == EXTRA_REG_RSP_1) > + return EXTRA_REG_RSP_0; > + > + return idx; > +} > + > +static void intel_fixup_er(struct perf_event *event, int idx) > +{ > + if (idx == EXTRA_REG_RSP_0) { > event->hw.config &= ~INTEL_ARCH_EVENT_MASK; > event->hw.config |= 0x01bb; > event->hw.extra_reg.idx = EXTRA_REG_RSP_1; > event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1; > - } else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) { > + } else if (idx == EXTRA_REG_RSP_1) { > event->hw.config &= ~INTEL_ARCH_EVENT_MASK; > event->hw.config |= 0x01b7; > event->hw.extra_reg.idx = EXTRA_REG_RSP_0; > event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0; > } > - > - if (event->hw.extra_reg.idx == orig_idx) > - return false; > - > - return true; > } > > /* > @@ -1157,14 +1163,14 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc, > struct event_constraint *c = &emptyconstraint; > struct er_account *era; > unsigned long flags; > - int orig_idx = reg->idx; > + int idx = reg->idx; > > /* already allocated shared msr */ > if (reg->alloc) > return NULL; /* call x86_get_event_constraint() */ > > again: > - era = &cpuc->shared_regs->regs[reg->idx]; > + era = &cpuc->shared_regs->regs[idx]; > /* > * we use spin_lock_irqsave() to avoid lockdep issues when > * passing a fake cpuc > @@ -1181,16 +1187,23 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc, > atomic_inc(&era->ref); > > /* no need to reallocate during incremental event scheduling */ > - reg->alloc = 1; > + if (!cpuc->is_fake) { > + if (idx != reg->idx) > + intel_fixup_er(event, idx); > + reg->alloc = 1; > + } > > /* > * need to call x86_get_event_constraint() > * to check if associated event has constraints > */ > c = NULL; > - } else if (intel_try_alt_er(event, orig_idx)) { > - raw_spin_unlock_irqrestore(&era->lock, flags); > - goto again; > + } else { > + idx = intel_alt_er(idx); > + if (idx != reg->idx) { > + raw_spin_unlock_irqrestore(&era->lock, flags); > + goto again; > + } > } > raw_spin_unlock_irqrestore(&era->lock, flags); > > -- 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/
| |