diff mbox

[RFC,3/3] slub: fallback to get_numa_mem() node if we want to allocate on memoryless node

Message ID 1391674026-20092-3-git-send-email-iamjoonsoo.kim@lge.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Joonsoo Kim Feb. 6, 2014, 8:07 a.m. UTC
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Comments

Christoph Lameter (Ampere) Feb. 6, 2014, 5:30 p.m. UTC | #1
On Thu, 6 Feb 2014, Joonsoo Kim wrote:

> diff --git a/mm/slub.c b/mm/slub.c
> index cc1f995..c851f82 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1700,6 +1700,14 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>  	void *object;
>  	int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
>
> +	if (node == NUMA_NO_NODE)
> +		searchnode = numa_mem_id();
> +	else {
> +		searchnode = node;
> +		if (!node_present_pages(node))

This check wouild need to be something that checks for other contigencies
in the page allocator as well. A simple solution would be to actually run
a GFP_THIS_NODE alloc to see if you can grab a page from the proper node.
If that fails then fallback. See how fallback_alloc() does it in slab.

> +			searchnode = get_numa_mem(node);
> +	}

> @@ -2277,11 +2285,18 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>  redo:
>
>  	if (unlikely(!node_match(page, node))) {
> -		stat(s, ALLOC_NODE_MISMATCH);
> -		deactivate_slab(s, page, c->freelist);
> -		c->page = NULL;
> -		c->freelist = NULL;
> -		goto new_slab;
> +		int searchnode = node;
> +
> +		if (node != NUMA_NO_NODE && !node_present_pages(node))

Same issue here. I would suggest not deactivating the slab and first check
if the node has no pages. If so then just take an object from the current
cpu slab. If that is not available do an allcoation from the indicated
node and take whatever the page allocator gave you.
Joonsoo Kim Feb. 7, 2014, 5:41 a.m. UTC | #2
On Thu, Feb 06, 2014 at 11:30:20AM -0600, Christoph Lameter wrote:
> On Thu, 6 Feb 2014, Joonsoo Kim wrote:
> 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index cc1f995..c851f82 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1700,6 +1700,14 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
> >  	void *object;
> >  	int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
> >
> > +	if (node == NUMA_NO_NODE)
> > +		searchnode = numa_mem_id();
> > +	else {
> > +		searchnode = node;
> > +		if (!node_present_pages(node))
> 
> This check wouild need to be something that checks for other contigencies
> in the page allocator as well. A simple solution would be to actually run
> a GFP_THIS_NODE alloc to see if you can grab a page from the proper node.
> If that fails then fallback. See how fallback_alloc() does it in slab.
> 

Hello, Christoph.

This !node_present_pages() ensure that allocation on this node cannot succeed.
So we can directly use numa_mem_id() here.

> > +			searchnode = get_numa_mem(node);
> > +	}
> 
> > @@ -2277,11 +2285,18 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> >  redo:
> >
> >  	if (unlikely(!node_match(page, node))) {
> > -		stat(s, ALLOC_NODE_MISMATCH);
> > -		deactivate_slab(s, page, c->freelist);
> > -		c->page = NULL;
> > -		c->freelist = NULL;
> > -		goto new_slab;
> > +		int searchnode = node;
> > +
> > +		if (node != NUMA_NO_NODE && !node_present_pages(node))
> 
> Same issue here. I would suggest not deactivating the slab and first check
> if the node has no pages. If so then just take an object from the current
> cpu slab. If that is not available do an allcoation from the indicated
> node and take whatever the page allocator gave you.

Here I do is not to deactivate the slab. I first check if the node has no pages.
And then, not taking an object from the current cpu slab. Instead, checking
current cpu slab comes from proper node getting from introduced get_numa_mem().
I think that this approach is better than just taking an object whatever node
requested.

Thanks.
Christoph Lameter (Ampere) Feb. 7, 2014, 5:49 p.m. UTC | #3
On Fri, 7 Feb 2014, Joonsoo Kim wrote:

> > This check wouild need to be something that checks for other contigencies
> > in the page allocator as well. A simple solution would be to actually run
> > a GFP_THIS_NODE alloc to see if you can grab a page from the proper node.
> > If that fails then fallback. See how fallback_alloc() does it in slab.
> >
>
> Hello, Christoph.
>
> This !node_present_pages() ensure that allocation on this node cannot succeed.
> So we can directly use numa_mem_id() here.

Yes of course we can use numa_mem_id().

But the check is only for not having any memory at all on a node. There
are other reason for allocations to fail on a certain node. The node could
have memory that cannot be reclaimed, all dirty, beyond certain
thresholds, not in the current set of allowed nodes etc etc.
Joonsoo Kim Feb. 10, 2014, 1:22 a.m. UTC | #4
On Fri, Feb 07, 2014 at 11:49:57AM -0600, Christoph Lameter wrote:
> On Fri, 7 Feb 2014, Joonsoo Kim wrote:
> 
> > > This check wouild need to be something that checks for other contigencies
> > > in the page allocator as well. A simple solution would be to actually run
> > > a GFP_THIS_NODE alloc to see if you can grab a page from the proper node.
> > > If that fails then fallback. See how fallback_alloc() does it in slab.
> > >
> >
> > Hello, Christoph.
> >
> > This !node_present_pages() ensure that allocation on this node cannot succeed.
> > So we can directly use numa_mem_id() here.
> 
> Yes of course we can use numa_mem_id().
> 
> But the check is only for not having any memory at all on a node. There
> are other reason for allocations to fail on a certain node. The node could
> have memory that cannot be reclaimed, all dirty, beyond certain
> thresholds, not in the current set of allowed nodes etc etc.

Yes. There are many other cases, but I prefer that we think them separately.
Maybe they needs another approach. For now, to solve memoryless node problem,
my solution is enough and safe.

Thanks.
diff mbox

Patch

diff --git a/mm/slub.c b/mm/slub.c
index cc1f995..c851f82 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1700,6 +1700,14 @@  static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
 	void *object;
 	int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
 
+	if (node == NUMA_NO_NODE)
+		searchnode = numa_mem_id();
+	else {
+		searchnode = node;
+		if (!node_present_pages(node))
+			searchnode = get_numa_mem(node);
+	}
+
 	object = get_partial_node(s, get_node(s, searchnode), c, flags);
 	if (object || node != NUMA_NO_NODE)
 		return object;
@@ -2277,11 +2285,18 @@  static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
 redo:
 
 	if (unlikely(!node_match(page, node))) {
-		stat(s, ALLOC_NODE_MISMATCH);
-		deactivate_slab(s, page, c->freelist);
-		c->page = NULL;
-		c->freelist = NULL;
-		goto new_slab;
+		int searchnode = node;
+
+		if (node != NUMA_NO_NODE && !node_present_pages(node))
+			searchnode = get_numa_mem(node);
+
+		if (!node_match(page, searchnode)) {
+			stat(s, ALLOC_NODE_MISMATCH);
+			deactivate_slab(s, page, c->freelist);
+			c->page = NULL;
+			c->freelist = NULL;
+			goto new_slab;
+		}
 	}
 
 	/*