All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Zefan <lizefan@huawei.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>, Glauber Costa <glommer@parallels.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Cgroups <cgroups@vger.kernel.org>,
	linux-mm@kvack.org
Subject: [PATCH 2/9] memcg, kmem: fix reference count handling on the error path
Date: Fri, 17 May 2013 15:03:23 +0800	[thread overview]
Message-ID: <5195D63B.4010202@huawei.com> (raw)
In-Reply-To: <5195D5F8.7000609@huawei.com>

mem_cgroup_css_online calls mem_cgroup_put if memcg_init_kmem
fails. This is not correct because only memcg_propagate_kmem takes an
additional reference while mem_cgroup_sockets_init is allowed to fail as
well (although no current implementation fails) but it doesn't take any
reference. This all suggests that it should be memcg_propagate_kmem that
should clean up after itself so this patch moves mem_cgroup_put over
there.

Unfortunately this is not that easy (as pointed out by Li Zefan) because
memcg_kmem_mark_dead marks the group dead (KMEM_ACCOUNTED_DEAD) if it
is marked active (KMEM_ACCOUNTED_ACTIVE) which is the case even if
memcg_propagate_kmem fails so the additional reference is dropped in
that case in kmem_cgroup_destroy which means that the reference would be
dropped two times.

The easiest way then would be to simply remove mem_cgrroup_put from
mem_cgroup_css_online and rely on kmem_cgroup_destroy doing the right
thing.

Cc: <stable@vger.kernel.org> # 3.8+
Signed-off-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Li Zefan <lizefan@huawei.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/memcontrol.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5918e90..4d0458d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6290,14 +6290,6 @@ mem_cgroup_css_online(struct cgroup *cont)
 
 	error = memcg_init_kmem(memcg, &mem_cgroup_subsys);
 	mutex_unlock(&memcg_create_mutex);
-	if (error) {
-		/*
-		 * We call put now because our (and parent's) refcnts
-		 * are already in place. mem_cgroup_put() will internally
-		 * call __mem_cgroup_free, so return directly
-		 */
-		mem_cgroup_put(memcg);
-	}
 	return error;
 }
 
-- 
1.8.0.2

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

WARNING: multiple messages have this Message-ID (diff)
From: Li Zefan <lizefan@huawei.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>, Glauber Costa <glommer@parallels.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Cgroups <cgroups@vger.kernel.org>, <linux-mm@kvack.org>
Subject: [PATCH 2/9] memcg, kmem: fix reference count handling on the error path
Date: Fri, 17 May 2013 15:03:23 +0800	[thread overview]
Message-ID: <5195D63B.4010202@huawei.com> (raw)
In-Reply-To: <5195D5F8.7000609@huawei.com>

mem_cgroup_css_online calls mem_cgroup_put if memcg_init_kmem
fails. This is not correct because only memcg_propagate_kmem takes an
additional reference while mem_cgroup_sockets_init is allowed to fail as
well (although no current implementation fails) but it doesn't take any
reference. This all suggests that it should be memcg_propagate_kmem that
should clean up after itself so this patch moves mem_cgroup_put over
there.

Unfortunately this is not that easy (as pointed out by Li Zefan) because
memcg_kmem_mark_dead marks the group dead (KMEM_ACCOUNTED_DEAD) if it
is marked active (KMEM_ACCOUNTED_ACTIVE) which is the case even if
memcg_propagate_kmem fails so the additional reference is dropped in
that case in kmem_cgroup_destroy which means that the reference would be
dropped two times.

The easiest way then would be to simply remove mem_cgrroup_put from
mem_cgroup_css_online and rely on kmem_cgroup_destroy doing the right
thing.

Cc: <stable@vger.kernel.org> # 3.8+
Signed-off-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Li Zefan <lizefan@huawei.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/memcontrol.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5918e90..4d0458d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6290,14 +6290,6 @@ mem_cgroup_css_online(struct cgroup *cont)
 
 	error = memcg_init_kmem(memcg, &mem_cgroup_subsys);
 	mutex_unlock(&memcg_create_mutex);
-	if (error) {
-		/*
-		 * We call put now because our (and parent's) refcnts
-		 * are already in place. mem_cgroup_put() will internally
-		 * call __mem_cgroup_free, so return directly
-		 */
-		mem_cgroup_put(memcg);
-	}
 	return error;
 }
 
-- 
1.8.0.2

  parent reply	other threads:[~2013-05-17  7:03 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-17  7:02 [PATCH 0/12][V3] memcg: make memcg's life cycle the same as cgroup Li Zefan
2013-05-17  7:02 ` Li Zefan
2013-05-17  7:03 ` [PATCH 1/9] Revert "memcg: avoid dangling reference count in creation failure." Li Zefan
2013-05-17  7:03   ` Li Zefan
2013-05-17  7:03   ` Li Zefan
2013-05-17  7:03 ` Li Zefan [this message]
2013-05-17  7:03   ` [PATCH 2/9] memcg, kmem: fix reference count handling on the error path Li Zefan
2013-05-17  7:03 ` [PATCH 4/9] memcg: don't use mem_cgroup_get() when creating a kmemcg cache Li Zefan
2013-05-17  7:03   ` Li Zefan
2013-05-17  7:03   ` Li Zefan
2013-05-17  7:04 ` [PATCH 5/9] memcg: use css_get/put when charging/uncharging kmem Li Zefan
2013-05-17  7:04   ` Li Zefan
     [not found]   ` <5195D666.6030408-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-05-17 18:08     ` Tejun Heo
2013-05-17 18:08       ` Tejun Heo
2013-05-17 18:08       ` Tejun Heo
     [not found]       ` <20130517180822.GC12632-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-05-22  8:36         ` Li Zefan
2013-05-22  8:36           ` Li Zefan
2013-05-22  8:36           ` Li Zefan
     [not found]           ` <519C838B.9060609-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-05-24  7:54             ` Michal Hocko
2013-05-24  7:54               ` Michal Hocko
2013-05-24  7:54               ` Michal Hocko
2013-05-30  5:48               ` Tejun Heo
2013-05-30  5:48                 ` Tejun Heo
     [not found]                 ` <20130530054852.GA9305-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-05-30 15:12                   ` Michal Hocko
2013-05-30 15:12                     ` Michal Hocko
2013-05-30 15:12                     ` Michal Hocko
2013-05-17  7:04 ` [PATCH 6/9] memcg: use css_get/put for swap memcg Li Zefan
2013-05-17  7:04   ` Li Zefan
2013-05-17  7:04 ` [PATCH 7/9] memcg: don't need to get a reference to the parent Li Zefan
2013-05-17  7:04   ` Li Zefan
2013-05-17  7:05 ` [PATCH 8/9] memcg: kill memcg refcnt Li Zefan
2013-05-17  7:05   ` Li Zefan
2013-05-17  7:05   ` Li Zefan
2013-05-17  7:05 ` [PATCH 9/9] memcg: don't need to free memcg via RCU or workqueue Li Zefan
2013-05-17  7:05   ` Li Zefan
     [not found] ` <5195D5F8.7000609-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-05-17  7:03   ` [PATCH 3/9] memcg: use css_get() in sock_update_memcg() Li Zefan
2013-05-17  7:03     ` Li Zefan
2013-05-17  7:03     ` Li Zefan
2013-05-17  7:06   ` [PATCH 0/12][V3] memcg: make memcg's life cycle the same as cgroup Li Zefan
2013-05-17  7:06     ` Li Zefan
2013-05-17  7:06     ` Li Zefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5195D63B.4010202@huawei.com \
    --to=lizefan@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=glommer@parallels.com \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.