Messages in this thread | | | Date | Wed, 16 Oct 2013 12:25:57 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 5/6] net: __this_cpu_inc in route.c |
| |
On Wed, Oct 16, 2013 at 02:22:49AM -0700, Eric Dumazet wrote: > On Wed, 2013-10-16 at 10:46 +0200, Peter Zijlstra wrote: > > > Are we sure all !x86 implementations will DTRT in that it will increment > > some CPU and not get horribly confused? I suppose it would; but is > > that a guarantee given someplace? > > I think we should be fine, these are only stats exposed > on /proc/net/stat/rt_cache
My concern was really that an unprotected __this_cpu op would complete coherent and not corrupt state. If we cannot guarantee this it should always be a full BUG to use it without proper protection.
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.
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.
| |