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
Subject: [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling
Date: Fri, 28 Jun 2013 16:45:42 -0700	[thread overview]
Message-ID: <1372463145-4245-7-git-send-email-tj__15869.6844046519$1372463211$gmane$org@kernel.org> (raw)
In-Reply-To: <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Currently, creating and removing cgroup files in the root directory
are handled separately from the actual subsystem binding and unbinding
which happens in rebind_subsystems().  Also, rebind_subsystems() users
aren't handling file creation errors properly.  Let's integrate
top_cgroup file handling into rebind_subsystems() so that it's simpler
to use and everyone handles file creation errors correctly.

* On a successful return, rebind_subsystems() is guaranteed to have
  created all files of the new subsystems and deleted the ones
  belonging to the removed subsystems.  After a failure, no file is
  created or removed.

* cgroup_remount() no longer needs to make explicit populate/clear
  calls as it's all handled by rebind_subsystems(), and it gets proper
  error handling automatically.

* cgroup_mount() has been updated such that the root dentry and cgroup
  are linked before rebind_subsystems().  Also, the init_cred dancing
  and base file handling are moved right above rebind_subsystems()
  call and proper error handling for the base files is added.

* cgroup_kill_sb() calls rebind_subsystems() to unbind all subsystems
  which now implies removing all subsystem files which requires the
  directory's i_mutex.  Grab it.  This means that files on the root
  cgroup are removed earlier - they used to be deleted from generic
  super_block cleanup from vfs.  This doesn't lead to any functional
  difference and it's cleaner to do the clean up explicitly for all
  files.

Combined with the previous changes, this makes all cgroup file
creation errors handled correctly.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3475267..f0f8bb9 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1003,7 +1003,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 {
 	struct cgroup *cgrp = &root->top_cgroup;
 	struct cgroup_subsys *ss;
-	int i;
+	int i, ret;
 
 	BUG_ON(!mutex_is_locked(&cgroup_mutex));
 	BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
@@ -1028,7 +1028,16 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 	if (root->number_of_cgroups > 1)
 		return -EBUSY;
 
-	/* Process each subsystem */
+	ret = cgroup_populate_dir(cgrp, added_mask);
+	if (ret)
+		return ret;
+
+	/*
+	 * Nothing can fail from this point on.  Remove files for the
+	 * removed subsystems and rebind each subsystem.
+	 */
+	cgroup_clear_dir(cgrp, removed_mask);
+
 	for_each_subsys(ss, i) {
 		unsigned long bit = 1UL << i;
 
@@ -1364,22 +1373,9 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
 		goto out_unlock;
 	}
 
-	/*
-	 * Clear out the files of subsystems that should be removed, do
-	 * this before rebind_subsystems, since rebind_subsystems may
-	 * change this hierarchy's subsys_list.
-	 */
-	cgroup_clear_dir(cgrp, removed_mask);
-
 	ret = rebind_subsystems(root, added_mask, removed_mask);
-	if (ret) {
-		/* rebind_subsystems failed, re-populate the removed files */
-		cgroup_populate_dir(cgrp, removed_mask);
+	if (ret)
 		goto out_unlock;
-	}
-
-	/* re-populate subsystem files */
-	cgroup_populate_dir(cgrp, added_mask);
 
 	if (opts.release_agent)
 		strcpy(root->release_agent_path, opts.release_agent);
@@ -1579,6 +1575,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	struct super_block *sb;
 	struct cgroupfs_root *new_root;
 	struct inode *inode;
+	const struct cred *cred;
 
 	/* First find the desired set of subsystems */
 	mutex_lock(&cgroup_mutex);
@@ -1613,7 +1610,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		struct list_head tmp_links;
 		struct cgroup *root_cgrp = &root->top_cgroup;
 		struct cgroupfs_root *existing_root;
-		const struct cred *cred;
 		int i;
 		struct css_set *cset;
 
@@ -1651,26 +1647,32 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		if (ret)
 			goto unlock_drop;
 
+		sb->s_root->d_fsdata = root_cgrp;
+		root_cgrp->dentry = sb->s_root;
+
+		cred = override_creds(&init_cred);
+
+		ret = cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
+		if (ret)
+			goto rm_base_files;
+
 		ret = rebind_subsystems(root, root->subsys_mask, 0);
-		if (ret == -EBUSY) {
+		if (ret) {
 			free_cgrp_cset_links(&tmp_links);
-			goto unlock_drop;
+			goto rm_base_files;
 		}
+
+		revert_creds(cred);
+
 		/*
 		 * There must be no failure case after here, since rebinding
 		 * takes care of subsystems' refcounts, which are explicitly
 		 * dropped in the failure exit path.
 		 */
 
-		/* EBUSY should be the only error here */
-		BUG_ON(ret);
-
 		list_add(&root->root_list, &cgroup_roots);
 		cgroup_root_count++;
 
-		sb->s_root->d_fsdata = root_cgrp;
-		root->top_cgroup.dentry = sb->s_root;
-
 		/* Link the top cgroup in this hierarchy into all
 		 * the css_set objects */
 		write_lock(&css_set_lock);
@@ -1683,10 +1685,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		BUG_ON(!list_empty(&root_cgrp->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
-		cred = override_creds(&init_cred);
-		cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
-		cgroup_populate_dir(root_cgrp, root->subsys_mask);
-		revert_creds(cred);
 		mutex_unlock(&cgroup_root_mutex);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
@@ -1715,6 +1713,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	kfree(opts.name);
 	return dget(sb->s_root);
 
+ rm_base_files:
+	cgroup_addrm_files(&root->top_cgroup, NULL, cgroup_base_files, false);
+	revert_creds(cred);
  unlock_drop:
 	cgroup_exit_root_id(root);
 	mutex_unlock(&cgroup_root_mutex);
@@ -1741,6 +1742,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
 	BUG_ON(root->number_of_cgroups != 1);
 	BUG_ON(!list_empty(&cgrp->children));
 
+	mutex_lock(&cgrp->dentry->d_inode->i_mutex);
 	mutex_lock(&cgroup_mutex);
 	mutex_lock(&cgroup_root_mutex);
 
@@ -1773,6 +1775,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
 
 	mutex_unlock(&cgroup_root_mutex);
 	mutex_unlock(&cgroup_mutex);
+	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 
 	simple_xattrs_free(&cgrp->xattrs);
 
-- 
1.8.3.1

  parent reply	other threads:[~2013-06-28 23:45 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-28 23:45 [PATCHSET] cgroup: fix and clean up cgroup file creations and removals Tejun Heo
     [not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45   ` [PATCH 1/9] cgroup: minor updates around cgroup_clear_directory() Tejun Heo
     [not found]     ` <1372463145-4245-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  6:42       ` Li Zefan
2013-06-28 23:45   ` Tejun Heo
2013-06-28 23:45   ` [PATCH 2/9] cgroup: fix error path of cgroup_addrm_files() Tejun Heo
     [not found]     ` <1372463145-4245-3-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  6:43       ` Li Zefan
2013-06-28 23:45   ` Tejun Heo
2013-06-28 23:45   ` [PATCH 3/9] cgroup: fix cgroup_add_cftypes() error handling Tejun Heo
2013-06-28 23:45   ` Tejun Heo
     [not found]     ` <1372463145-4245-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  6:43       ` Li Zefan
2013-06-28 23:45   ` [PATCH 4/9] cgroup: separate out cgroup_base_files[] handling out of cgroup_populate/clear_dir() Tejun Heo
     [not found]     ` <1372463145-4245-5-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  6:44       ` Li Zefan
2013-06-28 23:45   ` Tejun Heo
2013-06-28 23:45   ` [PATCH 5/9] cgroup: update error handling in cgroup_populate_dir() Tejun Heo
     [not found]     ` <1372463145-4245-6-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  7:08       ` Li Zefan
     [not found]         ` <51DE59D7.2000203-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-07-11 17:01           ` Tejun Heo
     [not found]             ` <20130711170123.GA10195-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-12  6:36               ` Li Zefan
2013-07-11 17:18       ` [PATCH v2 " Tejun Heo
     [not found]         ` <20130711171825.GD10195-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-12  6:07           ` Li Zefan
2013-07-11 17:18       ` Tejun Heo
2013-06-28 23:45   ` [PATCH " Tejun Heo
2013-06-28 23:45   ` Tejun Heo [this message]
2013-06-28 23:45   ` [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling Tejun Heo
     [not found]     ` <1372463145-4245-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-29  0:10       ` [PATCH v2 " Tejun Heo
2013-07-11  7:09       ` [PATCH " Li Zefan
2013-07-11 17:20       ` [PATCH v3 " Tejun Heo
     [not found]         ` <20130711172001.GF10195-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-12  6:13           ` Li Zefan
2013-07-12 19:37       ` [PATCH v4 " Tejun Heo
2013-07-12 19:37       ` Tejun Heo
2013-06-28 23:45   ` [PATCH 7/9] cgroup: cosmetic follow-up cleanups Tejun Heo
     [not found]     ` <1372463145-4245-8-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-29  0:09       ` Tejun Heo
2013-06-29  0:09       ` Tejun Heo
2013-07-11  7:10       ` Li Zefan
2013-06-28 23:45   ` Tejun Heo
2013-06-28 23:45   ` [PATCH 8/9] cgroup: move number_of_cgroups test out of rebind_subsystems() into cgroup_remount() Tejun Heo
2013-06-28 23:45   ` Tejun Heo
     [not found]     ` <1372463145-4245-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-07-11  7:10       ` Li Zefan
2013-07-11  7:10       ` Li Zefan
2013-06-28 23:45   ` [PATCH 9/9] blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures Tejun Heo
2013-06-28 23:45   ` Tejun Heo
2013-07-11 17:19   ` [PATCH 5.5/9] cgroup: use for_each_subsys() instead of for_each_root_subsys() in cgroup_populate/clear_dir() Tejun Heo
2013-07-11 17:19   ` Tejun Heo
     [not found]     ` <20130711171927.GE10195-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-12  6:13       ` Li Zefan
2013-07-12  7:45   ` [PATCHSET] cgroup: fix and clean up cgroup file creations and removals Tejun Heo
2013-07-12  7:45   ` Tejun Heo

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='1372463145-4245-7-git-send-email-tj__15869.6844046519$1372463211$gmane$org@kernel.org' \
    --to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@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.