Re: [PATCH 5/6] net: __this_cpu_inc in route.c

[Christoph Lameter]

On Wed, 16 Oct 2013, Peter Zijlstra wrote:

> For x86 its fairly easy to see its correct this way; but for load-store
> archs this isn't immediately obvious to me.
>
> Suppose; r1 contains our per-cpu pointer:
>
> 	LOAD  r2, per-cpu-base
> 	ADD   r1, r2
> 	LOAD  r2, $(r1) # load value
> 	INC   r2
> 	STORE $(r1), r2 # store value
>
> If such a thing is done without preempt disable; we could be
> preempted/migrated at any place along that chain. In that case the STORE
> could be to another CPUs memory (we get migrated near the INC) and could
> conflict with a per-cpu operation on that CPU corrupting state.
>
> If I look at percpu.h; the generic __this_cpu versions look like they
> generate the above for such archs.

Yes they do.

> In that case; I don't see how even for statistics (where we really don't
> care what cpu the op happens on, as long as it happens to a cpu,
> coherently) it is correct to use the raw_this_cpu stuff without
> preemption protection.
>
> In fact; I think the comment near __this_cpu_read actually alludes to
> this.

Indeed. This is well known. For some statistics this has been
judged to be acceptable in the past when the alternative was disabling
interrupts since the overhead was so much higher and the statistics
operations had to be used in very performance critical sections.

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