Re: [PATCH] perf_counter: Check task on counter read IPI

[Peter Zijlstra]

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@xxxxxxxxx>
> ---
> 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?


--
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/