Re: [RFC][PATCH] slub: Check for page NULL before doing the node_match check
From: Eric Dumazet <hidden>
Date: 2013-01-17 21:36:29
Also in:
linux-mm, lkml
On Thu, 2013-01-17 at 21:28 +0000, Christoph Lameter wrote:
quoted hunk ↗ jump to hunk
On Thu, 17 Jan 2013, Steven Rostedt wrote:quoted
quoted
--- a/mm/slub.c +++ b/mm/slub.c@@ -2399,7 +2399,7 @@ redo: object = c->freelist; page = c->page; - if (unlikely(!object || !node_match(page, node))) + if (unlikely(!object || !page || !node_match(page, node)))I'm still trying to see if c->freelist != NULL and c->page == NULL isn't a bug. The cmpxchg_doubles are a little confusing. If it's not expected that page is NULL but freelist isn't than we need to figure out why it happened.hmmm.. We may want to change the sequence of updates to c->page and c->freelist. Update c->freelist to be NULL first so that we always enter the slow path for these cases where we can do more expensive synchronization. Index: linux/mm/slub.c ===================================================================--- linux.orig/mm/slub.c 2013-01-15 10:42:08.490183607 -0600 +++ linux/mm/slub.c 2013-01-17 15:27:48.973051155 -0600@@ -1993,8 +1993,8 @@ static inline void flush_slab(struct kme deactivate_slab(s, c->page, c->freelist); c->tid = next_tid(c->tid); - c->page = NULL; c->freelist = NULL; + c->page = NULL; } /*@@ -2227,8 +2227,8 @@ redo: if (unlikely(!node_match(page, node))) { stat(s, ALLOC_NODE_MISMATCH); deactivate_slab(s, page, c->freelist); - c->page = NULL; c->freelist = NULL; + c->page = NULL; goto new_slab; }@@ -2239,8 +2239,8 @@ redo: */ if (unlikely(!pfmemalloc_match(page, gfpflags))) { deactivate_slab(s, page, c->freelist); - c->page = NULL; c->freelist = NULL; + c->page = NULL; goto new_slab; }
Without appropriate barriers, this change is a shoot in the dark.