| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABPqkBRF_2LXE-54tLMBnQgLKbKyR9qo+4VohQiOchpqe1pgyw@mail.gmail.com>
Date: Tue, 5 Jun 2012 15:31:18 +0200
From: Stephane Eranian <eranian@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: "Yan, Zheng" <zheng.z.yan@...ux.intel.com>,
"Yan, Zheng" <zheng.z.yan@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf: Fix intel shared extra msr allocation
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@...radead.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists