SLAB: Fix a couple of slab memory hotadd issues

When fallback_alloc() runs the node of the CPU might not be initialized yet. Handle this case by allocating in another node.

Signed-off-by Andi Kleen <[email protected]>

---
 mm/slab.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Index: linux-2.6.33-rc3-ak/mm/slab.c
===================================================================

mm/slab.c +1 -1

@@ -3210,7 +3210,24 @@ retry:
 		if (local_flags & __GFP_WAIT)
 			local_irq_enable();
 		kmem_flagcheck(cache, flags);
-		obj = kmem_getpages(cache, local_flags, numa_node_id());
+

Quoted 3 lines · Andi Kleen

When fallback_alloc() runs the node of the CPU might not be initialized yet. Handle this case by allocating in another node.

That other node must be allowed by current's cpuset, otherwise kmem_getpages() will fail when get_page_from_freelist() iterates only over unallowed nodes.

Quoted 3 lines

That other node must be allowed by current's cpuset, otherwise kmem_getpages() will fail when get_page_from_freelist() iterates only over unallowed nodes.

All theses cases are really only interesting in the memory hotplug path itself (afterwards the slab is working anyways and memory is there) and if someone sets funny cpusets for those he gets what he deserves ...

If a hot-added node has not been initialized for the cache, your code is picking an existing one in zonelist order which may be excluded by current's cpuset. Thus, your code has a very real chance of having kmem_getpages() return NULL because get_page_from_freelist() will reject non-atomic ALLOC_CPUSET allocations for prohibited nodes. That isn't a scenario that requires a "funny cpuset," it just has to not allow whatever initialized node comes first in the zonelist.

My suggested alternative does not pick a single initialized node, rather it tries all nodes that actually have a chance of having kmem_getpages() succeed which increases the probability that your patch actually has an effect for cpuset users.

Quoted 7 lines

If a hot-added node has not been initialized for the cache, your code is picking an existing one in zonelist order which may be excluded by current's cpuset. Thus, your code has a very real chance of having kmem_getpages() return NULL because get_page_from_freelist() will reject non-atomic ALLOC_CPUSET allocations for prohibited nodes. That isn't a scenario that requires a "funny cpuset," it just has to not allow whatever initialized node comes first in the zonelist.

The point was that you would need to run whoever triggers the memory hotadd in a cpuset with limitations. That would be a clear don't do that if hurts(tm)

Requires previous refactor patch.

Index: linux-2.6.33-rc3-ak/mm/slab.c
===================================================================

mm/slab.c +1 -0

@@ -115,6 +115,7 @@
 #include	<linux/reciprocal_div.h>
 #include	<linux/debugobjects.h>
 #include	<linux/kmemcheck.h>
+#include	<linux/memory.h>
 
 #include	<asm/cacheflush.h>
 #include	<asm/tlbflush.h>

mm/slab.c +10 -0

+
+	/*
+	 * When a node goes online allocate l3s early.	 This way
+	 * kmalloc_node() works for it.
+	 */
+	if (action == MEM_ONLINE && mn->status_change_nid >= 0)
+		slab_node_prepare(mn->status_change_nid);
+	return NOTIFY_OK;
+}
+
 void __init kmem_cache_init_late(void)
 {
 	struct kmem_cache *cachep;

mm/slab.c void __init kmem_cache_init(void) · line 1567

Quoted 4 lines

mm/slab.c +4 -0

+static int slab_memory_callback(struct notifier_block *self,
+				unsigned long action, void *arg)
+{
+	struct memory_notify *mn = (struct memory_notify *)arg;

No cast necessary.

mm/slab.c void __init kmem_cache_init_late(void) · line 1601

Quoted 8 lines

void __init kmem_cache_init_late(void)
{

mm/slab.c +1 -0

 	struct kmem_cache *cachep;

@@ -1583,6 +1598,8 @@ void __init kmem_cache_init_late(void)
 	 */
 	register_cpu_notifier(&cpucache_notifier);
 
+	hotplug_memory_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);

Only needed for CONFIG_NUMA.

Quoted 6 lines

mm/slab.c void __init kmem_cache_init(void) · line 1567

Quoted 4 lines

mm/slab.c +4 -0

+static int slab_memory_callback(struct notifier_block *self,
+				unsigned long action, void *arg)
+{
+	struct memory_notify *mn = (struct memory_notify *)arg;

No cast necessary.

It's standard practice to cast void *.

No functional changes.

Needed for next patch.

Signed-off-by Andi Kleen <[email protected]>

---
 mm/slab.c |   34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

Index: linux-2.6.33-rc3-ak/mm/slab.c
===================================================================

mm/slab.c +1 -4

@@ -1171,19 +1171,9 @@ free_array_cache:
 	}
 }
 
-static int __cpuinit cpuup_prepare(long cpu)
+static int slab_node_prepare(int node)
 {
 	struct kmem_cache *cachep;
-	struct kmem_list3 *l3 = NULL;
-	int node = cpu_to_node(cpu);
-	const int memsize = sizeof(struct kmem_list3);

mm/slab.c +13 -2

@@ -1192,9 +1182,10 @@ static int __cpuinit cpuup_prepare(long
 		 * node has not already allocated this
 		 */
 		if (!cachep->nodelists[node]) {
-			l3 = kmalloc_node(memsize, GFP_KERNEL, node);
+			struct kmem_list3 *l3;
+			l3 = kmalloc_node(sizeof(struct kmem_list3), GFP_KERNEL, node);
 			if (!l3)
-				goto bad;
+				return -1;
 			kmem_list3_init(l3);
 			l3->next_reap = jiffies + REAPTIMEOUT_LIST3 +
 			    ((unsigned long)cachep) % REAPTIMEOUT_LIST3;

@@ -1213,6 +1204,23 @@ static int __cpuinit cpuup_prepare(long
 			cachep->batchcount + cachep->num;
 		spin_unlock_irq(&cachep->nodelists[node]->list_lock);
 	}
+	return 0;
+}
+
+static int __cpuinit cpuup_prepare(long cpu)
+{
+	struct kmem_cache *cachep;
+	struct kmem_list3 *l3 = NULL;
+	int node = cpu_to_node(cpu);
+
+	/*

As Christoph mentioned, this patch is out of order with the previous one

in the series; slab_node_prepare() is called in that previous patch by a memory hotplug callback without holding cache_chain_mutex (it's taken by the cpu hotplug callback prior to calling cpuup_prepare() currently). So slab_node_prepare() should note that we require the mutex and the memory hotplug callback should take it in the previous patch.

Quoted 5 lines

in the series; slab_node_prepare() is called in that previous patch by a memory hotplug callback without holding cache_chain_mutex (it's taken by the cpu hotplug callback prior to calling cpuup_prepare() currently). So slab_node_prepare() should note that we require the mutex and the memory hotplug callback should take it in the previous patch.

AFAIK the code is correct. If you feel the need for additional documentation feel free to send patches yourself.

This fixes various problems in slab found during memory hotadd testing.

All straight forward bug fixes, so could be still .33 candidates.

This fixes various problems in slab found during memory hotadd testing.

Quoted 1 line · Andi Kleen

This fixes various problems in slab found during memory hotadd testing.

It changes the bootstrap semantics. The requirement was so far that slab initialization must be complete before slab operations can be used.

This patchset allows such use before bootstrap on a node is complete and also allows the running of cache reaper before bootstrap is done.

I have a bad feeling that this could be the result of Pekka's changes to the bootstrap.

Quoted 2 lines

It changes the bootstrap semantics. The requirement was so far that slab initialization must be complete before slab operations can be used.

The problem is that slab itself uses slab it initialize itself.

Quoted 5 lines

This patchset allows such use before bootstrap on a node is complete and also allows the running of cache reaper before bootstrap is done.

I have a bad feeling that this could be the result of Pekka's changes to the bootstrap.

Not sure I fully follow what you're saying.

Are you saying this is a regression fix after all?