Messages in this thread Patch in this message | | | Subject | Re: [PATCH] perf: Fix intel shared extra msr allocation | From | Peter Zijlstra <> | Date | Tue, 05 Jun 2012 15:04:50 +0200 |
| |
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);
| |