[PATCH] memcg: fix memcg_cache_name() to use cgroup_name()

[Glauber Costa]

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.

This patch also takes the opportunity to be smarter about string
allocation. Most strings related to cache names will be relatively
small, including with the addition of the memcg suffix. Therefore
we try our best to use a buffer that may hold all allocations. If
we can't, we allocate a page. And leave it there for the rest of
the life of the system. While this is slightly more code-complex,
it makes us run less into the risk of failed allocations, which
is always a good thing.

Signed-off-by: Glauber Costa <glommer@xxxxxxxxxxxxx>
---
 mm/memcontrol.c | 73 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 29 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0f67257..1821d2f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3462,31 +3462,56 @@ void mem_cgroup_destroy_cache(struct kmem_cache *cachep)
 	schedule_work(&cachep->memcg_params->destroy);
 }
 
-static char *memcg_cache_name(struct mem_cgroup *memcg, struct kmem_cache *s)
+/*
+ * This lock protects updaters, not readers. We want readers to be as fast as
+ * they can, and they will either see NULL or a valid cache value. Our model
+ * allow them to see NULL, in which case the root memcg will be selected.
+ *
+ * We need this lock because multiple allocations to the same cache from a non
+ * will span more than one worker. Only one of them can create the cache.
+ */
+static DEFINE_MUTEX(memcg_cache_mutex);
+/*
+ * Called with memcg_cache_mutex held
+ */
+static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,
+					 struct kmem_cache *s)
 {
-	char *name;
-	struct dentry *dentry;
+	const char *cgname; /* actual cache name */
+	char *name = NULL; /* actual cache name */
+	char buf[256]; /* stack buffer for small allocations */
+	int buf_len;
+	static char *buf_name; /* pointer to a page, if we ever need */
+	struct kmem_cache *new;
+
+	lockdep_assert_held(&memcg_cache_mutex);
 
 	rcu_read_lock();
-	dentry = rcu_dereference(memcg->css.cgroup->dentry);
+	cgname = cgroup_name(memcg->css.cgroup);
 	rcu_read_unlock();
 
-	BUG_ON(dentry == NULL);
-
-	name = kasprintf(GFP_KERNEL, "%s(%d:%s)", s->name,
-			 memcg_cache_id(memcg), dentry->d_name.name);
-
-	return name;
-}
-
-static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,
-					 struct kmem_cache *s)
-{
-	char *name;
-	struct kmem_cache *new;
+	if (strlen(name) < sizeof(buf)) {
+		name = buf;
+		buf_len = 256;
+	} else if (buf_name != NULL) {
+		name = buf_name;
+		buf_len = PAGE_SIZE;
+	} else {
+		/*
+		 * We will now reuse this page, so no need to free that.  Will
+		 * only go away at root memcg destruction, although we could
+		 * free it when all non-root memcg goes away if we really
+		 * wanted the trouble.
+		 */
+		buf_name = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!buf_name)
+			return NULL;
+		name = buf_name;
+		buf_len = PAGE_SIZE;
+	}
 
-	name = memcg_cache_name(memcg, s);
-	if (!name)
+	if (snprintf(name, buf_len, "%s(%d:%s)", s->name,
+				memcg_cache_id(memcg), cgname))
 		return NULL;
 
 	new = kmem_cache_create_memcg(memcg, name, s->object_size, s->align,
@@ -3495,19 +3520,9 @@ static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,
 	if (new)
 		new->allocflags |= __GFP_KMEMCG;
 
-	kfree(name);
 	return new;
 }
 
-/*
- * This lock protects updaters, not readers. We want readers to be as fast as
- * they can, and they will either see NULL or a valid cache value. Our model
- * allow them to see NULL, in which case the root memcg will be selected.
- *
- * We need this lock because multiple allocations to the same cache from a non
- * will span more than one worker. Only one of them can create the cache.
- */
-static DEFINE_MUTEX(memcg_cache_mutex);
 static struct kmem_cache *memcg_create_kmem_cache(struct mem_cgroup *memcg,
 						  struct kmem_cache *cachep)
 {
-- 
1.8.1.4


--------------000309060002060402080600--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/