Re: [PATCH -mm 1/2] memcg: get rid of memcg_create_cache_name

[Michal Hocko <mhocko@suse.cz>]

On Wed 07-05-14 12:15:29, Vladimir Davydov wrote:
> Instead of calling back to memcontrol.c from kmem_cache_create_memcg in
> order to just create the name of a per memcg cache, let's allocate it in
> place. We only need to pass the memcg name to kmem_cache_create_memcg
> for that - everything else can be done in slab_common.c.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>

Seems good to me.
I would keep the comment about the static buffer as mentioned below.
Other than that
Acked-by: Michal Hocko <mhocko@suse.cz>

[...]
> -char *memcg_create_cache_name(struct mem_cgroup *memcg,
> -			      struct kmem_cache *root_cache)
> -{
> -	static char *buf;
> -
> -	/*
> -	 * We need a mutex here to protect the shared buffer. Since this is
> -	 * expected to be called only on cache creation, we can employ the
> -	 * slab_mutex for that purpose.
> -	 */
> -	lockdep_assert_held(&slab_mutex);
> -
> -	if (!buf) {
> -		buf = kmalloc(NAME_MAX + 1, GFP_KERNEL);
> -		if (!buf)
> -			return NULL;
> -	}
> -
> -	cgroup_name(memcg->css.cgroup, buf, NAME_MAX + 1);
> -	return kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
> -			 memcg_cache_id(memcg), buf);
> -}
> -
>  int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
>  			     struct kmem_cache *root_cache)
>  {
> @@ -3164,6 +3141,7 @@ void memcg_free_cache_params(struct kmem_cache *s)
>  static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
>  				    struct kmem_cache *root_cache)
>  {
> +	static char *memcg_name_buf;
>  	struct kmem_cache *cachep;
>  	int id;

So we are relying on memcg_slab_mutex now, right? Worth a comment I
suppose.

-- 
Michal Hocko
SUSE Labs

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