Re: [Slub cleanup6 4/5] slub: Move node determination out of hotpath

[David Rientjes <rientjes@google.com>]

On Fri, 15 Apr 2011, Christoph Lameter wrote:

> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c	2011-04-15 12:52:17.000000000 -0500
> +++ linux-2.6/mm/slub.c	2011-04-15 12:54:15.000000000 -0500
> @@ -1828,7 +1828,6 @@ load_freelist:
>  	c->freelist = get_freepointer(s, object);
>  	page->inuse = page->objects;
>  	page->freelist = NULL;
> -	c->node = page_to_nid(page);
>  
>  unlock_out:
>  	slab_unlock(page);
> @@ -1845,8 +1844,10 @@ another_slab:
>  new_slab:
>  	page = get_partial(s, gfpflags, node);
>  	if (page) {
> -		c->page = page;
>  		stat(s, ALLOC_FROM_PARTIAL);
> +load_from_page:
> +		c->node = page_to_nid(page);
> +		c->page = page;
>  		goto load_freelist;
>  	}
>  

I don't like this because it's using a label within a conditional in an 
already very cluttered __slab_alloc().  This function could benefit from 
some serious cleanup instead of adding even more code that resembles 
spaghetti code from BASIC just to avoid two lines of duplicate code.

> @@ -1867,8 +1868,8 @@ new_slab:
>  
>  		slab_lock(page);
>  		__SetPageSlubFrozen(page);
> -		c->page = page;
> -		goto load_freelist;
> +
> +		goto load_from_page;

I'd much prefer to just add a

	c->node = page_to_nid(page);

rather than the new label and goto into a conditional.

>  	}
>  	if (!(gfpflags & __GFP_NOWARN) && printk_ratelimit())
>  		slab_out_of_memory(s, gfpflags, node);

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>