On 10/27/20 4:12 PM, Laurent Dufour wrote:
Le 27/10/2020 à 16:03, Michal Hocko a écrit :
On Tue 27-10-20 15:39:46, Laurent Dufour wrote:
Le 27/10/2020 à 15:24, Michal Hocko a écrit :
[Cc Vlastimil]
On Tue 27-10-20 15:09:26, Laurent Dufour wrote:
Could you be more specific? I am especially confused how the memory hotplug is involved here. What kind of flush are we talking about?
This happens when flush_cpu_slab() is called when a memory block is about to be offlined, see slab_mem_going_offline_callback() called by the MEM_GOING_OFFLINE's callback triggered by offline_pages().
This would be a very valuable information for the changelog. I have to admit that a more detailed description would help somebody not really familiar with slub internals like me.
Agreed, please include that.
I still fail to see why do we get an inconsistent state though. I thought that no object is associated with an offlined page so how come we have an object without any page?
The inconsistent state came from the IPI interrupt calling flush_cpu_slab() being taken between reading c->freelist and c->page.
Yes; also good to state explicitly.
How does this allocation path synchronizes with the offline callback?
My understanding is that this is done by the call to this_cpu_cmpxchg_double() done later, but I would let the slub experts detail that point.
Yes, cmpxchg will detect that c->freelist changed. If we managed to read both c->freelist and c->page before the interrupt (and thus not crash), cmpxchg_double will fail on the s->cpu_slab->tid part as flush_slab() will also bump the tid.
In commit 6159d0f5c03e ("mm/slub.c: page is always non-NULL in node_match()") check on the page pointer has been removed assuming that page is always valid when it is called. It happens that this is not true in that particular case, so check for page before calling node_match() here.
Fixes: 6159d0f5c03e ("mm/slub.c: page is always non-NULL in node_match()") Signed-off-by: Laurent Dufour ldufour@linux.ibm.com
With the expanded changelog,
Acked-by: Vlastimil Babka vbabka@suse.cz Thanks!
Cc: Christoph Lameter cl@linux.com Cc: Pekka Enberg penberg@kernel.org Cc: David Rientjes rientjes@google.com Cc: Joonsoo Kim iamjoonsoo.kim@lge.com Cc: Andrew Morton akpm@linux-foundation.org Cc: stable@vger.kernel.org
mm/slub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c index 8f66de8a5ab3..7dc5c6aaf4b7 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2852,7 +2852,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s, object = c->freelist; page = c->page;
- if (unlikely(!object || !node_match(page, node))) {
- if (unlikely(!object || !page || !node_match(page, node))) { object = __slab_alloc(s, gfpflags, node, addr, c); } else { void *next_object = get_freepointer_safe(s, object);
-- 2.29.1