Messages in this thread | | | Subject | Re: [PATCH] perf_counter: Check task on counter read IPI | From | Peter Zijlstra <> | Date | Mon, 17 Aug 2009 10:16:56 +0200 |
| |
On Fri, 2009-08-14 at 15:39 +1000, Paul Mackerras wrote: > In general, code in perf_counter.c that is called through an IPI > checks, for per-task counters, that the counter's task is still the > current task. This is to handle the race condition where the cpu > switches from the task we want to another task in the interval between > sending the IPI and the IPI arriving and being handled on the target > CPU. > > For some reason, __perf_counter_read is missing this check, yet there > is no reason why the race condition can't occur. This adds a check > that the current task is the one we want. If it isn't, we just > return. In that case the counter->count value should be up to date, > since it will have been updated when the counter was scheduled out, > which must have happened since the IPI was sent. > > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > I don't have an example of an actual failure due to this race, but it > seems obvious that it could occur and we need to guard against it, so > I think this should go in .31.
Hmm, right.
However those other sites have retry loops in the caller, but callers of __perf_counter_read() do not. Granted, I'm not sure what they should retry on exactly, but this patch trades an invalid update to a missing update.
While I think the balance tips towards favouring the missing update, its not really much of an improvement.
I guess we could keep a sequence count with the update and loop until it gets increased or something?
| |