diff for duplicates of <51515DEE.70105@parallels.com> diff --git a/a/2.txt b/N1/2.txt index a62b8cf..8b13789 100644 --- a/a/2.txt +++ b/N1/2.txt @@ -1,125 +1 @@ -From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001 -From: Glauber Costa <glommer@parallels.com> -Date: Tue, 26 Mar 2013 12:30:09 +0400 -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. - -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@parallels.com> ---- - 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 diff --git a/a/content_digest b/N1/content_digest index d1cf7ba..7262a4c 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -58,130 +58,5 @@ "\01:2\0" "fn\00001-memcg-fix-memcg_cache_name-to-use-cgroup_name.patch\0" "b\0" - "From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001\n" - "From: Glauber Costa <glommer@parallels.com>\n" - "Date: Tue, 26 Mar 2013 12:30:09 +0400\n" - "Subject: [PATCH] memcg: fix memcg_cache_name() to use cgroup_name()\n" - "\n" - "As cgroup supports rename, it's unsafe to dereference dentry->d_name\n" - "without proper vfs locks. Fix this by using cgroup_name() rather than\n" - "dentry directly.\n" - "\n" - "This patch also takes the opportunity to be smarter about string\n" - "allocation. Most strings related to cache names will be relatively\n" - "small, including with the addition of the memcg suffix. Therefore\n" - "we try our best to use a buffer that may hold all allocations. If\n" - "we can't, we allocate a page. And leave it there for the rest of\n" - "the life of the system. While this is slightly more code-complex,\n" - "it makes us run less into the risk of failed allocations, which\n" - "is always a good thing.\n" - "\n" - "Signed-off-by: Glauber Costa <glommer@parallels.com>\n" - "---\n" - " mm/memcontrol.c | 73 ++++++++++++++++++++++++++++++++++-----------------------\n" - " 1 file changed, 44 insertions(+), 29 deletions(-)\n" - "\n" - "diff --git a/mm/memcontrol.c b/mm/memcontrol.c\n" - "index 0f67257..1821d2f 100644\n" - "--- a/mm/memcontrol.c\n" - "+++ b/mm/memcontrol.c\n" - "@@ -3462,31 +3462,56 @@ void mem_cgroup_destroy_cache(struct kmem_cache *cachep)\n" - " \tschedule_work(&cachep->memcg_params->destroy);\n" - " }\n" - " \n" - "-static char *memcg_cache_name(struct mem_cgroup *memcg, struct kmem_cache *s)\n" - "+/*\n" - "+ * This lock protects updaters, not readers. We want readers to be as fast as\n" - "+ * they can, and they will either see NULL or a valid cache value. Our model\n" - "+ * allow them to see NULL, in which case the root memcg will be selected.\n" - "+ *\n" - "+ * We need this lock because multiple allocations to the same cache from a non\n" - "+ * will span more than one worker. Only one of them can create the cache.\n" - "+ */\n" - "+static DEFINE_MUTEX(memcg_cache_mutex);\n" - "+/*\n" - "+ * Called with memcg_cache_mutex held\n" - "+ */\n" - "+static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,\n" - "+\t\t\t\t\t struct kmem_cache *s)\n" - " {\n" - "-\tchar *name;\n" - "-\tstruct dentry *dentry;\n" - "+\tconst char *cgname; /* actual cache name */\n" - "+\tchar *name = NULL; /* actual cache name */\n" - "+\tchar buf[256]; /* stack buffer for small allocations */\n" - "+\tint buf_len;\n" - "+\tstatic char *buf_name; /* pointer to a page, if we ever need */\n" - "+\tstruct kmem_cache *new;\n" - "+\n" - "+\tlockdep_assert_held(&memcg_cache_mutex);\n" - " \n" - " \trcu_read_lock();\n" - "-\tdentry = rcu_dereference(memcg->css.cgroup->dentry);\n" - "+\tcgname = cgroup_name(memcg->css.cgroup);\n" - " \trcu_read_unlock();\n" - " \n" - "-\tBUG_ON(dentry == NULL);\n" - "-\n" - "-\tname = kasprintf(GFP_KERNEL, \"%s(%d:%s)\", s->name,\n" - "-\t\t\t memcg_cache_id(memcg), dentry->d_name.name);\n" - "-\n" - "-\treturn name;\n" - "-}\n" - "-\n" - "-static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,\n" - "-\t\t\t\t\t struct kmem_cache *s)\n" - "-{\n" - "-\tchar *name;\n" - "-\tstruct kmem_cache *new;\n" - "+\tif (strlen(name) < sizeof(buf)) {\n" - "+\t\tname = buf;\n" - "+\t\tbuf_len = 256;\n" - "+\t} else if (buf_name != NULL) {\n" - "+\t\tname = buf_name;\n" - "+\t\tbuf_len = PAGE_SIZE;\n" - "+\t} else {\n" - "+\t\t/*\n" - "+\t\t * We will now reuse this page, so no need to free that. Will\n" - "+\t\t * only go away at root memcg destruction, although we could\n" - "+\t\t * free it when all non-root memcg goes away if we really\n" - "+\t\t * wanted the trouble.\n" - "+\t\t */\n" - "+\t\tbuf_name = kmalloc(PAGE_SIZE, GFP_KERNEL);\n" - "+\t\tif (!buf_name)\n" - "+\t\t\treturn NULL;\n" - "+\t\tname = buf_name;\n" - "+\t\tbuf_len = PAGE_SIZE;\n" - "+\t}\n" - " \n" - "-\tname = memcg_cache_name(memcg, s);\n" - "-\tif (!name)\n" - "+\tif (snprintf(name, buf_len, \"%s(%d:%s)\", s->name,\n" - "+\t\t\t\tmemcg_cache_id(memcg), cgname))\n" - " \t\treturn NULL;\n" - " \n" - " \tnew = kmem_cache_create_memcg(memcg, name, s->object_size, s->align,\n" - "@@ -3495,19 +3520,9 @@ static struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg,\n" - " \tif (new)\n" - " \t\tnew->allocflags |= __GFP_KMEMCG;\n" - " \n" - "-\tkfree(name);\n" - " \treturn new;\n" - " }\n" - " \n" - "-/*\n" - "- * This lock protects updaters, not readers. We want readers to be as fast as\n" - "- * they can, and they will either see NULL or a valid cache value. Our model\n" - "- * allow them to see NULL, in which case the root memcg will be selected.\n" - "- *\n" - "- * We need this lock because multiple allocations to the same cache from a non\n" - "- * will span more than one worker. Only one of them can create the cache.\n" - "- */\n" - "-static DEFINE_MUTEX(memcg_cache_mutex);\n" - " static struct kmem_cache *memcg_create_kmem_cache(struct mem_cgroup *memcg,\n" - " \t\t\t\t\t\t struct kmem_cache *cachep)\n" - " {\n" - "-- \n" - 1.8.1.4 -3e5ab7e86afba82baf9bd6a74c0ad12369a4f35a130246321b3c93bc1816a580 +7102cf210a8ce11ef17ee091d2e4c0e65df3fee6c4a34084222d3ae11ccb3d09
diff --git a/a/2.txt b/N2/2.txt index a62b8cf..acaf79c 100644 --- a/a/2.txt +++ b/N2/2.txt @@ -1,4 +1,4 @@ -From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001 +>From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001 From: Glauber Costa <glommer@parallels.com> Date: Tue, 26 Mar 2013 12:30:09 +0400 Subject: [PATCH] memcg: fix memcg_cache_name() to use cgroup_name() diff --git a/a/content_digest b/N2/content_digest index d1cf7ba..e721c3d 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -16,7 +16,7 @@ Tejun Heo <tj@kernel.org> LKML <linux-kernel@vger.kernel.org> Cgroups <cgroups@vger.kernel.org> - linux-mm@kvack.org + <linux-mm@kvack.org> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> " Johannes Weiner <hannes@cmpxchg.org>\0" "\01:1\0" @@ -58,7 +58,7 @@ "\01:2\0" "fn\00001-memcg-fix-memcg_cache_name-to-use-cgroup_name.patch\0" "b\0" - "From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001\n" + ">From 9bbcc5e0e3da5200abdd3499806f356868481925 Mon Sep 17 00:00:00 2001\n" "From: Glauber Costa <glommer@parallels.com>\n" "Date: Tue, 26 Mar 2013 12:30:09 +0400\n" "Subject: [PATCH] memcg: fix memcg_cache_name() to use cgroup_name()\n" @@ -184,4 +184,4 @@ "-- \n" 1.8.1.4 -3e5ab7e86afba82baf9bd6a74c0ad12369a4f35a130246321b3c93bc1816a580 +04089253fc966161a89b6676ca2b7f4a1bf18843fed5e54f8c3e863e95075b89
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.