All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH 1/9] cgroup: fix css leaks on online_css() failure
Date: Wed, 28 Aug 2013 17:03:41 -0400	[thread overview]
Message-ID: <1377723829-22814-2-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1377723829-22814-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

ae7f164a09 ("cgroup: move cgroup->subsys[] assignment to
online_css()") moved cgroup->subsys[] assignements later in
cgroup_create() but didn't update error handling path accordingly
leaking later css's after an online_css() failure.

This patch moves reference bumping inside online_css() loop, clears
css_ar[] as css's are brought online successfully, and updates
err_destroy path so that either a css is fully online and destroyed by
cgroup_destroy_locked() or the error path frees it.  This creates a
duplicate css free logic in the error path but it will be cleaned up
soon.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 kernel/cgroup.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b5f4989..a4168cf 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4489,14 +4489,6 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
 
-	/* each css holds a ref to the cgroup's dentry and the parent css */
-	for_each_root_subsys(root, ss) {
-		struct cgroup_subsys_state *css = css_ar[ss->subsys_id];
-
-		dget(dentry);
-		css_get(css->parent);
-	}
-
 	/* hold a ref to the parent's dentry */
 	dget(parent->dentry);
 
@@ -4508,6 +4500,13 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		if (err)
 			goto err_destroy;
 
+		/* each css holds a ref to the cgroup's dentry and parent css */
+		dget(dentry);
+		css_get(css->parent);
+
+		/* mark it consumed for error path */
+		css_ar[ss->subsys_id] = NULL;
+
 		if (ss->broken_hierarchy && !ss->warned_broken_hierarchy &&
 		    parent->parent) {
 			pr_warning("cgroup: %s (%d) created nested cgroup for controller \"%s\" which has incomplete hierarchy support. Nested cgroups may change behavior in the future.\n",
@@ -4554,6 +4553,14 @@ err_free_cgrp:
 	return err;
 
 err_destroy:
+	for_each_root_subsys(root, ss) {
+		struct cgroup_subsys_state *css = css_ar[ss->subsys_id];
+
+		if (css) {
+			percpu_ref_cancel_init(&css->refcnt);
+			ss->css_free(css);
+		}
+	}
 	cgroup_destroy_locked(cgrp);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&dentry->d_inode->i_mutex);
-- 
1.8.3.1

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: lizefan@huawei.com
Cc: containers@lists.linux-foundation.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org, Tejun Heo <tj@kernel.org>
Subject: [PATCH 1/9] cgroup: fix css leaks on online_css() failure
Date: Wed, 28 Aug 2013 17:03:41 -0400	[thread overview]
Message-ID: <1377723829-22814-2-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1377723829-22814-1-git-send-email-tj@kernel.org>

ae7f164a09 ("cgroup: move cgroup->subsys[] assignment to
online_css()") moved cgroup->subsys[] assignements later in
cgroup_create() but didn't update error handling path accordingly
leaking later css's after an online_css() failure.

This patch moves reference bumping inside online_css() loop, clears
css_ar[] as css's are brought online successfully, and updates
err_destroy path so that either a css is fully online and destroyed by
cgroup_destroy_locked() or the error path frees it.  This creates a
duplicate css free logic in the error path but it will be cleaned up
soon.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/cgroup.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b5f4989..a4168cf 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4489,14 +4489,6 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
 
-	/* each css holds a ref to the cgroup's dentry and the parent css */
-	for_each_root_subsys(root, ss) {
-		struct cgroup_subsys_state *css = css_ar[ss->subsys_id];
-
-		dget(dentry);
-		css_get(css->parent);
-	}
-
 	/* hold a ref to the parent's dentry */
 	dget(parent->dentry);
 
@@ -4508,6 +4500,13 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		if (err)
 			goto err_destroy;
 
+		/* each css holds a ref to the cgroup's dentry and parent css */
+		dget(dentry);
+		css_get(css->parent);
+
+		/* mark it consumed for error path */
+		css_ar[ss->subsys_id] = NULL;
+
 		if (ss->broken_hierarchy && !ss->warned_broken_hierarchy &&
 		    parent->parent) {
 			pr_warning("cgroup: %s (%d) created nested cgroup for controller \"%s\" which has incomplete hierarchy support. Nested cgroups may change behavior in the future.\n",
@@ -4554,6 +4553,14 @@ err_free_cgrp:
 	return err;
 
 err_destroy:
+	for_each_root_subsys(root, ss) {
+		struct cgroup_subsys_state *css = css_ar[ss->subsys_id];
+
+		if (css) {
+			percpu_ref_cancel_init(&css->refcnt);
+			ss->css_free(css);
+		}
+	}
 	cgroup_destroy_locked(cgrp);
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&dentry->d_inode->i_mutex);
-- 
1.8.3.1


  parent reply	other threads:[~2013-08-28 21:03 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28 21:03 [PATCHSET cgroup/for-3.12] cgroup: factor out css creation into create_css() Tejun Heo
2013-08-28 21:03 ` Tejun Heo
2013-08-28 21:03 ` [PATCH 7/9] cgroup: factor out cgroup_subsys_state " Tejun Heo
     [not found] ` <1377723829-22814-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-08-28 21:03   ` Tejun Heo [this message]
2013-08-28 21:03     ` [PATCH 1/9] cgroup: fix css leaks on online_css() failure Tejun Heo
2013-09-02  9:44     ` Li Zefan
     [not found]       ` <52245DE0.1010701-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-09-03 20:06         ` Tejun Heo
2013-09-03 20:06         ` Tejun Heo
2013-09-03 20:06           ` Tejun Heo
     [not found]     ` <1377723829-22814-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-09-03 20:51       ` [PATCH v2 " Tejun Heo
2013-09-03 20:51         ` Tejun Heo
2013-08-28 21:03   ` [PATCH 2/9] cgroup: css iterations and css_from_dir() are safe under cgroup_mutex Tejun Heo
2013-08-28 21:03     ` Tejun Heo
     [not found]     ` <1377723829-22814-3-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-08-31 13:56       ` [PATCH v2 " Tejun Heo
2013-08-31 13:56         ` Tejun Heo
2013-08-28 21:03   ` [PATCH 3/9] cgroup: make for_each_subsys() useable under cgroup_root_mutex Tejun Heo
2013-08-28 21:03     ` Tejun Heo
     [not found]     ` <1377723829-22814-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-08-31 13:59       ` [PATCH v2 " Tejun Heo
2013-08-31 13:59         ` Tejun Heo
2013-08-31 13:59       ` Tejun Heo
2013-08-28 21:03   ` [PATCH 4/9] cgroup: move css_id commit from cgroup_populate_dir() to online_css() Tejun Heo
2013-08-28 21:03     ` Tejun Heo
2013-08-28 21:03   ` [PATCH 5/9] cgroup: reorder operations in cgroup_create() Tejun Heo
2013-08-28 21:03     ` Tejun Heo
2013-08-28 21:03   ` [PATCH 6/9] cgroup: combine css handling loops " Tejun Heo
2013-08-28 21:03     ` Tejun Heo
2013-08-28 21:03   ` [PATCH 7/9] cgroup: factor out cgroup_subsys_state creation into create_css() Tejun Heo
2013-08-28 21:03   ` [PATCH 8/9] cgroup: implement for_each_css() Tejun Heo
2013-08-28 21:03     ` Tejun Heo
     [not found]     ` <1377723829-22814-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-09-03 20:54       ` [PATCH v2 " Tejun Heo
2013-09-03 20:54         ` Tejun Heo
2013-08-28 21:03   ` [PATCH 9/9] cgroup: remove for_each_root_subsys() Tejun Heo
2013-08-28 21:03     ` Tejun Heo
2013-08-28 21:03   ` Tejun Heo
2013-09-05  3:24   ` [PATCHSET cgroup/for-3.12] cgroup: factor out css creation into create_css() Li Zefan
2013-09-05  3:24     ` 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=1377723829-22814-2-git-send-email-tj@kernel.org \
    --to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.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.