lkml.org 
[lkml]   [2012]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] perf, x86: Fix Intel shared extra MSR allocation
From
Date
On Tue, 2012-06-05 at 15:30 +0200, Peter Zijlstra wrote:

> @@ -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() */

I'm afraid that needs to be:

/*
* reg->alloc can be set due to existing state, so for fake cpuc
* we need to ignore this, otherwise we might fail to allocate
* proper fake state for this extra reg constraint. Also see
* the comment below.
*/
if (reg->alloc && !cpuc->is_fake)
return NULL; /* call x86_get_event_constraints() */

>
> 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
> @@ -1173,6 +1179,29 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>
> if (!atomic_read(&era->ref) || era->config == reg->config) {
>
> + /*
> + * If its a fake cpuc -- as per validate_{group,event}() we
> + * shouldn't touch event state and we can avoid doing so
> + * since both will only call get_event_constraints() once
> + * on each event, this avoids the need for reg->alloc.
> + *
> + * Not doing the ER fixup will only result in era->reg being
> + * wrong, but since we won't actually try and program hardware
> + * this isn't a problem either.
> + */
> + if (!cpuc->is_fake) {
> + if (idx != reg->idx)
> + intel_fixup_er(event, idx);
> +
> + /*
> + * x86_schedule_events() can call get_event_constraints()
> + * multiple times on events in the case of incremental
> + * scheduling(). reg->alloc ensures we only do the ER
> + * allocation once.
> + */
> + reg->alloc = 1;
> + }
> +
> /* lock in msr value */
> era->config = reg->config;
> era->reg = reg->reg;



\
 
 \ /
  Last update: 2012-06-05 16:41    [W:0.077 / U:2.680 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site