[PATCH] slab: cache alloc cleanups

[PATCH] slab: cache alloc cleanups

[Pekka J Enberg]

[Andrew, I have been unable to find a NUMA-capable tester for this patch, 
 so can we please put this in to -mm for some exposure?]

From: Pekka Enberg <penberg@cs.helsinki.fi>

This patch cleans up __cache_alloc and __cache_alloc_node functions.  We no
longer need to do NUMA_BUILD tricks and the UMA allocation path is much
simpler. Note: we now do alternate_node_alloc() for kmem_cache_alloc_node as
well.

Cc: Andy Whitcroft <apw@shadowen.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Manfred Spraul <manfred@colorfullife.com>
Cc: Christoph Lameter <christoph@lameter.com>
Cc: Paul Jackson <pj@sgi.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 mm/slab.c |  165 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 84 insertions(+), 81 deletions(-)

Index: 2.6/mm/slab.c
===================================================================
--- 2.6.orig/mm/slab.c
+++ 2.6/mm/slab.c
@@ -3197,35 +3197,6 @@ static inline void *____cache_alloc(stru
 	return objp;
 }
 
-static __always_inline void *__cache_alloc(struct kmem_cache *cachep,
-						gfp_t flags, void *caller)
-{
-	unsigned long save_flags;
-	void *objp = NULL;
-
-	cache_alloc_debugcheck_before(cachep, flags);
-
-	local_irq_save(save_flags);
-
-	if (unlikely(NUMA_BUILD &&
-			current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY)))
-		objp = alternate_node_alloc(cachep, flags);
-
-	if (!objp)
-		objp = ____cache_alloc(cachep, flags);
-	/*
-	 * We may just have run out of memory on the local node.
-	 * ____cache_alloc_node() knows how to locate memory on other nodes
-	 */
- 	if (NUMA_BUILD && !objp)
- 		objp = ____cache_alloc_node(cachep, flags, numa_node_id());
-	local_irq_restore(save_flags);
-	objp = cache_alloc_debugcheck_after(cachep, flags, objp,
-					    caller);
-	prefetchw(objp);
-	return objp;
-}
-
 #ifdef CONFIG_NUMA
 /*
  * Try allocating on another node if PF_SPREAD_SLAB|PF_MEMPOLICY.
@@ -3383,7 +3354,90 @@ must_grow:
 done:
 	return obj;
 }
-#endif
+
+/**
+ * __do_cache_alloc_node - Allocate an object on the specified node
+ * @cachep: The cache to allocate from.
+ * @flags: See kmalloc().
+ * @nodeid: node number of the target node.
+ *
+ * Fallback to other node is possible if __GFP_THISNODE is not set.
+ */
+static __always_inline void *
+__do_cache_alloc_node(struct kmem_cache *cache, gfp_t flags, int nodeid)
+{
+	void *obj;
+
+	if (nodeid == -1 || nodeid == numa_node_id()) {
+		if (unlikely(current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY))) {
+			obj = alternate_node_alloc(cache, flags);
+			if (obj)
+				goto out;
+		}
+
+		/*
+		 * Use the locally cached objects if possible. However,
+		 * ____cache_alloc does not allow fallback to other nodes.
+		 * It may fail while we still have objects on other nodes
+		 * available.
+		 */
+		obj = ____cache_alloc(cache, flags);
+		if (obj)
+			goto out;
+
+		/* Fall back to other nodes. */
+		obj = ____cache_alloc_node(cache, flags, numa_node_id());
+	} else {
+		if (likely(cache->nodelists[nodeid]))
+			obj = ____cache_alloc_node(cache, flags, nodeid);
+		else {
+			/* Node is not bootstrapped yet. */
+			if (!(flags & __GFP_THISNODE))
+				obj = fallback_alloc(cache, flags);
+			else
+				obj = NULL;
+		}
+	}
+  out:
+	return obj;
+}
+
+#else
+
+static __always_inline void *
+__do_cache_alloc_node(struct kmem_cache *cache, gfp_t flags, int nodeid)
+{
+	/*
+	 * For UMA, we always allocate from the local cache.
+	 */
+	return ____cache_alloc(cache, flags);
+}
+#endif /* CONFIG_NUMA */
+
+static __always_inline void *
+__cache_alloc_node(struct kmem_cache *cache, gfp_t flags, int nodeid,
+		   void *caller)
+{
+	unsigned long save_flags;
+	void *obj;
+
+	cache_alloc_debugcheck_before(cache, flags);
+	local_irq_save(save_flags);
+	obj = __do_cache_alloc_node(cache, flags, nodeid);
+	local_irq_restore(save_flags);
+	obj = cache_alloc_debugcheck_after(cache, flags, obj, caller);
+	return obj;
+}
+
+static __always_inline void *
+__cache_alloc(struct kmem_cache *cache, gfp_t flags, void *caller)
+{
+	void *obj;
+
+	obj = __cache_alloc_node(cache, flags, -1, caller);
+	prefetchw(obj);
+	return obj;
+}
 
 /*
  * Caller needs to acquire correct kmem_list's list_lock
@@ -3582,57 +3636,6 @@ out:
 }
 
 #ifdef CONFIG_NUMA
-/**
- * kmem_cache_alloc_node - Allocate an object on the specified node
- * @cachep: The cache to allocate from.
- * @flags: See kmalloc().
- * @nodeid: node number of the target node.
- * @caller: return address of caller, used for debug information
- *
- * Identical to kmem_cache_alloc but it will allocate memory on the given
- * node, which can improve the performance for cpu bound structures.
- *
- * Fallback to other node is possible if __GFP_THISNODE is not set.
- */
-static __always_inline void *
-__cache_alloc_node(struct kmem_cache *cachep, gfp_t flags,
-		int nodeid, void *caller)
-{
-	unsigned long save_flags;
-	void *ptr = NULL;
-
-	cache_alloc_debugcheck_before(cachep, flags);
-	local_irq_save(save_flags);
-
-	if (unlikely(nodeid == -1))
-		nodeid = numa_node_id();
-
-	if (likely(cachep->nodelists[nodeid])) {
-		if (nodeid == numa_node_id()) {
-			/*
-			 * Use the locally cached objects if possible.
-			 * However ____cache_alloc does not allow fallback
-			 * to other nodes. It may fail while we still have
-			 * objects on other nodes available.
-			 */
-			ptr = ____cache_alloc(cachep, flags);
-		}
-		if (!ptr) {
-			/* ___cache_alloc_node can fall back to other nodes */
-			ptr = ____cache_alloc_node(cachep, flags, nodeid);
-		}
-	} else {
-		/* Node not bootstrapped yet */
-		if (!(flags & __GFP_THISNODE))
-			ptr = fallback_alloc(cachep, flags);
-	}
-
-	local_irq_restore(save_flags);
-	ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
-
-	return ptr;
-}
-
 void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 {
 	return __cache_alloc_node(cachep, flags, nodeid,

Re: [PATCH] slab: cache alloc cleanups

[Andy Whitcroft]

Pekka J Enberg wrote:
> [Andrew, I have been unable to find a NUMA-capable tester for this patch, 
>  so can we please put this in to -mm for some exposure?]
> 
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> This patch cleans up __cache_alloc and __cache_alloc_node functions.  We no
> longer need to do NUMA_BUILD tricks and the UMA allocation path is much
> simpler. Note: we now do alternate_node_alloc() for kmem_cache_alloc_node as
> well.

I'll push this through our tests here if that helps.  I need to rerun
the -rc2-mm1 tests by the looks of it ... looks like the test lab had
some time off over Christmas.

-apw

Re: [PATCH] slab: cache alloc cleanups

[Christoph Lameter]

On Tue, 2 Jan 2007, Pekka J Enberg wrote:

> +
> +	if (nodeid == -1 || nodeid == numa_node_id()) {
> +		if (unlikely(current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY))) {
> +			obj = alternate_node_alloc(cache, flags);
> +			if (obj)
> +				goto out;
> +		}

This reintroduces a bug that was fixed a while ago.

kmalloc_node() must never obey memory policies. 
Alternate_node_alloc implements memory policies.

With this patch kmalloc_node(...., numa_node_id()) would get redirected 
again to other nodes if a memory policy is in effect.

Re: [PATCH] slab: cache alloc cleanups

[Pekka Enberg]

On Tue, 2 Jan 2007, Pekka J Enberg wrote:
> > +
> > +     if (nodeid == -1 || nodeid == numa_node_id()) {
> > +             if (unlikely(current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY))) {
> > +                     obj = alternate_node_alloc(cache, flags);
> > +                     if (obj)
> > +                             goto out;
> > +             }

On 1/2/07, Christoph Lameter <clameter@sgi.com> wrote:
> This reintroduces a bug that was fixed a while ago.

Aah, well, we could have a can_mempolicy parameter, but I'm not sure
if it's an improvement to the current version still...

Re: [PATCH] slab: cache alloc cleanups

[Andrew Morton]

On Tue, 2 Jan 2007 15:47:06 +0200 (EET)
Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> I have been unable to find a NUMA-capable tester for this patch, 

Any x86_64 box can be used to test NUMA code via the numa=fake=N boot option.

fake-numa is somewhat sick in mainline and you might find that it doesn't
work right on some machines, or that it fails with high values of N, but
works OK with N=2.  There are fixes to address this problem in -mm.

Re: [PATCH] slab: cache alloc cleanups

[Christoph Hellwig]

On Tue, Jan 02, 2007 at 03:47:06PM +0200, Pekka J Enberg wrote:
> [Andrew, I have been unable to find a NUMA-capable tester for this patch, 
>  so can we please put this in to -mm for some exposure?]
> 
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> This patch cleans up __cache_alloc and __cache_alloc_node functions.  We no
> longer need to do NUMA_BUILD tricks and the UMA allocation path is much
> simpler. Note: we now do alternate_node_alloc() for kmem_cache_alloc_node as
> well.

Seems to work nicely on my 2node cell blade. 

Re: [PATCH] slab: cache alloc cleanups

[Pekka Enberg]

On 1/4/07, Christoph Hellwig <hch@lst.de> wrote:
> Seems to work nicely on my 2node cell blade.

Thanks for testing. Unfortunately as the other Christoph pointed out,
my patch reintroduces a bug that was fixed a while ago. kmalloc_node
should not be using mempolicies...

[PATCH] slab: NUMA kmem_cache diet

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

Some NUMA machines have a big MAX_NUMNODES (possibly 1024), but fewer possible 
nodes. This patch dynamically sizes the 'struct kmem_cache' to allocate only 
needed space.

I moved nodelists[] field at the end of struct kmem_cache, and use the 
following computation in kmem_cache_init()

cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +
                                 nr_node_ids * sizeof(struct kmem_list3 *);


On my two nodes x86_64 machine, kmem_cache.obj_size is now 192 instead of 704
(This is because on x86_64, MAX_NUMNODES is 64)

On bigger NUMA setups, this might reduce the gfporder of "cache_cache"

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


[-- Attachment #2: kmem_cache_diet.patch --]
[-- Type: text/plain, Size: 1772 bytes --]

diff --git a/mm/slab.c b/mm/slab.c
index abf46ae..b187618 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -389,7 +389,6 @@ struct kmem_cache {
 	unsigned int buffer_size;
 	u32 reciprocal_buffer_size;
 /* 3) touched by every alloc & free from the backend */
-	struct kmem_list3 *nodelists[MAX_NUMNODES];
 
 	unsigned int flags;		/* constant flags */
 	unsigned int num;		/* # of objs per slab */
@@ -444,6 +443,17 @@ #if DEBUG
 	int obj_offset;
 	int obj_size;
 #endif
+	/*
+	 * We put nodelists[] at the end of kmem_cache, because we want to size
+	 * this array to nr_node_ids slots instead of MAX_NUMNODES
+	 * (see kmem_cache_init())
+	 * We still use [MAX_NUMNODES] and not [1] or [0] because cache_cache
+	 * is statically defined, so we reserve the max number of nodes.
+	 */
+	struct kmem_list3 *nodelists[MAX_NUMNODES];
+	/*
+	 * Do not add fields after nodelists[]
+	 */
 };
 
 #define CFLGS_OFF_SLAB		(0x80000000UL)
@@ -678,9 +688,6 @@ static struct kmem_cache cache_cache = {
 	.shared = 1,
 	.buffer_size = sizeof(struct kmem_cache),
 	.name = "kmem_cache",
-#if DEBUG
-	.obj_size = sizeof(struct kmem_cache),
-#endif
 };
 
 #define BAD_ALIEN_MAGIC 0x01020304ul
@@ -1437,6 +1444,15 @@ void __init kmem_cache_init(void)
 	cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
 	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
 
+	/*
+	 * struct kmem_cache size depends on nr_node_ids, which
+	 * can be less than MAX_NUMNODES.
+	 */
+	cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +
+				 nr_node_ids * sizeof(struct kmem_list3 *);
+#if DEBUG
+	cache_cache.obj_size = cache_cache.buffer_size;
+#endif
 	cache_cache.buffer_size = ALIGN(cache_cache.buffer_size,
 					cache_line_size());
 	cache_cache.reciprocal_buffer_size =

Re: [PATCH] slab: NUMA kmem_cache diet

[Pekka J Enberg]

(Please inline patches to the mail, makes it easier to review.)

On Thu, 22 Mar 2007, Eric Dumazet wrote:
> Some NUMA machines have a big MAX_NUMNODES (possibly 1024), but fewer possible
> nodes. This patch dynamically sizes the 'struct kmem_cache' to allocate only
> needed space.
> 
> I moved nodelists[] field at the end of struct kmem_cache, and use the
> following computation in kmem_cache_init()

Hmm, what seems bit worrying is:

diff --git a/mm/slab.c b/mm/slab.c
index abf46ae..b187618 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -389,7 +389,6 @@ struct kmem_cache {
        unsigned int buffer_size;
        u32 reciprocal_buffer_size;
 /* 3) touched by every alloc & free from the backend */
-       struct kmem_list3 *nodelists[MAX_NUMNODES];

I think nodelists is placed at the beginning of the struct for a reason. 
But I have no idea if it actually makes any difference...

			Pekka

Re: [PATCH] slab: NUMA kmem_cache diet

[Eric Dumazet]

Pekka J Enberg a écrit :
> (Please inline patches to the mail, makes it easier to review.)
> 
> On Thu, 22 Mar 2007, Eric Dumazet wrote:
>> Some NUMA machines have a big MAX_NUMNODES (possibly 1024), but fewer possible
>> nodes. This patch dynamically sizes the 'struct kmem_cache' to allocate only
>> needed space.
>>
>> I moved nodelists[] field at the end of struct kmem_cache, and use the
>> following computation in kmem_cache_init()
> 
> Hmm, what seems bit worrying is:
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index abf46ae..b187618 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -389,7 +389,6 @@ struct kmem_cache {
>         unsigned int buffer_size;
>         u32 reciprocal_buffer_size;
>  /* 3) touched by every alloc & free from the backend */
> -       struct kmem_list3 *nodelists[MAX_NUMNODES];
> 
> I think nodelists is placed at the beginning of the struct for a reason. 
> But I have no idea if it actually makes any difference...

It might make a difference if STATS is on, because freehit/freemiss might 
share a cache line with nodelists. Apart that, a kmem_cache struct is 
read_mostly : All changes are done outside of it, via array_cache or nodelists[].


Anyway slab STATS is already a SMP/NUMA nightmare because of cache line ping 
pongs. We might place STATS counter in a/some dedicated cache line(s)...

Re: [PATCH] slab: NUMA kmem_cache diet

[Andy Whitcroft]

Eric Dumazet wrote:
> Some NUMA machines have a big MAX_NUMNODES (possibly 1024), but fewer
> possible nodes. This patch dynamically sizes the 'struct kmem_cache' to
> allocate only needed space.
> 
> I moved nodelists[] field at the end of struct kmem_cache, and use the
> following computation in kmem_cache_init()
> 
> cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +
>                                 nr_node_ids * sizeof(struct kmem_list3 *);
> 
> 
> On my two nodes x86_64 machine, kmem_cache.obj_size is now 192 instead
> of 704
> (This is because on x86_64, MAX_NUMNODES is 64)
> 
> On bigger NUMA setups, this might reduce the gfporder of "cache_cache"

That is a dramatic size difference, and I seem to have 128 slabs wow.
I'll try and find some time to test this on some of our numa kit.

> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> 
> 
> ------------------------------------------------------------------------
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index abf46ae..b187618 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -389,7 +389,6 @@ struct kmem_cache {
>  	unsigned int buffer_size;
>  	u32 reciprocal_buffer_size;
>  /* 3) touched by every alloc & free from the backend */
> -	struct kmem_list3 *nodelists[MAX_NUMNODES];
>  
>  	unsigned int flags;		/* constant flags */
>  	unsigned int num;		/* # of objs per slab */
> @@ -444,6 +443,17 @@ #if DEBUG
>  	int obj_offset;
>  	int obj_size;
>  #endif
> +	/*
> +	 * We put nodelists[] at the end of kmem_cache, because we want to size
> +	 * this array to nr_node_ids slots instead of MAX_NUMNODES
> +	 * (see kmem_cache_init())
> +	 * We still use [MAX_NUMNODES] and not [1] or [0] because cache_cache
> +	 * is statically defined, so we reserve the max number of nodes.
> +	 */
> +	struct kmem_list3 *nodelists[MAX_NUMNODES];
> +	/*
> +	 * Do not add fields after nodelists[]
> +	 */
>  };
>  
>  #define CFLGS_OFF_SLAB		(0x80000000UL)
> @@ -678,9 +688,6 @@ static struct kmem_cache cache_cache = {
>  	.shared = 1,
>  	.buffer_size = sizeof(struct kmem_cache),
>  	.name = "kmem_cache",
> -#if DEBUG
> -	.obj_size = sizeof(struct kmem_cache),
> -#endif

Is there any reason to not initialise the .obj_size here?  You are
initialising both .buffer_size and .obj_size in kmem_cache_init anyhow
so I would expect either both or neither to be initialised in your new
scheme.  Doing only one seems very strange.

>  };
>  
>  #define BAD_ALIEN_MAGIC 0x01020304ul
> @@ -1437,6 +1444,15 @@ void __init kmem_cache_init(void)
>  	cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
>  	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
>  
> +	/*
> +	 * struct kmem_cache size depends on nr_node_ids, which
> +	 * can be less than MAX_NUMNODES.
> +	 */
> +	cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) +
> +				 nr_node_ids * sizeof(struct kmem_list3 *);
> +#if DEBUG
> +	cache_cache.obj_size = cache_cache.buffer_size;
> +#endif
>  	cache_cache.buffer_size = ALIGN(cache_cache.buffer_size,
>  					cache_line_size());
>  	cache_cache.reciprocal_buffer_size =

-apw

Re: [PATCH] slab: NUMA kmem_cache diet

[Christoph Lameter]

On Fri, 23 Mar 2007, Andy Whitcroft wrote:

> > +	/*
> > +	 * We put nodelists[] at the end of kmem_cache, because we want to size
> > +	 * this array to nr_node_ids slots instead of MAX_NUMNODES
> > +	 * (see kmem_cache_init())
> > +	 * We still use [MAX_NUMNODES] and not [1] or [0] because cache_cache
> > +	 * is statically defined, so we reserve the max number of nodes.
> > +	 */

Good idea.

[PATCH] slab: cache alloc cleanups

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

Clean up __cache_alloc and __cache_alloc_node functions a bit.  We no 
longer need to do NUMA_BUILD tricks and the UMA allocation path is much
simpler. No functional changes in this patch.

Note: saves few kernel text bytes on x86 NUMA build due to using gotos in
__cache_alloc_node() and moving __GFP_THISNODE check in to fallback_alloc().

Cc: Andy Whitcroft <apw@shadowen.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Manfred Spraul <manfred@colorfullife.com>
Cc: Christoph Lameter <christoph@lameter.com>
Cc: Paul Jackson <pj@sgi.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

diff --git a/mm/slab.c b/mm/slab.c
index 0d4e574..5edb7bf 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3197,35 +3197,6 @@ static inline void *____cache_alloc(stru
 	return objp;
 }
 
-static __always_inline void *__cache_alloc(struct kmem_cache *cachep,
-						gfp_t flags, void *caller)
-{
-	unsigned long save_flags;
-	void *objp = NULL;
-
-	cache_alloc_debugcheck_before(cachep, flags);
-
-	local_irq_save(save_flags);
-
-	if (unlikely(NUMA_BUILD &&
-			current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY)))
-		objp = alternate_node_alloc(cachep, flags);
-
-	if (!objp)
-		objp = ____cache_alloc(cachep, flags);
-	/*
-	 * We may just have run out of memory on the local node.
-	 * ____cache_alloc_node() knows how to locate memory on other nodes
-	 */
- 	if (NUMA_BUILD && !objp)
- 		objp = ____cache_alloc_node(cachep, flags, numa_node_id());
-	local_irq_restore(save_flags);
-	objp = cache_alloc_debugcheck_after(cachep, flags, objp,
-					    caller);
-	prefetchw(objp);
-	return objp;
-}
-
 #ifdef CONFIG_NUMA
 /*
  * Try allocating on another node if PF_SPREAD_SLAB|PF_MEMPOLICY.
@@ -3257,14 +3228,20 @@ static void *alternate_node_alloc(struct
  * allocator to do its reclaim / fallback magic. We then insert the
  * slab into the proper nodelist and then allocate from it.
  */
-void *fallback_alloc(struct kmem_cache *cache, gfp_t flags)
+static void *fallback_alloc(struct kmem_cache *cache, gfp_t flags)
 {
-	struct zonelist *zonelist = &NODE_DATA(slab_node(current->mempolicy))
-					->node_zonelists[gfp_zone(flags)];
+	struct zonelist *zonelist;
+	gfp_t local_flags;
 	struct zone **z;
 	void *obj = NULL;
 	int nid;
-	gfp_t local_flags = (flags & GFP_LEVEL_MASK);
+
+	if (flags & __GFP_THISNODE)
+		return NULL;
+
+	zonelist = &NODE_DATA(slab_node(current->mempolicy))
+			->node_zonelists[gfp_zone(flags)];
+	local_flags = (flags & GFP_LEVEL_MASK);
 
 retry:
 	/*
@@ -3374,16 +3351,110 @@ must_grow:
 	if (x)
 		goto retry;
 
-	if (!(flags & __GFP_THISNODE))
-		/* Unable to grow the cache. Fall back to other nodes. */
-		return fallback_alloc(cachep, flags);
-
-	return NULL;
+	return fallback_alloc(cachep, flags);
 
 done:
 	return obj;
 }
-#endif
+
+/**
+ * kmem_cache_alloc_node - Allocate an object on the specified node
+ * @cachep: The cache to allocate from.
+ * @flags: See kmalloc().
+ * @nodeid: node number of the target node.
+ * @caller: return address of caller, used for debug information
+ *
+ * Identical to kmem_cache_alloc but it will allocate memory on the given
+ * node, which can improve the performance for cpu bound structures.
+ *
+ * Fallback to other node is possible if __GFP_THISNODE is not set.
+ */
+static __always_inline void *
+__cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
+		   void *caller)
+{
+	unsigned long save_flags;
+	void *ptr;
+
+	cache_alloc_debugcheck_before(cachep, flags);
+	local_irq_save(save_flags);
+
+	if (unlikely(nodeid == -1))
+		nodeid = numa_node_id();
+
+	if (unlikely(!cachep->nodelists[nodeid])) {
+		/* Node not bootstrapped yet */
+		ptr = fallback_alloc(cachep, flags);
+		goto out;
+	}
+
+	if (nodeid == numa_node_id()) {
+		/*
+		 * Use the locally cached objects if possible.
+		 * However ____cache_alloc does not allow fallback
+		 * to other nodes. It may fail while we still have
+		 * objects on other nodes available.
+		 */
+		ptr = ____cache_alloc(cachep, flags);
+		if (ptr)
+			goto out;
+	}
+	/* ___cache_alloc_node can fall back to other nodes */
+	ptr = ____cache_alloc_node(cachep, flags, nodeid);
+  out:
+	local_irq_restore(save_flags);
+	ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
+
+	return ptr;
+}
+
+static __always_inline void *
+__do_cache_alloc(struct kmem_cache *cache, gfp_t flags)
+{
+	void *objp;
+
+	if (unlikely(current->flags & (PF_SPREAD_SLAB | PF_MEMPOLICY))) {
+		objp = alternate_node_alloc(cache, flags);
+		if (objp)
+			goto out;
+	}
+	objp = ____cache_alloc(cache, flags);
+
+	/*
+	 * We may just have run out of memory on the local node.
+	 * ____cache_alloc_node() knows how to locate memory on other nodes
+	 */
+ 	if (!objp)
+ 		objp = ____cache_alloc_node(cache, flags, numa_node_id());
+
+  out:
+	return objp;
+}
+#else
+
+static __always_inline void *
+__do_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
+{
+	return ____cache_alloc(cachep, flags);
+}
+
+#endif /* CONFIG_NUMA */
+
+static __always_inline void *
+__cache_alloc(struct kmem_cache *cachep, gfp_t flags, void *caller)
+{
+	unsigned long save_flags;
+	void *objp;
+
+	cache_alloc_debugcheck_before(cachep, flags);
+	local_irq_save(save_flags);
+	objp = __do_cache_alloc(cachep, flags);
+	local_irq_restore(save_flags);
+	objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
+	prefetchw(objp);
+
+	return objp;
+}
 
 /*
  * Caller needs to acquire correct kmem_list's list_lock
@@ -3582,57 +3653,6 @@ out:
 }
 
 #ifdef CONFIG_NUMA
-/**
- * kmem_cache_alloc_node - Allocate an object on the specified node
- * @cachep: The cache to allocate from.
- * @flags: See kmalloc().
- * @nodeid: node number of the target node.
- * @caller: return address of caller, used for debug information
- *
- * Identical to kmem_cache_alloc but it will allocate memory on the given
- * node, which can improve the performance for cpu bound structures.
- *
- * Fallback to other node is possible if __GFP_THISNODE is not set.
- */
-static __always_inline void *
-__cache_alloc_node(struct kmem_cache *cachep, gfp_t flags,
-		int nodeid, void *caller)
-{
-	unsigned long save_flags;
-	void *ptr = NULL;
-
-	cache_alloc_debugcheck_before(cachep, flags);
-	local_irq_save(save_flags);
-
-	if (unlikely(nodeid == -1))
-		nodeid = numa_node_id();
-
-	if (likely(cachep->nodelists[nodeid])) {
-		if (nodeid == numa_node_id()) {
-			/*
-			 * Use the locally cached objects if possible.
-			 * However ____cache_alloc does not allow fallback
-			 * to other nodes. It may fail while we still have
-			 * objects on other nodes available.
-			 */
-			ptr = ____cache_alloc(cachep, flags);
-		}
-		if (!ptr) {
-			/* ___cache_alloc_node can fall back to other nodes */
-			ptr = ____cache_alloc_node(cachep, flags, nodeid);
-		}
-	} else {
-		/* Node not bootstrapped yet */
-		if (!(flags & __GFP_THISNODE))
-			ptr = fallback_alloc(cachep, flags);
-	}
-
-	local_irq_restore(save_flags);
-	ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
-
-	return ptr;
-}
-
 void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 {
 	return __cache_alloc_node(cachep, flags, nodeid,

Re: [PATCH] slab: cache alloc cleanups

[Christoph Lameter]

On Fri, 5 Jan 2007, Pekka J Enberg wrote:

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Clean up __cache_alloc and __cache_alloc_node functions a bit.  We no 
> longer need to do NUMA_BUILD tricks and the UMA allocation path is much
> simpler. No functional changes in this patch.

Looks good.

Re: [PATCH] slab: cache alloc cleanups

[Andrew Morton]

On Fri, 5 Jan 2007 13:46:45 +0200 (EET)
Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> Clean up __cache_alloc and __cache_alloc_node functions a bit.  We no 
> longer need to do NUMA_BUILD tricks and the UMA allocation path is much
> simpler. No functional changes in this patch.
> 
> Note: saves few kernel text bytes on x86 NUMA build due to using gotos in
> __cache_alloc_node() and moving __GFP_THISNODE check in to fallback_alloc().

Does this actually clean things up, or does it randomly move things around
while carefully retaining existing obscurity?  Not sure..

Re: [PATCH] slab: cache alloc cleanups

[Christoph Lameter]

On Fri, 5 Jan 2007, Andrew Morton wrote:

> Does this actually clean things up, or does it randomly move things around
> while carefully retaining existing obscurity?  Not sure..

Looks like a good cleanup to me.