Re: [PATCH 4/4] slab: Use for_each_kmem_cache_node function

[Joonsoo Kim <iamjoonsoo.kim@lge.com>]

On Fri, May 30, 2014 at 01:27:57PM -0500, Christoph Lameter wrote:
> Reduce code somewhat by the use of kmem_cache_node.

Hello,

There are some other places that we can replace such as get_slabinfo(),
leaks_show(), etc.. If you want to replace for_each_online_node()
with for_each_kmem_cache_node, please also replace them.

Meanwhile, I think that this change is not good for readability. There
are many for_each_online_node() usage that we can't replace, so I don't
think this abstraction is really helpful clean-up. Possibly, using
for_each_online_node() consistently would be more readable than this
change.

Thanks.

> 
> Signed-off-by: Christoph Lameter <cl@linux.com>
> 
> Index: linux/mm/slab.c
> ===================================================================
> --- linux.orig/mm/slab.c	2014-05-30 13:08:32.986856450 -0500
> +++ linux/mm/slab.c	2014-05-30 13:08:32.986856450 -0500
> @@ -2415,17 +2415,12 @@ static void drain_cpu_caches(struct kmem
>  
>  	on_each_cpu(do_drain, cachep, 1);
>  	check_irq_on();
> -	for_each_online_node(node) {
> -		n = get_node(cachep, node);
> -		if (n && n->alien)
> +	for_each_kmem_cache_node(cachep, node, n)
> +		if (n->alien)
>  			drain_alien_cache(cachep, n->alien);
> -	}
>  
> -	for_each_online_node(node) {
> -		n = get_node(cachep, node);
> -		if (n)
> -			drain_array(cachep, n, n->shared, 1, node);
> -	}
> +	for_each_kmem_cache_node(cachep, node, n)
> +		drain_array(cachep, n, n->shared, 1, node);
>  }
>  
>  /*
> @@ -2478,11 +2473,7 @@ static int __cache_shrink(struct kmem_ca
>  	drain_cpu_caches(cachep);
>  
>  	check_irq_on();
> -	for_each_online_node(i) {
> -		n = get_node(cachep, i);
> -		if (!n)
> -			continue;
> -
> +	for_each_kmem_cache_node(cachep, i, n) {
>  		drain_freelist(cachep, n, slabs_tofree(cachep, n));
>  
>  		ret += !list_empty(&n->slabs_full) ||
> @@ -2525,13 +2516,10 @@ int __kmem_cache_shutdown(struct kmem_ca
>  	    kfree(cachep->array[i]);
>  
>  	/* NUMA: free the node structures */
> -	for_each_online_node(i) {
> -		n = get_node(cachep, i);
> -		if (n) {
> -			kfree(n->shared);
> -			free_alien_cache(n->alien);
> -			kfree(n);
> -		}
> +	for_each_kmem_cache_node(cachep, i, n) {
> +		kfree(n->shared);
> +		free_alien_cache(n->alien);
> +		kfree(n);
>  	}
>  	return 0;
>  }
> 
> --
> 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>

--
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>