lkml.org 
[lkml]   [2012]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] perf, x86: Fix Intel shared extra MSR allocation
From
On Tue, Jun 5, 2012 at 3:56 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> 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() */
>
>>
Yes.

>>  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 23:41    [W:0.060 / U:1.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site