Re: [PATCH V3 1/2] slub: fix kmem cgroup bug in kmem_cache_alloc_bulk

[Vladimir Davydov <vdavydov@virtuozzo.com>]

On Mon, Nov 09, 2015 at 07:17:31PM +0100, Jesper Dangaard Brouer wrote:
...
> @@ -2556,7 +2563,7 @@ redo:
>  	if (unlikely(gfpflags & __GFP_ZERO) && object)
>  		memset(object, 0, s->object_size);
>  
> -	slab_post_alloc_hook(s, gfpflags, object);
> +	slab_post_alloc_hook(s, gfpflags, 1, object);

I think it must be &object

BTW why is object defined as void **? I suspect we can safely drop one
star.

>  
>  	return object;
>  }
...
> @@ -2953,11 +2958,15 @@ bool kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
>  			memset(p[j], 0, s->object_size);
>  	}
>  
> +	/* memcg and kmem_cache debug support */
> +	slab_post_alloc_hook(s, flags, size, p);
> +
>  	return true;
>  
>  error:
>  	__kmem_cache_free_bulk(s, i, p);
>  	local_irq_enable();
> +	memcg_kmem_put_cache(s);

I wouldn't tear memcg_kmem_put_cache from slab_post_alloc_hook. If we
add something else to slab_post_alloc_hook (e.g. we might want to call
tracing functions from there), we'll have to modify this error path
either, which is easy to miss.

What about calling

	slab_post_alloc_hook(s, flags, 0, NULL);

here?

Thanks,
Vladimir

>  	return false;
>  }
>  EXPORT_SYMBOL(kmem_cache_alloc_bulk);
> 

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