linux-kernel - Re: [PATCH -mm 1/2] memcg: get rid of memcg_create_cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140507095127.GC9489@dhcp22.suse.cz>
Date:	Wed, 7 May 2014 11:51:27 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Vladimir Davydov <vdavydov@...allels.com>
Cc:	akpm@...ux-foundation.org, hannes@...xchg.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH -mm 1/2] memcg: get rid of memcg_create_cache_name

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@...allels.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@...e.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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/