On Fri, Mar 13, 2015 at 04:29:23PM +0000, Christoph Lameter wrote: > On Fri, 13 Mar 2015, Mark Rutland wrote: > > > */ > > - do { > > - tid = this_cpu_read(s->cpu_slab->tid); > > - c = raw_cpu_ptr(s->cpu_slab); > > - } while (IS_ENABLED(CONFIG_PREEMPT) && unlikely(tid != c->tid)); > > + c = raw_cpu_ptr(s->cpu_slab); > > + tid = READ_ONCE(c->tid); > > > > Ok that works for the !SMP case. What about SMP and PREEMPT now?
>From testing on boards I have access to, things seem fine so far with SMP && PREEMPT. If we have any allocator stress tests I'm more than happy to give them a go. As I mentioned, it's not clear to me that the the READ_ONCE(c->tid) is safe (i.e. it is atomic and non-destructive). If READ_ONCE(c->tid) is not safe then the code added in 9aabf810a67cd97e is similarly broken given the access in the loop condition, in addition to the hoisting done by the compiler. > And yes code like this was deemed safe for years and the race condition is > very subtle and difficult to trigger (also given that PREEMPT is rarely > used these days). The this_cpu_cmpxchg_double is the saving grace here: if c->tid is read from a different CPU it will fail and we'll retry the whole thing. That's exactly what the original patch relied on in the case a preemption occured after the loop. w.r.t. CONFIG_PREEMPT, git grep tells me otherwise: [mark@leverpostej:~/src/linux]% git grep -w 'CONFIG_PREEMPT' -- arch/*/configs/* | wc -l 109 [mark@leverpostej:~/src/linux]% git grep -w 'CONFIG_PREEMPT is not set' -- arch/*/configs/* | wc -l 2 [mark@leverpostej:~/src/linux]% git grep -w 'CONFIG_PREEMPT=y' -- arch/*/configs/* | wc -l 107 [mark@leverpostej:~/src/linux]% git grep -w 'CONFIG_PREEMPT=n' -- arch/*/configs/* | wc -l 0 Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/