Messages in this thread | | | Date | Tue, 26 May 2015 15:22:21 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 01/11] perf,x86: Fix event/group validation |
| |
On Tue, May 26, 2015 at 05:25:59AM -0700, Stephane Eranian wrote: > > IIRC the problem was that the copy from c2 into c1: > > > > if (c1 && (c1->flags & PERF_X86_EVENT_DYNAMIC)) { > > bitmap_copy(c1->idxmsk, c2->idxmsk, X86_PMC_IDX_MAX); > > c1->weight = c2->weight; > > c2 = c1; > > } > > > > is incomplete. For instance, flags is not copied, and some code down the > > line might check that and get wrong flags. > > > Ok, now I remember this code. It has to do with incremental scheduling. > Suppose E1, E2, E3 events where E1 is exclusive. The first call is > for scheduling E1. It gets to get_event_constraint() which "allocates" a > dynamic constraint. The second call tries to schedule E1, E2. But the > second time for E1, you already have the dynamic constraint allocated, so > this code is reusing the constraint storage and just updates the bitmask > and weight. > > Now, that the storage is not actually dynamic (kmalloc'd), but taken from a > fixed size array in cpuc, I believe we can simplify this and "re-allocate" > the constraint for each incremental call to intel_get_event_constraints(). > Do you agree?
That would probably work, the whole incremental thing seems superfluous to me.
| |