Re: [PATCH] perf: Fix intel shared extra msr allocation

[Stephane Eranian]

On Tue, Jun 5, 2012 at 2:39 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Tue, 2012-06-05 at 14:07 +0200, Peter Zijlstra wrote:
>> On Tue, 2012-06-05 at 12:38 +0200, Stephane Eranian wrote:
>> > How about we add a field or flag to cpuc to tell it's fake, and then
>> > in
>> > try_alt_er() and __intel_shared_reg_get_constraints() we avoid
>> > touching
>> > live struct (like reg->alloc) if fake==1. I think he was trying to do
>> > the same with the core_id == -1 test.
>>
>> We might have to do something like that, however I'm trying to figure
>> out when that reg->alloc test in __intel_shared_reg_get_contraints() is
>> useful.
>>
>> If it is useful in event scheduling, we cannot just leave it out in
>> validate_group().
>
> OK, so x86_schedule_events() can call that multiple times on the same
> event in case we keep adding events.. it needs the constraints of the
> previous events as well, and in that case we skip the whole extra_reg
> muck.

That reg->alloc is to avoid going through that shared MSR allocation
code again because we already have a valid assignment. It helps with
incremental scheduling of events (collect() -> schedule()).
>
> But for validate_group() we only do this once, so it should work. Nasty
> though.

True, only one pass. 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/