[S+Q Cleanup3 0/6] SLUB: Cleanups V3

Christoph Lameter

author

2010-08-19 20:34 UTC #0 lore raw

cover SLUB: Cleanups V3

V1->V2: Fixes as discussed with David.
V2->V3: More deeper fixes. Return pointer to kmem_cache from create_kmalloc_cache.

These are just the 6 remaining cleanup patches (after the 2.6.36 merge got the other in) in preparation for the Unified patches.

I think it may be best to first try to merge these and make sure that they are fine before we go step by step through the unification patches. I hope they can go into -next.

Patch 1

Uninline debug functions in hot paths. There is no point of the compiler folding them in because they are typically unused.

Patch 2

Remove dynamic creation of DMA caches and create them statically (will be turned dynamic by patch 4 but will then always be preallocated on boot and not from the hotpath)

Patch 3

Remove static allocation of kmem_cache_cpu array and rely on the percpu allocator to allocate memory for the array on bootup.

Patch 4

Remove static allocation of kmem_cache structure for kmalloc and friends.

Patch 5

Extract hooks for memory checkers.

Patch 6

Move gfpflag masking out of the allocator hotpath

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #1 lore raw

PATCH 1/6 Slub: Force no inlining of debug functions

Compiler folds the debgging functions into the critical paths. Avoid that by adding noinline to the functions that check for problems.

Acked-by David Rientjes <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

mm/slub.c +3 -3

Index: linux-2.6/mm/slub.c
===================================================================
@@ -862,7 +862,7 @@ static void setup_object_debug(struct km
 	init_tracking(s, object);
 }
 
-static int alloc_debug_processing(struct kmem_cache *s, struct page *page,
+static noinline int alloc_debug_processing(struct kmem_cache *s, struct page *page,
 					void *object, unsigned long addr)
 {
 	if (!check_slab(s, page))

@@ -902,8 +902,8 @@ bad:
 	return 0;
 }
 
-static int free_debug_processing(struct kmem_cache *s, struct page *page,
-					void *object, unsigned long addr)
+static noinline int free_debug_processing(struct kmem_cache *s,
+		 struct page *page, void *object, unsigned long addr)
 {
 	if (!check_slab(s, page))
 		goto fail;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #2 lore raw

PATCH 2/6 slub: remove dynamic dma slab allocation

In reply to Christoph Lameter

Remove the dynamic dma slab allocation since this causes too many issues with nested locks etc etc. The change avoids passing gfpflags into many functions.

V3->V4:
- Create dma caches in kmem_cache_init() instead of kmem_cache_init_late().

Acked-by David Rientjes <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |  150 ++++++++++++++++----------------------------------------------
 1 file changed, 39 insertions(+), 111 deletions(-)

mm/slub.c +39 ~~-111~~

Index: linux-2.6/mm/slub.c
===================================================================
@@ -2064,7 +2064,7 @@ init_kmem_cache_node(struct kmem_cache_n
 
 static DEFINE_PER_CPU(struct kmem_cache_cpu, kmalloc_percpu[KMALLOC_CACHES]);
 
-static inline int alloc_kmem_cache_cpus(struct kmem_cache *s, gfp_t flags)
+static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 {
 	if (s < kmalloc_caches + KMALLOC_CACHES && s >= kmalloc_caches)
 		/*

@@ -2091,7 +2091,7 @@ static inline int alloc_kmem_cache_cpus(
  * when allocating for the kmalloc_node_cache. This is used for bootstrapping
  * memory on a fresh node that has no slab structures yet.
  */
-static void early_kmem_cache_node_alloc(gfp_t gfpflags, int node)
+static void early_kmem_cache_node_alloc(int node)
 {
 	struct page *page;
 	struct kmem_cache_node *n;

@@ -2099,7 +2099,7 @@ static void early_kmem_cache_node_alloc(
 
 	BUG_ON(kmalloc_caches->size < sizeof(struct kmem_cache_node));
 
-	page = new_slab(kmalloc_caches, gfpflags, node);
+	page = new_slab(kmalloc_caches, GFP_NOWAIT, node);
 
 	BUG_ON(!page);
 	if (page_to_nid(page) != node) {

@@ -2143,7 +2143,7 @@ static void free_kmem_cache_nodes(struct
 	}
 }
 
-static int init_kmem_cache_nodes(struct kmem_cache *s, gfp_t gfpflags)
+static int init_kmem_cache_nodes(struct kmem_cache *s)
 {
 	int node;

@@ -2151,11 +2151,11 @@ static int init_kmem_cache_nodes(struct 
 		struct kmem_cache_node *n;
 
 		if (slab_state == DOWN) {
-			early_kmem_cache_node_alloc(gfpflags, node);
+			early_kmem_cache_node_alloc(node);
 			continue;
 		}
 		n = kmem_cache_alloc_node(kmalloc_caches,
-						gfpflags, node);
+						GFP_KERNEL, node);
 
 		if (!n) {
 			free_kmem_cache_nodes(s);

@@ -2172,7 +2172,7 @@ static void free_kmem_cache_nodes(struct
 {
 }
 
-static int init_kmem_cache_nodes(struct kmem_cache *s, gfp_t gfpflags)
+static int init_kmem_cache_nodes(struct kmem_cache *s)
 {
 	init_kmem_cache_node(&s->local_node, s);
 	return 1;

@@ -2312,7 +2312,7 @@ static int calculate_sizes(struct kmem_c
 
 }
 
-static int kmem_cache_open(struct kmem_cache *s, gfp_t gfpflags,
+static int kmem_cache_open(struct kmem_cache *s,
 		const char *name, size_t size,
 		size_t align, unsigned long flags,
 		void (*ctor)(void *))

@@ -2348,10 +2348,10 @@ static int kmem_cache_open(struct kmem_c
 #ifdef CONFIG_NUMA
 	s->remote_node_defrag_ratio = 1000;
 #endif
-	if (!init_kmem_cache_nodes(s, gfpflags & ~SLUB_DMA))
+	if (!init_kmem_cache_nodes(s))
 		goto error;
 
-	if (alloc_kmem_cache_cpus(s, gfpflags & ~SLUB_DMA))
+	if (alloc_kmem_cache_cpus(s))
 		return 1;
 
 	free_kmem_cache_nodes(s);

@@ -2510,6 +2510,10 @@ EXPORT_SYMBOL(kmem_cache_destroy);
 struct kmem_cache kmalloc_caches[KMALLOC_CACHES] __cacheline_aligned;
 EXPORT_SYMBOL(kmalloc_caches);
 
+#ifdef CONFIG_ZONE_DMA
+static struct kmem_cache kmalloc_dma_caches[SLUB_PAGE_SHIFT];
+#endif
+
 static int __init setup_slub_min_order(char *str)
 {
 	get_option(&str, &slub_min_order);

@@ -2546,116 +2550,26 @@ static int __init setup_slub_nomerge(cha
 
 __setup("slub_nomerge", setup_slub_nomerge);
 
-static struct kmem_cache *create_kmalloc_cache(struct kmem_cache *s,
-		const char *name, int size, gfp_t gfp_flags)
+static void create_kmalloc_cache(struct kmem_cache *s,
+		const char *name, int size, unsigned int flags)
 {
-	unsigned int flags = 0;
-
-	if (gfp_flags & SLUB_DMA)
-		flags = SLAB_CACHE_DMA;
-
 	/*
 	 * This function is called with IRQs disabled during early-boot on
 	 * single CPU so there's no need to take slub_lock here.
 	 */
-	if (!kmem_cache_open(s, gfp_flags, name, size, ARCH_KMALLOC_MINALIGN,
+	if (!kmem_cache_open(s, name, size, ARCH_KMALLOC_MINALIGN,
 								flags, NULL))
 		goto panic;
 
 	list_add(&s->list, &slab_caches);
 
-	if (sysfs_slab_add(s))
-		goto panic;
-	return s;
+	if (!sysfs_slab_add(s))
+		return;
 
 panic:
 	panic("Creation of kmalloc slab %s size=%d failed.\n", name, size);
 }
 
-#ifdef CONFIG_ZONE_DMA
-static struct kmem_cache *kmalloc_caches_dma[SLUB_PAGE_SHIFT];
-
-static void sysfs_add_func(struct work_struct *w)
-{
-	struct kmem_cache *s;
-
-	down_write(&slub_lock);
-	list_for_each_entry(s, &slab_caches, list) {
-		if (s->flags & __SYSFS_ADD_DEFERRED) {
-			s->flags &= ~__SYSFS_ADD_DEFERRED;
-			sysfs_slab_add(s);
-		}
-	}
-	up_write(&slub_lock);
-}
-
-static DECLARE_WORK(sysfs_add_work, sysfs_add_func);
-
-static noinline struct kmem_cache *dma_kmalloc_cache(int index, gfp_t flags)
-{
-	struct kmem_cache *s;
-	char *text;
-	size_t realsize;
-	unsigned long slabflags;
-	int i;
-
-	s = kmalloc_caches_dma[index];
-	if (s)
-		return s;
-
-	/* Dynamically create dma cache */
-	if (flags & __GFP_WAIT)
-		down_write(&slub_lock);
-	else {
-		if (!down_write_trylock(&slub_lock))
-			goto out;
-	}
-
-	if (kmalloc_caches_dma[index])
-		goto unlock_out;
-
-	realsize = kmalloc_caches[index].objsize;
-	text = kasprintf(flags & ~SLUB_DMA, "kmalloc_dma-%d",
-			 (unsigned int)realsize);
-
-	s = NULL;
-	for (i = 0; i < KMALLOC_CACHES; i++)
-		if (!kmalloc_caches[i].size)
-			break;
-
-	BUG_ON(i >= KMALLOC_CACHES);
-	s = kmalloc_caches + i;
-
-	/*
-	 * Must defer sysfs creation to a workqueue because we don't know
-	 * what context we are called from. Before sysfs comes up, we don't
-	 * need to do anything because our sysfs initcall will start by
-	 * adding all existing slabs to sysfs.
-	 */
-	slabflags = SLAB_CACHE_DMA|SLAB_NOTRACK;
-	if (slab_state >= SYSFS)
-		slabflags |= __SYSFS_ADD_DEFERRED;
-
-	if (!text || !kmem_cache_open(s, flags, text,
-			realsize, ARCH_KMALLOC_MINALIGN, slabflags, NULL)) {
-		s->size = 0;
-		kfree(text);
-		goto unlock_out;
-	}
-
-	list_add(&s->list, &slab_caches);
-	kmalloc_caches_dma[index] = s;
-
-	if (slab_state >= SYSFS)
-		schedule_work(&sysfs_add_work);
-
-unlock_out:
-	up_write(&slub_lock);
-out:
-	return kmalloc_caches_dma[index];
-}
-#endif
-
 /*
  * Conversion table for small slabs sizes / 8 to the index in the
  * kmalloc array. This is necessary for slabs < 192 since we have non power

@@ -2708,7 +2622,7 @@ static struct kmem_cache *get_slab(size_
 
 #ifdef CONFIG_ZONE_DMA
 	if (unlikely((flags & SLUB_DMA)))
-		return dma_kmalloc_cache(index, flags);
+		return &kmalloc_dma_caches[index];
 
 #endif
 	return &kmalloc_caches[index];

@@ -3047,7 +2961,7 @@ void __init kmem_cache_init(void)
 	 * kmem_cache_open for slab_state == DOWN.
 	 */
 	create_kmalloc_cache(&kmalloc_caches[0], "kmem_cache_node",
-		sizeof(struct kmem_cache_node), GFP_NOWAIT);
+		sizeof(struct kmem_cache_node), 0);
 	kmalloc_caches[0].refcount = -1;
 	caches++;

@@ -3060,18 +2974,18 @@ void __init kmem_cache_init(void)
 	/* Caches that are not of the two-to-the-power-of size */
 	if (KMALLOC_MIN_SIZE <= 32) {
 		create_kmalloc_cache(&kmalloc_caches[1],
-				"kmalloc-96", 96, GFP_NOWAIT);
+				"kmalloc-96", 96, 0);
 		caches++;
 	}
 	if (KMALLOC_MIN_SIZE <= 64) {
 		create_kmalloc_cache(&kmalloc_caches[2],
-				"kmalloc-192", 192, GFP_NOWAIT);
+				"kmalloc-192", 192, 0);
 		caches++;
 	}
 
 	for (i = KMALLOC_SHIFT_LOW; i < SLUB_PAGE_SHIFT; i++) {
 		create_kmalloc_cache(&kmalloc_caches[i],
-			"kmalloc", 1 << i, GFP_NOWAIT);
+			"kmalloc", 1 << i, 0);
 		caches++;
 	}

@@ -3134,6 +3048,20 @@ void __init kmem_cache_init(void)
 	kmem_size = sizeof(struct kmem_cache);
 #endif
 
+#ifdef CONFIG_ZONE_DMA
+	for (i = 1; i < SLUB_PAGE_SHIFT; i++) {
+		struct kmem_cache *s = &kmalloc_caches[i];
+
+		if (s->size) {
+			char *name = kasprintf(GFP_NOWAIT,
+				 "dma-kmalloc-%d", s->objsize);
+
+			BUG_ON(!name);
+			create_kmalloc_cache(&kmalloc_dma_caches[i],
+				name, s->objsize, SLAB_CACHE_DMA);
+		}
+	}
+#endif
 	printk(KERN_INFO
 		"SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, MinObjects=%d,"
 		" CPUs=%d, Nodes=%d\n",

@@ -3236,7 +3164,7 @@ struct kmem_cache *kmem_cache_create(con
 
 	s = kmalloc(kmem_size, GFP_KERNEL);
 	if (s) {
-		if (kmem_cache_open(s, GFP_KERNEL, name,
+		if (kmem_cache_open(s, name,
 				size, align, flags, ctor)) {
 			list_add(&s->list, &slab_caches);
 			if (sysfs_slab_add(s)) {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #3 lore raw

PATCH 3/6 slub: Remove static kmem_cache_cpu array for boot

In reply to Christoph Lameter

The percpu allocator can now handle allocations during early boot. So drop the static kmem_cache_cpu array.

Cc Tejun Heo <[email protected]>

Acked-by David Rientjes <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |   17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

mm/slub.c +4 ~~-13~~

Index: linux-2.6/mm/slub.c
===================================================================
@@ -2062,23 +2062,14 @@ init_kmem_cache_node(struct kmem_cache_n
 #endif
 }
 
-static DEFINE_PER_CPU(struct kmem_cache_cpu, kmalloc_percpu[KMALLOC_CACHES]);
-
 static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 {
-	if (s < kmalloc_caches + KMALLOC_CACHES && s >= kmalloc_caches)
-		/*
-		 * Boot time creation of the kmalloc array. Use static per cpu data
-		 * since the per cpu allocator is not available yet.
-		 */
-		s->cpu_slab = kmalloc_percpu + (s - kmalloc_caches);
-	else
-		s->cpu_slab =  alloc_percpu(struct kmem_cache_cpu);
+	BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
+			SLUB_PAGE_SHIFT * sizeof(struct kmem_cache_cpu));
 
-	if (!s->cpu_slab)
-		return 0;
+	s->cpu_slab = alloc_percpu(struct kmem_cache_cpu);
 
-	return 1;
+	return s->cpu_slab != NULL;
 }
 
 #ifdef CONFIG_NUMA

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #4 lore raw

PATCH 4/6 slub: Dynamically size kmalloc cache allocations

In reply to Christoph Lameter

kmalloc caches are statically defined and may take up a lot of space just because the sizes of the node array has to be dimensioned for the largest node count supported.

This patch makes the size of the kmem_cache structure dynamic throughout by creating a kmem_cache slab cache for the kmem_cache objects. The bootstrap occurs by allocating the initial one or two kmem_cache objects from the page allocator.

C2->C3
- Fix various issues indicated by David
- Make create kmalloc_cache return a kmem_cache * pointer.

Signed-off-by Christoph Lameter <[email protected]>

---
 include/linux/slub_def.h |    7 -
 mm/slub.c                |  188 ++++++++++++++++++++++++++++++++++-------------
 2 files changed, 140 insertions(+), 55 deletions(-)

include/linux/slub_def.h +2 -5

Index: linux-2.6/include/linux/slub_def.h
===================================================================
@@ -139,19 +139,16 @@ struct kmem_cache {
 
 #ifdef CONFIG_ZONE_DMA
 #define SLUB_DMA __GFP_DMA
-/* Reserve extra caches for potential DMA use */
-#define KMALLOC_CACHES (2 * SLUB_PAGE_SHIFT)
 #else
 /* Disable DMA functionality */
 #define SLUB_DMA (__force gfp_t)0
-#define KMALLOC_CACHES SLUB_PAGE_SHIFT
 #endif
 
 /*
  * We keep the general caches in an array of slab caches that are used for
  * 2^x bytes of allocations.
  */
-extern struct kmem_cache kmalloc_caches[KMALLOC_CACHES];
+extern struct kmem_cache *kmalloc_caches[SLUB_PAGE_SHIFT];
 
 /*
  * Sorry that the following has to be that ugly but some versions of GCC

@@ -216,7 +213,7 @@ static __always_inline struct kmem_cache
 	if (index == 0)
 		return NULL;
 
-	return &kmalloc_caches[index];
+	return kmalloc_caches[index];
 }
 
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t);

mm/slub.c +138 ~~-50~~

312 lines not on this page · raw message

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

reviewer Acked-by on v1 4/6

2010-08-19 21:21 UTC #5 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 4 lines · Christoph Lameter

mm/slub.c +0 -0

@@ -2940,46 +2951,113 @@ static int slab_memory_callback(struct n
  *			Basic setup of slabs
  *******************************************************************/

mm/slub.c static int slab_memory_callback(struct n · line 2963

Quoted 10 lines

mm/slub.c +10 -0

+/*
+ * Used for early kmem_cache structures that were allocated using
+ * the page allocator
+ */
+
+static void __init kmem_cache_bootstrap_fixup(struct kmem_cache *s)
+{
+	int node;
+
+	list_add(&s->list, &slab_caches);

Since sysfs_slab_add() has been removed for kmem_cache and kmem_cache_node here, they apparently don't need the __SYSFS_ADD_DEFERRED flag even though we're waiting for the sysfs initcall since there's nothing that checks for it. That bit can be removed, the last users of it were the dynamic DMA cache support that was dropped in patch 2.

Acked-by David Rientjes <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 21:38 UTC #6 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 5 lines · David Rientjes

Since sysfs_slab_add() has been removed for kmem_cache and kmem_cache_node here, they apparently don't need the __SYSFS_ADD_DEFERRED flag even though we're waiting for the sysfs initcall since there's nothing that checks for it. That bit can be removed, the last users of it were the dynamic DMA cache support that was dropped in patch 2.

Correct. Then we also do not need the sysfs_slab_add in create_kmalloc_cache.

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

mm/slub.c +1 -4

Index: linux-2.6/mm/slub.c
===================================================================
@@ -148,7 +148,6 @@ static inline int kmem_cache_debug(struc
 
 /* Internal SLUB flags */
 #define __OBJECT_POISON		0x80000000UL /* Poison object */
-#define __SYSFS_ADD_DEFERRED	0x40000000UL /* Not yet visible via sysfs */
 #define __ALIEN_CACHE		0x20000000UL /* Slab has alien caches */
 
 static inline int aliens(struct kmem_cache *s)

@@ -3123,9 +3122,7 @@ static struct kmem_cache *__init create_
 		goto panic;
 
 	list_add(&s->list, &slab_caches);
-
-	if (!sysfs_slab_add(s))
-		return s;
+	return s;
 
 panic:
 	panic("Creation of kmalloc slab %s size=%d failed.\n", name, size);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

reviewer Acked-by

2010-08-19 23:01 UTC #7 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 4 lines · Christoph Lameter

Correct. Then we also do not need the sysfs_slab_add in create_kmalloc_cache.
Signed-off-by Christoph Lameter <[email protected]>

This doesn't apply on top of this patchset, it was generated from the entire SLUB+Q patchset (we don't have __ALIEN_CACHE yet). Besides the conflicts, the patch is good.

Acked-by David Rientjes <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 23:20 UTC #8 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 5 lines · David Rientjes

Quoted 3 lines · Christoph Lameter
Correct. Then we also do not need the sysfs_slab_add in create_kmalloc_cache.

Quoted 7 lines

Quoted 1 line
Signed-off-by Christoph Lameter <[email protected]>
This doesn't apply on top of this patchset, it was generated from the entire SLUB+Q patchset (we don't have __ALIEN_CACHE yet). Besides the conflicts, the patch is good.
Acked-by David Rientjes <[email protected]>

Right. I will merge this correctly for the next release that has all patches acked by you.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

reviewer

2010-08-19 23:39 UTC #9 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 3 lines · Christoph Lameter

Right. I will merge this correctly for the next release that has all patches acked by you.

It would really be nice to get rid of all the #ifdefs in kmem_cache_init() for CONFIG_NUMA by extracting them to helper functions if you're interested.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2010-08-20 17:08 UTC #10 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 9 lines · David Rientjes

Quoted 3 lines · Christoph Lameter
Right. I will merge this correctly for the next release that has all patches acked by you.
It would really be nice to get rid of all the #ifdefs in kmem_cache_init() for CONFIG_NUMA by extracting them to helper functions if you're interested.

That is difficult since the code segments share global and local variables. Also the location of the kmem_cache_node structure differs since it is embedded in the SMP case. We could just drop that optimization. Then it would be easy to drop lots of NUMA specifics in the allocator.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2010-08-20 17:33 UTC #11 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 3 lines

Quoted 3 lines
It would really be nice to get rid of all the #ifdefs in kmem_cache_init() for CONFIG_NUMA by extracting them to helper functions if you're interested.

Draft patch to drop SMP particularities.

---
 include/linux/slub_def.h |    5 +----
 mm/slub.c                |   39 +--------------------------------------
 2 files changed, 2 insertions(+), 42 deletions(-)

include/linux/slub_def.h +1 -4

Index: linux-2.6/include/linux/slub_def.h
===================================================================
@@ -96,11 +96,8 @@ struct kmem_cache {
 	 * Defragmentation by allocating from a remote node.
 	 */
 	int remote_node_defrag_ratio;
-	struct kmem_cache_node *node[MAX_NUMNODES];
-#else
-	/* Avoid an extra cache line for UP */
-	struct kmem_cache_node local_node;
 #endif
+	struct kmem_cache_node *node[MAX_NUMNODES];
 };
 
 /*

mm/slub.c +1 ~~-38~~

Index: linux-2.6/mm/slub.c
===================================================================
@@ -232,11 +232,7 @@ int slab_is_available(void)
 
 static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
 {
-#ifdef CONFIG_NUMA
 	return s->node[node];
-#else
-	return &s->local_node;
-#endif
 }
 
 /* Verify that a pointer has an address that is valid within a slab page */

@@ -837,7 +833,7 @@ static inline void inc_slabs_node(struct
 	 * dilemma by deferring the increment of the count during
 	 * bootstrap (see early_kmem_cache_node_alloc).
 	 */
-	if (!NUMA_BUILD || n) {
+	if (n) {
 		atomic_long_inc(&n->nr_slabs);
 		atomic_long_add(objects, &n->total_objects);
 	}

@@ -2071,7 +2067,6 @@ static inline int alloc_kmem_cache_cpus(
 	return s->cpu_slab != NULL;
 }
 
-#ifdef CONFIG_NUMA
 static struct kmem_cache *kmem_cache_node;
 
 /*

@@ -2161,17 +2156,6 @@ static int init_kmem_cache_nodes(struct
 	}
 	return 1;
 }
-#else
-static void free_kmem_cache_nodes(struct kmem_cache *s)
-{
-}
-
-static int init_kmem_cache_nodes(struct kmem_cache *s)
-{
-	init_kmem_cache_node(&s->local_node, s);
-	return 1;
-}
-#endif
 
 static void set_min_partial(struct kmem_cache *s, unsigned long min)
 {

@@ -2982,8 +2966,6 @@ void __init kmem_cache_init(void)
 	int caches = 0;
 	struct kmem_cache *temp_kmem_cache;
 	int order;
-
-#ifdef CONFIG_NUMA
 	struct kmem_cache *temp_kmem_cache_node;
 	unsigned long kmalloc_size;

@@ -3007,12 +2989,6 @@ void __init kmem_cache_init(void)
 		0, SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
 
 	hotplug_memory_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);
-#else
-	/* Allocate a single kmem_cache from the page allocator */
-	kmem_size = sizeof(struct kmem_cache);
-	order = get_order(kmem_size);
-	kmem_cache = (void *)__get_free_pages(GFP_NOWAIT, order);
-#endif
 
 	/* Able to allocate the per node structures */
 	slab_state = PARTIAL;

@@ -3023,7 +2999,6 @@ void __init kmem_cache_init(void)
 	kmem_cache = kmem_cache_alloc(kmem_cache, GFP_NOWAIT);
 	memcpy(kmem_cache, temp_kmem_cache, kmem_size);
 
-#ifdef CONFIG_NUMA
 	/*
 	 * Allocate kmem_cache_node properly from the kmem_cache slab.
 	 * kmem_cache_node is separately allocated so no need to

@@ -3037,18 +3012,6 @@ void __init kmem_cache_init(void)
 	kmem_cache_bootstrap_fixup(kmem_cache_node);
 
 	caches++;
-#else
-	/*
-	 * kmem_cache has kmem_cache_node embedded and we moved it!
-	 * Update the list heads
-	 */
-	INIT_LIST_HEAD(&kmem_cache->local_node.partial);
-	list_splice(&temp_kmem_cache->local_node.partial, &kmem_cache->local_node.partial);
-#ifdef CONFIG_SLUB_DEBUG
-	INIT_LIST_HEAD(&kmem_cache->local_node.full);
-	list_splice(&temp_kmem_cache->local_node.full, &kmem_cache->local_node.full);
-#endif
-#endif
 	kmem_cache_bootstrap_fixup(kmem_cache);
 	caches++;
 	/* Free temporary boot structure */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

reviewer

2010-09-28 10:06 UTC #12 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 2 lines · Christoph Lameter

Draft patch to drop SMP particularities.

s/SMP/NUMA/

Quoted 113 lines

---
 include/linux/slub_def.h |    5 +----
 mm/slub.c                |   39 +--------------------------------------
 2 files changed, 2 insertions(+), 42 deletions(-)

Index: linux-2.6/include/linux/slub_def.h
===================================================================

include/linux/slub_def.h +1 -4

@@ -96,11 +96,8 @@ struct kmem_cache {
 	 * Defragmentation by allocating from a remote node.
 	 */
 	int remote_node_defrag_ratio;
-	struct kmem_cache_node *node[MAX_NUMNODES];
-#else
-	/* Avoid an extra cache line for UP */
-	struct kmem_cache_node local_node;
 #endif
+	struct kmem_cache_node *node[MAX_NUMNODES];
 };
 
 /*

Index: linux-2.6/mm/slub.c
===================================================================

mm/slub.c +1 ~~-31~~

@@ -232,11 +232,7 @@ int slab_is_available(void)
 
 static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
 {
-#ifdef CONFIG_NUMA
 	return s->node[node];
-#else
-	return &s->local_node;
-#endif
 }
 
 /* Verify that a pointer has an address that is valid within a slab page */

@@ -837,7 +833,7 @@ static inline void inc_slabs_node(struct
 	 * dilemma by deferring the increment of the count during
 	 * bootstrap (see early_kmem_cache_node_alloc).
 	 */
-	if (!NUMA_BUILD || n) {
+	if (n) {
 		atomic_long_inc(&n->nr_slabs);
 		atomic_long_add(objects, &n->total_objects);
 	}

@@ -2071,7 +2067,6 @@ static inline int alloc_kmem_cache_cpus(
 	return s->cpu_slab != NULL;
 }
 
-#ifdef CONFIG_NUMA
 static struct kmem_cache *kmem_cache_node;
 
 /*

@@ -2161,17 +2156,6 @@ static int init_kmem_cache_nodes(struct
 	}
 	return 1;
 }
-#else
-static void free_kmem_cache_nodes(struct kmem_cache *s)
-{
-}
-
-static int init_kmem_cache_nodes(struct kmem_cache *s)
-{
-	init_kmem_cache_node(&s->local_node, s);
-	return 1;
-}
-#endif
 
 static void set_min_partial(struct kmem_cache *s, unsigned long min)
 {

@@ -2982,8 +2966,6 @@ void __init kmem_cache_init(void)
 	int caches = 0;
 	struct kmem_cache *temp_kmem_cache;
 	int order;
-
-#ifdef CONFIG_NUMA
 	struct kmem_cache *temp_kmem_cache_node;
 	unsigned long kmalloc_size;

@@ -3007,12 +2989,6 @@ void __init kmem_cache_init(void)
 		0, SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
 
 	hotplug_memory_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);
-#else
-	/* Allocate a single kmem_cache from the page allocator */
-	kmem_size = sizeof(struct kmem_cache);
-	order = get_order(kmem_size);
-	kmem_cache = (void *)__get_free_pages(GFP_NOWAIT, order);
-#endif
 
 	/* Able to allocate the per node structures */
 	slab_state = PARTIAL;

@@ -3023,7 +2999,6 @@ void __init kmem_cache_init(void)
 	kmem_cache = kmem_cache_alloc(kmem_cache, GFP_NOWAIT);
 	memcpy(kmem_cache, temp_kmem_cache, kmem_size);
 
-#ifdef CONFIG_NUMA
 	/*
 	 * Allocate kmem_cache_node properly from the kmem_cache slab.
 	 * kmem_cache_node is separately allocated so no need to

@@ -3037,18 +3012,6 @@ void __init kmem_cache_init(void)
 	kmem_cache_bootstrap_fixup(kmem_cache_node);
 
 	caches++;
-#else
-	/*
-	 * kmem_cache has kmem_cache_node embedded and we moved it!
-	 * Update the list heads
-	 */

mm/slub.c void __init kmem_cache_init(void) · line 3017

Quoted 10 lines

mm/slub.c +0 -7

-	INIT_LIST_HEAD(&kmem_cache->local_node.partial);
-	list_splice(&temp_kmem_cache->local_node.partial, &kmem_cache->local_node.partial);
-#ifdef CONFIG_SLUB_DEBUG
-	INIT_LIST_HEAD(&kmem_cache->local_node.full);
-	list_splice(&temp_kmem_cache->local_node.full, &kmem_cache->local_node.full);
-#endif
-#endif
 	kmem_cache_bootstrap_fixup(kmem_cache);
 	caches++;
 	/* Free temporary boot structure */

I really like this direction and I hope you push an updated version to Pekka because it cleans up a lot of the recently added init code without sacrificing any footprint for UMA.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2010-09-28 12:42 UTC #13 lore raw

Re: [S+Q Cleanup3 4/6] slub: Dynamically size kmalloc cache allocations

Quoted 2 lines · David Rientjes

Quoted 1 line
Draft patch to drop SMP particularities.
s/SMP/NUMA/

No its the special code for SMP. The local_node field is the main issue.

Quoted 3 lines

I really like this direction and I hope you push an updated version to Pekka because it cleans up a lot of the recently added init code without sacrificing any footprint for UMA.

Ok. I have another 2 cleanup patches here. Will update this and push all 3 all out today.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #14 lore raw

PATCH 5/6 slub: Extract hooks for memory checkers from hotpaths

In reply to Christoph Lameter

Extract the code that memory checkers and other verification tools use from the hotpaths. Makes it easier to add new ones and reduces the disturbances of the hotpaths.

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |   49 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 11 deletions(-)

mm/slub.c +38 ~~-11~~

Index: linux-2.6/mm/slub.c
===================================================================
@@ -792,6 +792,37 @@ static void trace(struct kmem_cache *s, 
 }
 
 /*
+ * Hooks for other subsystems that check memory allocations. In a typical
+ * production configuration these hooks all should produce no code at all.
+ */
+static inline int slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags)
+{
+	lockdep_trace_alloc(flags);
+	might_sleep_if(flags & __GFP_WAIT);
+
+	return should_failslab(s->objsize, flags, s->flags);
+}
+
+static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags, void *object)
+{
+	kmemcheck_slab_alloc(s, flags, object, s->objsize);
+	kmemleak_alloc_recursive(object, s->objsize, 1, s->flags, flags);
+}
+
+static inline void slab_free_hook(struct kmem_cache *s, void *x)
+{
+	kmemleak_free_recursive(x, s->flags);
+}
+
+static inline void slab_free_hook_irq(struct kmem_cache *s, void *object)
+{
+	kmemcheck_slab_free(s, object, s->objsize);
+	debug_check_no_locks_freed(object, s->objsize);
+	if (!(s->flags & SLAB_DEBUG_OBJECTS))
+		debug_check_no_obj_freed(object, s->objsize);
+}
+
+/*
  * Tracking of fully allocated slabs for debugging purposes.
  */
 static void add_full(struct kmem_cache_node *n, struct page *page)

@@ -1697,10 +1728,7 @@ static __always_inline void *slab_alloc(
 
 	gfpflags &= gfp_allowed_mask;
 
-	lockdep_trace_alloc(gfpflags);
-	might_sleep_if(gfpflags & __GFP_WAIT);
-
-	if (should_failslab(s->objsize, gfpflags, s->flags))
+	if (slab_pre_alloc_hook(s, gfpflags))
 		return NULL;
 
 	local_irq_save(flags);

@@ -1719,8 +1747,7 @@ static __always_inline void *slab_alloc(
 	if (unlikely(gfpflags & __GFP_ZERO) && object)
 		memset(object, 0, s->objsize);
 
-	kmemcheck_slab_alloc(s, gfpflags, object, s->objsize);
-	kmemleak_alloc_recursive(object, s->objsize, 1, s->flags, gfpflags);
+	slab_post_alloc_hook(s, gfpflags, object);
 
 	return object;
 }

@@ -1850,13 +1877,13 @@ static __always_inline void slab_free(st
 	struct kmem_cache_cpu *c;
 	unsigned long flags;
 
-	kmemleak_free_recursive(x, s->flags);
+	slab_free_hook(s, x);
+
 	local_irq_save(flags);
 	c = __this_cpu_ptr(s->cpu_slab);
-	kmemcheck_slab_free(s, object, s->objsize);
-	debug_check_no_locks_freed(object, s->objsize);
-	if (!(s->flags & SLAB_DEBUG_OBJECTS))
-		debug_check_no_obj_freed(object, s->objsize);
+
+	slab_free_hook_irq(s, x);
+
 	if (likely(page == c->page && c->node >= 0)) {
 		set_freepointer(s, object, c->freelist);
 		c->freelist = object;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

reviewer Acked-by on v1 5/6

2010-08-19 21:02 UTC #15 lore raw

Re: [S+Q Cleanup3 5/6] slub: Extract hooks for memory checkers from hotpaths

Quoted 5 lines · Christoph Lameter

Extract the code that memory checkers and other verification tools use from the hotpaths. Makes it easier to add new ones and reduces the disturbances of the hotpaths.
Signed-off-by Christoph Lameter <[email protected]>

Acked-by David Rientjes <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

author

2010-08-19 20:34 UTC #16 lore raw

PATCH 6/6 slub: Move gfpflag masking out of the hotpath

In reply to Christoph Lameter

Move the gfpflags masking into the hooks for checkers and into the slowpaths. gfpflag masking requires access to a global variable and thus adds an additional cacheline reference to the hotpaths.

If no hooks are active then the gfpflag masking will result in code that the compiler can toss out.

Acked-by David Rientjes <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 mm/slub.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

mm/slub.c +3 -2

Index: linux-2.6/mm/slub.c
===================================================================
@@ -797,6 +797,7 @@ static void trace(struct kmem_cache *s, 
  */
 static inline int slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags)
 {
+	flags &= gfp_allowed_mask;
 	lockdep_trace_alloc(flags);
 	might_sleep_if(flags & __GFP_WAIT);

@@ -805,6 +806,7 @@ static inline int slab_pre_alloc_hook(st
 
 static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags, void *object)
 {
+	flags &= gfp_allowed_mask;
 	kmemcheck_slab_alloc(s, flags, object, s->objsize);
 	kmemleak_alloc_recursive(object, s->objsize, 1, s->flags, flags);
 }

@@ -1678,6 +1680,7 @@ new_slab:
 		goto load_freelist;
 	}
 
+	gfpflags &= gfp_allowed_mask;
 	if (gfpflags & __GFP_WAIT)
 		local_irq_enable();

@@ -1726,8 +1729,6 @@ static __always_inline void *slab_alloc(
 	struct kmem_cache_cpu *c;
 	unsigned long flags;
 
-	gfpflags &= gfp_allowed_mask;
-
 	if (slab_pre_alloc_hook(s, gfpflags))
 		return NULL;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>