Messages in this thread | | | Date | Tue, 5 Jun 2012 14:51:52 +0200 | Subject | Re: [PATCH] perf: Fix intel shared extra msr allocation | From | Stephane Eranian <> |
| |
On Tue, Jun 5, 2012 at 2:39 PM, Peter Zijlstra <peterz@infradead.org> 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.
| |