Re: [PATCH] mm/slub: fix lockups on PREEMPT && !SMP kernels

[Mark Rutland]

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/