Messages in this thread | | | Subject | Re: [PATCH] perf_events: improve x86 event scheduling (v6 incremental) | From | Peter Zijlstra <> | Date | Sat, 30 Jan 2010 09:56:39 +0100 |
| |
On Sat, 2010-01-30 at 00:08 +0100, Stephane Eranian wrote: > I think there is a problem with this following code: > > void hw_perf_enable(void) > for (i = 0; i < cpuc->n_events; i++) { > > event = cpuc->event_list[i]; > hwc = &event->hw; > > if (hwc->idx == -1 || hwc->idx == cpuc->assign[i]) > continue; > > Here you are looking for events which are moving. I think the 2nd > part of the if is not good enough. It is not because hwc->idx is > identical to the assignment, that you can assume the event was > already there. It may have been there in the past, then scheduled > out and replaced at idx by another event. When it comes back, > it gets its spot back, but it needs to be reprogrammed. > > That is why in v6 incremental, I have added last_cpu, last_tag > to have a stronger checks and match_prev_assignment(). > > Somehow it is missing in the series you've committed unless > I am missing something.
Right, that went missing because I was assuming that was for the optimization of reducing to one loop. And since I didn't see that one loop version work I left that part out.
(The risk of doing more than one thing in one patch)
Still, shouldn't be hard to correct, I'll look at doing a patch for this on monday, unless you beat me to it :-)
| |