[patch 1/4] slub: replace SLAB_NODE_UNSPECIFIED with NUMA_NO_NODE

[patch 1/4] slub: replace SLAB_NODE_UNSPECIFIED with NUMA_NO_NODE

[David Rientjes]

NUMA_NO_NODE is used in generic kernel code to define the constant -1,
which means that no specific node is actually required.

Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/slab.h |    2 --
 mm/slub.c            |   10 +++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -92,8 +92,6 @@
 #define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
 				(unsigned long)ZERO_SIZE_PTR)
 
-#define SLAB_NODE_UNSPECIFIED (-1L)
-
 /*
  * struct kmem_cache related prototypes
  */
diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1092,7 +1092,7 @@ static inline struct page *alloc_slab_page(gfp_t flags, int node,
 
 	flags |= __GFP_NOTRACK;
 
-	if (node == SLAB_NODE_UNSPECIFIED)
+	if (node == NUMA_NO_NODE)
 		return alloc_pages(flags, order);
 	else
 		return alloc_pages_node(node, flags, order);
@@ -1743,7 +1743,7 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
 
 void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
 {
-	void *ret = slab_alloc(s, gfpflags, SLAB_NODE_UNSPECIFIED, _RET_IP_);
+	void *ret = slab_alloc(s, gfpflags, NUMA_NO_NODE, _RET_IP_);
 
 	trace_kmem_cache_alloc(_RET_IP_, ret, s->objsize, s->size, gfpflags);
 
@@ -1754,7 +1754,7 @@ EXPORT_SYMBOL(kmem_cache_alloc);
 #ifdef CONFIG_TRACING
 void *kmem_cache_alloc_notrace(struct kmem_cache *s, gfp_t gfpflags)
 {
-	return slab_alloc(s, gfpflags, SLAB_NODE_UNSPECIFIED, _RET_IP_);
+	return slab_alloc(s, gfpflags, NUMA_NO_NODE, _RET_IP_);
 }
 EXPORT_SYMBOL(kmem_cache_alloc_notrace);
 #endif
@@ -2758,7 +2758,7 @@ void *__kmalloc(size_t size, gfp_t flags)
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, flags, SLAB_NODE_UNSPECIFIED, _RET_IP_);
+	ret = slab_alloc(s, flags, NUMA_NO_NODE, _RET_IP_);
 
 	trace_kmalloc(_RET_IP_, ret, size, s->size, flags);
 
@@ -3342,7 +3342,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
 	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
-	ret = slab_alloc(s, gfpflags, SLAB_NODE_UNSPECIFIED, caller);
+	ret = slab_alloc(s, gfpflags, NUMA_NO_NODE, caller);
 
 	/* Honor the call site pointer we recieved. */
 	trace_kmalloc(caller, ret, size, s->size, gfpflags);

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

[patch 2/4] slub: rename debug_on to cache_debug_on

[David Rientjes]

debug_on() is too generic of a name for a slub function, so rename it to
the more appropriate cache_debug_on().

Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/slub.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -110,7 +110,7 @@
 #define SLAB_DEBUG_FLAGS (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
 		SLAB_TRACE | SLAB_DEBUG_FREE)
 
-static inline int debug_on(struct kmem_cache *s)
+static inline int cache_debug_on(struct kmem_cache *s)
 {
 #ifdef CONFIG_SLUB_DEBUG
 	return unlikely(s->flags & SLAB_DEBUG_FLAGS);
@@ -1202,7 +1202,7 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
 	int order = compound_order(page);
 	int pages = 1 << order;
 
-	if (debug_on(s)) {
+	if (cache_debug_on(s)) {
 		void *p;
 
 		slab_pad_check(s, page);
@@ -1433,7 +1433,7 @@ static void unfreeze_slab(struct kmem_cache *s, struct page *page, int tail)
 			stat(s, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
 		} else {
 			stat(s, DEACTIVATE_FULL);
-			if (debug_on(s) && (s->flags & SLAB_STORE_USER))
+			if (cache_debug_on(s) && (s->flags & SLAB_STORE_USER))
 				add_full(n, page);
 		}
 		slab_unlock(page);
@@ -1640,7 +1640,7 @@ load_freelist:
 	object = c->page->freelist;
 	if (unlikely(!object))
 		goto another_slab;
-	if (debug_on(s))
+	if (cache_debug_on(s))
 		goto debug;
 
 	c->freelist = get_freepointer(s, object);
@@ -1799,7 +1799,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 	stat(s, FREE_SLOWPATH);
 	slab_lock(page);
 
-	if (debug_on(s))
+	if (cache_debug_on(s))
 		goto debug;
 
 checks_ok:

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

Re: [patch 2/4] slub: rename debug_on to cache_debug_on

[Christoph Lameter]

On Tue, 8 Jun 2010, David Rientjes wrote:

> debug_on() is too generic of a name for a slub function, so rename it to
> the more appropriate cache_debug_on().

Urgh. Sounds Slabby. Cache is too generic. kmem_cache_debug_on()?

I thought the generic is ok here since its only use is within slub.c

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

[patch 3/4] slub: use is_kmalloc_cache in dma_kmalloc_cache

[David Rientjes]

dma_kmalloc_cache() can use the new is_kmalloc_cache() helper function.

Also removes an unnecessary assignment to local variable `s'.

Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/slub.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2649,13 +2649,12 @@ static noinline struct kmem_cache *dma_kmalloc_cache(int index, gfp_t flags)
 	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;
+	BUG_ON(!is_kmalloc_cache(s));
 
 	/*
 	 * Must defer sysfs creation to a workqueue because we don't know

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

Re: [patch 3/4] slub: use is_kmalloc_cache in dma_kmalloc_cache

[Christoph Lameter]

On Tue, 8 Jun 2010, David Rientjes wrote:

> diff --git a/mm/slub.c b/mm/slub.c
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2649,13 +2649,12 @@ static noinline struct kmem_cache *dma_kmalloc_cache(int index, gfp_t flags)
>  	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;
> +	BUG_ON(!is_kmalloc_cache(s));

The point here is to check if the index I is still within the bonds of
kmalloc_cache. Use of is_kmalloc_cache() will confuse the reader.

The assignment to s can be removed independently but my recent versions of
cleanup patches remove the dynmamic allocation of dma slab caches.

Sadly there is the next conference this week in Berlin. So nothing before
next week I think.


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

Re: [patch 3/4] slub: use is_kmalloc_cache in dma_kmalloc_cache

[David Rientjes]

On Wed, 9 Jun 2010, Christoph Lameter wrote:

> > diff --git a/mm/slub.c b/mm/slub.c
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -2649,13 +2649,12 @@ static noinline struct kmem_cache *dma_kmalloc_cache(int index, gfp_t flags)
> >  	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;
> > +	BUG_ON(!is_kmalloc_cache(s));
> 
> The point here is to check if the index I is still within the bonds of
> kmalloc_cache. Use of is_kmalloc_cache() will confuse the reader.
> 

Why does that confuse the reader?  It ensures that s is actually still a 
kmalloc_cache, meaning that i is within the bounds of the kmalloc_caches 
array.  Seems pretty straightforward to me.

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

[patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[David Rientjes]

create_kmalloc_cache() is always passed a gfp_t of GFP_NOWAIT, so it may
be hardwired into the function itself instead of passed.

Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/slub.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2572,7 +2572,7 @@ static int __init setup_slub_nomerge(char *str)
 __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)
+				const char *name, int size)
 {
 	unsigned int flags = 0;
 
@@ -2582,14 +2582,11 @@ static struct kmem_cache *create_kmalloc_cache(struct kmem_cache *s,
 		return s;
 	}
 
-	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, GFP_NOWAIT, name, size, ARCH_KMALLOC_MINALIGN,
 								flags, NULL))
 		goto panic;
 
@@ -3077,7 +3074,7 @@ void __init kmem_cache_init(void)
 	 */
 	i = kmalloc_index(sizeof(struct kmem_cache_node));
 	create_kmalloc_cache(&kmalloc_caches[i], "bootstrap",
-		sizeof(struct kmem_cache_node), GFP_NOWAIT);
+		sizeof(struct kmem_cache_node));
 	kmalloc_caches[i].refcount = -1;
 	caches++;
 
@@ -3089,19 +3086,16 @@ 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);
+		create_kmalloc_cache(&kmalloc_caches[1], "kmalloc-96", 96);
 		caches++;
 	}
 	if (KMALLOC_MIN_SIZE <= 64) {
-		create_kmalloc_cache(&kmalloc_caches[2],
-				"kmalloc-192", 192, GFP_NOWAIT);
+		create_kmalloc_cache(&kmalloc_caches[2], "kmalloc-192", 192);
 		caches++;
 	}
 
 	for (i = KMALLOC_SHIFT_LOW; i < SLUB_PAGE_SHIFT; i++) {
-		create_kmalloc_cache(&kmalloc_caches[i],
-			"kmalloc", 1 << i, GFP_NOWAIT);
+		create_kmalloc_cache(&kmalloc_caches[i], "kmalloc", 1 << i);
 		caches++;
 	}
 

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

Re: [patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[Christoph Lameter]

Acked-by: Christoph Lameter <cl@linux-foundation.org>


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

Re: [patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[Pekka Enberg]

Christoph Lameter wrote:
> Acked-by: Christoph Lameter <cl@linux-foundation.org>

Applied, thanks!

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

Re: [patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[Christoph Lameter]

On Wed, 9 Jun 2010, Pekka Enberg wrote:

> Christoph Lameter wrote:
> > Acked-by: Christoph Lameter <cl@linux-foundation.org>
>
> Applied, thanks!
>

Breaks DMA cache creation since one can no longer set the
SLAB_CACHE_DMA on create_kmalloc_cache.

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

Re: [patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[David Rientjes]

On Tue, 15 Jun 2010, Christoph Lameter wrote:

> > > Acked-by: Christoph Lameter <cl@linux-foundation.org>
> >
> > Applied, thanks!
> >
> 
> Breaks DMA cache creation since one can no longer set the
> SLAB_CACHE_DMA on create_kmalloc_cache.
> 

How?  There are no callers to create_kmalloc_cache() that pass anything 
except GFP_NOWAIT.

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

Re: [patch 4/4] slub: remove gfp_flags argument from create_kmalloc_cache

[Christoph Lameter]

On Tue, 15 Jun 2010, David Rientjes wrote:

> > Breaks DMA cache creation since one can no longer set the
> > SLAB_CACHE_DMA on create_kmalloc_cache.
> >
>
> How?  There are no callers to create_kmalloc_cache() that pass anything
> except GFP_NOWAIT.

Ok it breaks it with my changes that use create_kmalloc_dma for dma caches
again.

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

Re: [patch 1/4] slub: replace SLAB_NODE_UNSPECIFIED with NUMA_NO_NODE

[Christoph Lameter]

Sigh. I wish we could have avoided this. Neither name is satisfactory
here. The meaning of -1 is unspecified. A node specification may be
implicit here depending on the memory allocation policy. I was not sure
how exactly to resolve the situation.

But this way work with NUMA_NO_NODE. Certainly better.

Reviewed-by: Christoph Lameter <cl@linux-foundation.org>


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

Re: [patch 1/4] slub: replace SLAB_NODE_UNSPECIFIED with NUMA_NO_NODE

[Pekka Enberg]

Christoph Lameter wrote:
> Sigh. I wish we could have avoided this. Neither name is satisfactory
> here. The meaning of -1 is unspecified. A node specification may be
> implicit here depending on the memory allocation policy. I was not sure
> how exactly to resolve the situation.
> 
> But this way work with NUMA_NO_NODE. Certainly better.
> 
> Reviewed-by: Christoph Lameter <cl@linux-foundation.org>

Applied, thanks!

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