Re: [PATCH] memcg: fix memcg_cache_name() to use cgroup_name()
From: Li Zefan <hidden>
Date: 2013-03-24 07:33:40
Also in:
cgroups, lkml
quoted
Thanks for identifying and fixing this. Li is right. The cache name will live long, but this is because the slab/slub caches will strdup it internally. So the actual memcg allocation is short lived.OK, I have totally missed that. Sorry about the confusion. Then all the churn around the allocation is pointless, no? What about: ---quoted
From 7ed7f53bb597e8cb40d9ac91ce16142fb60f1e93 Mon Sep 17 00:00:00 2001From: Michal Hocko <redacted> Date: Fri, 22 Mar 2013 10:22:54 +0100 Subject: [PATCH] memcg: fix memcg_cache_name() to use cgroup_name() As cgroup supports rename, it's unsafe to dereference dentry->d_name without proper vfs locks. Fix this by using cgroup_name() rather than dentry directly. Also open code memcg_cache_name because it is called only from kmem_cache_dup which frees the returned name right after kmem_cache_create_memcg makes a copy of it. Such a short-lived allocation doesn't make too much sense. So replace it by a static buffer as kmem_cache_dup is called with memcg_cache_mutex.
I doubt it's a win to add 4K to kernel text size instead of adding a few extra lines of code... but it's up to you.
Signed-off-by: Li Zefan <redacted> Signed-off-by: Michal Hocko <redacted> --- mm/memcontrol.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-)
...
static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,
struct kmem_cache *s)
{
char *name;
struct kmem_cache *new;
+ static char tmp_name[PAGE_SIZE];
- name = memcg_cache_name(memcg, s);
- if (!name)
- return NULL;
+ lockdep_assert_held(&memcg_cache_mutex);
+
+ rcu_read_lock();
+ tmp_name = snprintf(tmp_name, sizeof(tmp_name), "%s(%d:%s)", s->name,
+ memcg_cache_id(memcg), cgroup_name(memcg->css.cgroup));I guess you didn't turn on CONFIG_MEMCG_KMEM? snprintf() returns a int value.
+ rcu_read_unlock(); - new = kmem_cache_create_memcg(memcg, name, s->object_size, s->align, + new = kmem_cache_create_memcg(memcg, tmp_name, s->object_size, s->align, (s->flags & ~SLAB_PANIC), s->ctor, s); if (new) new->allocflags |= __GFP_KMEMCG; - kfree(name); return new; }
-- 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>