[PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[Witold Krecicki]

This patchset adds namespace-like feature to the existing cgroup system.
When used with a container system (eg. lxc) it allows containers to have
its own cgroup hierarchy, enabling use of 'systemd' (using cgroups) inside 
a container. 

The behaviour is not changed at all when isolation_root flag is not set, 
no additional overhead is added.

I'm really not sure if the 'mount' part (patch 5) is done correctly, please 
review carefully.

Witold Krecicki (6):
  cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem
  cgroup: make 'cgroup_is_descendant' function take cgroup as a
    'descendant of' argument
  cgroup: add 'root' parameter to cgroup_path function
  cgroup: disallow task from leaving cgroup isolated root
  cgroup: make cgroup filesystem mounts performed by task inside
    isolation root see its isolation root as top cgroup
  cgroup: documentation of isolation_root cgroup flag

 Documentation/cgroups/cgroups.txt |   20 +++++++-
 block/blk-cgroup.c                |    2 +-
 include/linux/cgroup.h            |   12 +++-
 kernel/cgroup.c                   |  102 +++++++++++++++++++++++++++++++-----
 kernel/cpuset.c                   |    2 +-
 kernel/sched_debug.c              |    2 +-
 mm/memcontrol.c                   |    6 +-
 7 files changed, 120 insertions(+), 26 deletions(-)

-- 
1.7.4.1

[PATCH 1/6] cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem

[Witold Krecicki]

This patch adds cgroup.isolation_root flag to cgroups.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 include/linux/cgroup.h |    4 +++
 kernel/cgroup.c        |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index da7e4bc..ee51f79 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -158,6 +158,10 @@ enum {
 	 * Clone cgroup values when creating a new child cgroup
 	 */
 	CGRP_CLONE_CHILDREN,
+	/*
+	 * CGroup is an isolation root
+	 */
+	CGRP_ISOLATION_ROOT,
 };
 
 /* which pidlist file are we talking about? */
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 1d2b6ce..a4d002c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -251,6 +251,11 @@ static int clone_children(const struct cgroup *cgrp)
 	return test_bit(CGRP_CLONE_CHILDREN, &cgrp->flags);
 }
 
+static int isolation_root(const struct cgroup *cgrp)
+{
+	return test_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
+}
+
 /*
  * for_each_subsys() allows you to iterate on each subsystem attached to
  * an active hierarchy
@@ -394,6 +399,24 @@ static inline void put_css_set_taskexit(struct css_set *cg)
 }
 
 /*
+ * cgroup_get_isolation_root - gets the isolation root for cgroup
+ * @cgrp: cgroup for which we're looking for isolation root
+ *
+ * Returns isolated root cgroup or NULL if there's no isolation root
+ */
+static struct cgroup *cgroup_get_isolation_root(struct cgroup *cgrp)
+{
+	for (;;) {
+		if (!cgrp)
+			return NULL;
+		if (isolation_root(cgrp))
+			return cgrp;
+		cgrp = cgrp->parent;
+	}
+	return NULL;
+}
+
+/*
  * compare_css_sets - helper function for find_existing_css_set().
  * @cg: candidate css_set being tested
  * @old_cg: existing css_set for a task
@@ -3620,6 +3643,28 @@ static int cgroup_clone_children_write(struct cgroup *cgrp,
 	return 0;
 }
 
+
+static u64 cgroup_isolation_root_read(struct cgroup *cgrp,
+				    struct cftype *cft)
+{
+	return isolation_root(cgrp);
+}
+
+static int cgroup_isolation_root_write(struct cgroup *cgrp,
+				     struct cftype *cft,
+				     u64 val)
+{
+	if (cgrp->parent == NULL)
+		return -EBUSY;
+	if (atomic_read(&cgrp->count))
+		return -EBUSY;
+	if (val)
+		set_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
+	else
+		clear_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
+	return 0;
+}
+
 /*
  * for the common functions, 'private' gives the type of file
  */
@@ -3655,6 +3700,11 @@ static struct cftype files[] = {
 		.read_u64 = cgroup_clone_children_read,
 		.write_u64 = cgroup_clone_children_write,
 	},
+	{
+		.name = "cgroup.isolation_root",
+		.read_u64 = cgroup_isolation_root_read,
+		.write_u64 = cgroup_isolation_root_write,
+	},
 };
 
 static struct cftype cft_release_agent = {
-- 
1.7.4.1

[PATCH 2/6] cgroup: make 'cgroup_is_descendant' function take cgroup as a 'descendant of' argument

[Witold Krecicki]

Make currently unused (previously used by cgroup_ns) 'cgroup_is_descendant' function
take cgroup as a 'descendant of' argument instead of task.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 include/linux/cgroup.h |    5 +++--
 kernel/cgroup.c        |   16 +++++++---------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index ee51f79..6965591 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -440,8 +440,9 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
 
 int cgroup_task_count(const struct cgroup *cgrp);
 
-/* Return true if cgrp is a descendant of the task's cgroup */
-int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task);
+/* Return true if cgrp is a descendant of the topcgrp */
+int cgroup_is_descendant(const struct cgroup *cgrp,
+			  const struct cgroup *topcgrp);
 
 /*
  * When the subsys has to access css and may add permanent refcnt to css,
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index a4d002c..0177472 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4684,30 +4684,28 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
 }
 
 /**
- * cgroup_is_descendant - see if @cgrp is a descendant of @task's cgrp
+ * cgroup_is_descendant - see if @cgrp is a descendant of @topcgrp
  * @cgrp: the cgroup in question
- * @task: the task in question
+ * @topcgrp: the top cgroup
  *
- * See if @cgrp is a descendant of @task's cgroup in the appropriate
+ * See if @cgrp is a descendant of @topcgrp in the appropriate
  * hierarchy.
  *
  * If we are sending in dummytop, then presumably we are creating
  * the top cgroup in the subsystem.
  *
- * Called only by the ns (nsproxy) cgroup.
  */
-int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task)
+int cgroup_is_descendant(const struct cgroup *cgrp,
+			  const struct cgroup *topcgrp)
 {
 	int ret;
-	struct cgroup *target;
 
 	if (cgrp == dummytop)
 		return 1;
 
-	target = task_cgroup_from_root(task, cgrp->root);
-	while (cgrp != target && cgrp!= cgrp->top_cgroup)
+	while (cgrp != topcgrp && cgrp != cgrp->top_cgroup)
 		cgrp = cgrp->parent;
-	ret = (cgrp == target);
+	ret = (cgrp == topcgrp);
 	return ret;
 }
 
-- 
1.7.4.1

[PATCH 3/6] cgroup: add 'root' parameter to cgroup_path function

[Witold Krecicki]

This patch adds 'root' parameter to 'cgroup_path' function. When it's
not NULL creating cgroup path will stop when current cgroup is 'root'.
A task contained in cgroup isolation root will see its cgroup in
/proc/$PID/cgroup with isolation root as top.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 block/blk-cgroup.c     |    2 +-
 include/linux/cgroup.h |    3 ++-
 kernel/cgroup.c        |   16 ++++++++++++----
 kernel/cpuset.c        |    2 +-
 kernel/sched_debug.c   |    2 +-
 mm/memcontrol.c        |    6 +++---
 6 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b596e54..2dd96e7 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -491,7 +491,7 @@ void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg,
 	blkg->plid = plid;
 	spin_unlock_irqrestore(&blkcg->lock, flags);
 	/* Need to take css reference ? */
-	cgroup_path(blkcg->css.cgroup, blkg->path, sizeof(blkg->path));
+	cgroup_path(blkcg->css.cgroup, blkg->path, sizeof(blkg->path), NULL);
 	blkg->dev = dev;
 }
 EXPORT_SYMBOL_GPL(blkiocg_add_blkio_group);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 6965591..3b36554 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -436,7 +436,8 @@ int cgroup_add_files(struct cgroup *cgrp,
 
 int cgroup_is_removed(const struct cgroup *cgrp);
 
-int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
+int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen,
+		 const struct cgroup *root);
 
 int cgroup_task_count(const struct cgroup *cgrp);
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 0177472..f9b4bdf 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1716,12 +1716,14 @@ static inline struct cftype *__d_cft(struct dentry *dentry)
  * @cgrp: the cgroup in question
  * @buf: the buffer to write the path into
  * @buflen: the length of the buffer
+ * @root: root cgroup for isolation
  *
  * Called with cgroup_mutex held or else with an RCU-protected cgroup
  * reference.  Writes path of cgroup into buf.  Returns 0 on success,
  * -errno on error.
  */
-int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
+int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen,
+		 const struct cgroup *root)
 {
 	char *start;
 	struct dentry *dentry = rcu_dereference_check(cgrp->dentry,
@@ -1740,6 +1742,10 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 
 	*--start = '\0';
 	for (;;) {
+		if (cgrp == root) {
+			*start = '/';
+			break;
+		}
 		int len = dentry->d_name.len;
 
 		if ((start -= len) < buf)
@@ -4459,7 +4465,7 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
 	for_each_active_root(root) {
 		struct cgroup_subsys *ss;
-		struct cgroup *cgrp;
+		struct cgroup *cgrp, *asking_cgrp, *isol_root_cgrp;
 		int count = 0;
 
 		seq_printf(m, "%d:", root->hierarchy_id);
@@ -4470,7 +4476,9 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 				   root->name);
 		seq_putc(m, ':');
 		cgrp = task_cgroup_from_root(tsk, root);
-		retval = cgroup_path(cgrp, buf, PAGE_SIZE);
+		asking_cgrp = task_cgroup_from_root(current, root);
+		isol_root_cgrp = cgroup_get_isolation_root(asking_cgrp);
+		retval = cgroup_path(cgrp, buf, PAGE_SIZE, isol_root_cgrp);
 		if (retval < 0)
 			goto out_unlock;
 		seq_puts(m, buf);
@@ -4790,7 +4798,7 @@ static void cgroup_release_agent(struct work_struct *work)
 		pathbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!pathbuf)
 			goto continue_free;
-		if (cgroup_path(cgrp, pathbuf, PAGE_SIZE) < 0)
+		if (cgroup_path(cgrp, pathbuf, PAGE_SIZE, NULL) < 0)
 			goto continue_free;
 		agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL);
 		if (!agentbuf)
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 10131fd..6206a5c 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2583,7 +2583,7 @@ static int proc_cpuset_show(struct seq_file *m, void *unused_v)
 	retval = -EINVAL;
 	cgroup_lock();
 	css = task_subsys_state(tsk, cpuset_subsys_id);
-	retval = cgroup_path(css->cgroup, buf, PAGE_SIZE);
+	retval = cgroup_path(css->cgroup, buf, PAGE_SIZE, NULL);
 	if (retval < 0)
 		goto out_unlock;
 	seq_puts(m, buf);
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index a6710a1..bdd8e9a 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -103,7 +103,7 @@ static char *task_group_path(struct task_group *tg)
 		group_path[0] = '\0';
 		return group_path;
 	}
-	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	cgroup_path(tg->css.cgroup, group_path, PATH_MAX, NULL);
 	return group_path;
 }
 #endif
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3508777..6e5b35e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1388,7 +1388,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
 	mem_cgrp = memcg->css.cgroup;
 	task_cgrp = task_cgroup(p, mem_cgroup_subsys_id);
 
-	ret = cgroup_path(task_cgrp, memcg_name, PATH_MAX);
+	ret = cgroup_path(task_cgrp, memcg_name, PATH_MAX, NULL);
 	if (ret < 0) {
 		/*
 		 * Unfortunately, we are unable to convert to a useful name
@@ -1402,7 +1402,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
 	printk(KERN_INFO "Task in %s killed", memcg_name);
 
 	rcu_read_lock();
-	ret = cgroup_path(mem_cgrp, memcg_name, PATH_MAX);
+	ret = cgroup_path(mem_cgrp, memcg_name, PATH_MAX, NULL);
 	if (ret < 0) {
 		rcu_read_unlock();
 		goto done;
@@ -3390,7 +3390,7 @@ void mem_cgroup_print_bad_page(struct page *page)
 		if (path) {
 			rcu_read_lock();
 			ret = cgroup_path(pc->mem_cgroup->css.cgroup,
-							path, PATH_MAX);
+						path, PATH_MAX, NULL);
 			rcu_read_unlock();
 		}
 
-- 
1.7.4.1

[PATCH 4/6] cgroup: disallow task from leaving cgroup isolated root

[Witold Krecicki]

This patch makes it impossible for a task to exit cgroup isolated
root environment.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 kernel/cgroup.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f9b4bdf..c3fee33 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1850,13 +1850,17 @@ int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
 {
 	int retval;
 	struct cgroup_subsys *ss, *failed_ss = NULL;
-	struct cgroup *oldcgrp;
+	struct cgroup *oldcgrp, *isol_root;
 	struct cgroupfs_root *root = cgrp->root;
 
 	/* Nothing to do if the task is already in that cgroup */
 	oldcgrp = task_cgroup_from_root(tsk, root);
 	if (cgrp == oldcgrp)
 		return 0;
+	/* We need to check if the new cgrp is inside the isolation root */
+	isol_root = cgroup_get_isolation_root(oldcgrp);
+	if (isol_root && !cgroup_is_descendant(cgrp, isol_root))
+		return -EBUSY;
 
 	for_each_subsys(root, ss) {
 		if (ss->can_attach) {
-- 
1.7.4.1

[PATCH 5/6] cgroup: make cgroup filesystem mounts performed by task inside isolation root see its isolation root as top cgroup

[Witold Krecicki]

This patch makes all mounts of cgroup filesystems performed by task
contained in isolation root see its isolation root as top cgroup.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 kernel/cgroup.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index c3fee33..db66a82 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1504,6 +1504,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	int ret = 0;
 	struct super_block *sb;
 	struct cgroupfs_root *new_root;
+	struct dentry *dret;
 
 	/* First find the desired set of subsystems */
 	mutex_lock(&cgroup_mutex);
@@ -1624,7 +1625,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		revert_creds(cred);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);
+		dret = sb->s_root;
 	} else {
+		struct cgroup *isol_root;
 		/*
 		 * We re-used an existing hierarchy - the new root (if
 		 * any) is not needed
@@ -1632,11 +1635,20 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		cgroup_drop_root(opts.new_root);
 		/* no subsys rebinding, so refcounts don't change */
 		drop_parsed_module_refcounts(opts.subsys_bits);
+		/* If root isolation is enabled give partial root */
+		cgroup_lock();
+		isol_root = cgroup_get_isolation_root(
+				task_cgroup_from_root(current, root));
+		if (isol_root)
+			dret = isol_root->dentry;
+		else
+			dret = sb->s_root;
+		cgroup_unlock();
 	}
 
 	kfree(opts.release_agent);
 	kfree(opts.name);
-	return dget(sb->s_root);
+	return dget(dret);
 
  drop_new_super:
 	deactivate_locked_super(sb);
-- 
1.7.4.1

[PATCH 6/6] cgroup: documentation of isolation_root cgroup flag

[Witold Krecicki]

Documentation of isolation_root cgroup flag.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 Documentation/cgroups/cgroups.txt |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/cgroups/cgroups.txt b/Documentation/cgroups/cgroups.txt
index cd67e90..66880a6 100644
--- a/Documentation/cgroups/cgroups.txt
+++ b/Documentation/cgroups/cgroups.txt
@@ -19,7 +19,8 @@ CONTENTS:
   1.3 How are cgroups implemented ?
   1.4 What does notify_on_release do ?
   1.5 What does clone_children do ?
-  1.6 How do I use cgroups ?
+  1.6 What does isolation_root do ?
+  1.7 How do I use cgroups ?
 2. Usage Examples and Syntax
   2.1 Basic Usage
   2.2 Attaching processes
@@ -304,7 +305,22 @@ subsystem of the newly created cgroup. Usually when this callback is
 implemented for a subsystem, it copies the values of the parent
 subsystem, this is the case for the cpuset.
 
-1.6 How do I use cgroups ?
+1.6 What does isolation_root do ?
+---------------------------------
+
+If the isolation_root flag is enabled (1) in a cgroup, then all tasks
+in this cgroup and its children will see this group as its top group
+in /proc/$PID/cgroup. Also mounts of cgroup filesystem performed by a task
+in isolation root cgroup will see isolation root cgroup as top cgroup.
+It is impossible for a task to exit isolated root. Flag may only be set or
+cleared when the cgroup is not in use.
+The preffered way to use isolation_root for containers (eg. lxc) is to
+create 'master' cgroup - /sys/fs/cgroup/cpu/container_name with settings
+for the container, then create /sys/fs/cgroup/cpu/container_name/root with
+isolation_root flag set and add the 'init' task for the container to
+this cgroup.
+
+1.7 How do I use cgroups ?
 --------------------------
 
 To start a new job that is to be contained within a cgroup, using
-- 
1.7.4.1

Re: [PATCH 3/6] cgroup: add 'root' parameter to cgroup_path function

[Witold Krecicki]

Original patch was broken (showing incorrect cgroups inside the container), 
here goes the corrected version.



This patch adds 'root' parameter to 'cgroup_path' function. When it's
not NULL creating cgroup path will stop when current cgroup is 'root'.
A task contained in cgroup isolation root will see its cgroup in
/proc/$PID/cgroup with isolation root as top.

Signed-off-by: Witold Krecicki <wpk@culm.net>
---
 block/blk-cgroup.c     |    2 +-
 include/linux/cgroup.h |    3 ++-
 kernel/cgroup.c        |   21 +++++++++++++++------
 kernel/cpuset.c        |    2 +-
 kernel/sched_debug.c   |    2 +-
 mm/memcontrol.c        |    6 +++---
 6 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b596e54..2dd96e7 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -491,7 +491,7 @@ void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg,
 	blkg->plid = plid;
 	spin_unlock_irqrestore(&blkcg->lock, flags);
 	/* Need to take css reference ? */
-	cgroup_path(blkcg->css.cgroup, blkg->path, sizeof(blkg->path));
+	cgroup_path(blkcg->css.cgroup, blkg->path, sizeof(blkg->path), NULL);
 	blkg->dev = dev;
 }
 EXPORT_SYMBOL_GPL(blkiocg_add_blkio_group);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 6965591..3b36554 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -436,7 +436,8 @@ int cgroup_add_files(struct cgroup *cgrp,
 
 int cgroup_is_removed(const struct cgroup *cgrp);
 
-int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
+int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen,
+		 const struct cgroup *root);
 
 int cgroup_task_count(const struct cgroup *cgrp);
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 0177472..182c435 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1716,18 +1716,20 @@ static inline struct cftype *__d_cft(struct dentry 
*dentry)
  * @cgrp: the cgroup in question
  * @buf: the buffer to write the path into
  * @buflen: the length of the buffer
+ * @root: root cgroup for isolation
  *
  * Called with cgroup_mutex held or else with an RCU-protected cgroup
  * reference.  Writes path of cgroup into buf.  Returns 0 on success,
  * -errno on error.
  */
-int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
+int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen,
+		 const struct cgroup *root)
 {
 	char *start;
 	struct dentry *dentry = rcu_dereference_check(cgrp->dentry,
 						      cgroup_lock_is_held());
 
-	if (!dentry || cgrp == dummytop) {
+	if (!dentry || cgrp == dummytop || cgrp == root) {
 		/*
 		 * Inactive subsystems have no dentry for their root
 		 * cgroup
@@ -1748,7 +1750,12 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, 
int buflen)
 		cgrp = cgrp->parent;
 		if (!cgrp)
 			break;
-
+		if (cgrp == root) {
+			if (--start < buf)
+				return -ENAMETOOLONG;
+			*start = '/';
+			break;
+		}
 		dentry = rcu_dereference_check(cgrp->dentry,
 					       cgroup_lock_is_held());
 		if (!cgrp->parent)
@@ -4459,7 +4466,7 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 
 	for_each_active_root(root) {
 		struct cgroup_subsys *ss;
-		struct cgroup *cgrp;
+		struct cgroup *cgrp, *asking_cgrp, *isol_root_cgrp;
 		int count = 0;
 
 		seq_printf(m, "%d:", root->hierarchy_id);
@@ -4470,7 +4477,9 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
 				   root->name);
 		seq_putc(m, ':');
 		cgrp = task_cgroup_from_root(tsk, root);
-		retval = cgroup_path(cgrp, buf, PAGE_SIZE);
+		asking_cgrp = task_cgroup_from_root(current, root);
+		isol_root_cgrp = cgroup_get_isolation_root(asking_cgrp);
+		retval = cgroup_path(cgrp, buf, PAGE_SIZE, isol_root_cgrp);
 		if (retval < 0)
 			goto out_unlock;
 		seq_puts(m, buf);
@@ -4790,7 +4799,7 @@ static void cgroup_release_agent(struct work_struct 
*work)
 		pathbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!pathbuf)
 			goto continue_free;
-		if (cgroup_path(cgrp, pathbuf, PAGE_SIZE) < 0)
+		if (cgroup_path(cgrp, pathbuf, PAGE_SIZE, NULL) < 0)
 			goto continue_free;
 		agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL);
 		if (!agentbuf)
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 10131fd..6206a5c 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2583,7 +2583,7 @@ static int proc_cpuset_show(struct seq_file *m, void 
*unused_v)
 	retval = -EINVAL;
 	cgroup_lock();
 	css = task_subsys_state(tsk, cpuset_subsys_id);
-	retval = cgroup_path(css->cgroup, buf, PAGE_SIZE);
+	retval = cgroup_path(css->cgroup, buf, PAGE_SIZE, NULL);
 	if (retval < 0)
 		goto out_unlock;
 	seq_puts(m, buf);
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index a6710a1..bdd8e9a 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -103,7 +103,7 @@ static char *task_group_path(struct task_group *tg)
 		group_path[0] = '\0';
 		return group_path;
 	}
-	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	cgroup_path(tg->css.cgroup, group_path, PATH_MAX, NULL);
 	return group_path;
 }
 #endif
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3508777..6e5b35e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1388,7 +1388,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, 
struct task_struct *p)
 	mem_cgrp = memcg->css.cgroup;
 	task_cgrp = task_cgroup(p, mem_cgroup_subsys_id);
 
-	ret = cgroup_path(task_cgrp, memcg_name, PATH_MAX);
+	ret = cgroup_path(task_cgrp, memcg_name, PATH_MAX, NULL);
 	if (ret < 0) {
 		/*
 		 * Unfortunately, we are unable to convert to a useful name
@@ -1402,7 +1402,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, 
struct task_struct *p)
 	printk(KERN_INFO "Task in %s killed", memcg_name);
 
 	rcu_read_lock();
-	ret = cgroup_path(mem_cgrp, memcg_name, PATH_MAX);
+	ret = cgroup_path(mem_cgrp, memcg_name, PATH_MAX, NULL);
 	if (ret < 0) {
 		rcu_read_unlock();
 		goto done;
@@ -3390,7 +3390,7 @@ void mem_cgroup_print_bad_page(struct page *page)
 		if (path) {
 			rcu_read_lock();
 			ret = cgroup_path(pc->mem_cgroup->css.cgroup,
-							path, PATH_MAX);
+						path, PATH_MAX, NULL);
 			rcu_read_unlock();
 		}
 
-- 
1.7.4.1

Re: [PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[Witold Krecicki]

Dnia czwartek, 13 października 2011 o 07:30:27 Matthew Helsley napisał(a):
> Unless I'm misunderstanding the problem this idea does not look great. It
> looks vaguely like mount namespaces, subtree mounts, and chroot all sort of
> rolled into one and presented in a very strange interface specific to
> cgroups.
>
> We already have the ability to mount a subtree of the filesystem as if it
> were the root of that filesystem. This seems somewhat similar except we
> lack the permission to mount or see anything above that particular subtree
> (kind of like chroot).
>
> I think it would be better to make a generic way to do this with user
> namespaces and mount namespaces and without this odd flag file. It may be
> useful for other filesystems within containers.
> (...)
'Mount namespace' is just a part of this patchset, the fact that cgroups are 
exported as a filesystems is 'on top', but we need to control everything 'from 
the bottom'.
For this to really work we have to guarantee that:
1. A task inside a container will never see that it's root cgroup is an 
'isolation root' either:
 a) via cgroupfs interface
 b) via /proc/[pid]/cgroup
2. A task inside an isolation root will not be able to leave the isolation 
root it's in
3. We have full control from the outside on the isolation root

The 'mount namespaces'/mounting 'with path' is a solution only to 1b, and even 
if it'd be implemented the fact is that I can't imagine any other situation 
where this would be useful .

-- 
WK

Re: [PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[Paul Menage]

On Fri, Sep 30, 2011 at 4:36 AM, Witold Krecicki <wpk@culm.net> wrote:
> This patchset adds namespace-like feature to the existing cgroup system.
> When used with a container system (eg. lxc) it allows containers to have
> its own cgroup hierarchy, enabling use of 'systemd' (using cgroups) inside
> a container.

The basic idea is, I think, a necessary one for containers to be fully
useful. This patch set looks well designed.

After talking with Eric Biederman at LPC about the virtualizability of
containers, I was wondering whether we could go even further, and say
that a hierarchy (in the sense of a tree of cgroups with a bound set
of subsystems) could be broken at the point of an isolation root. The
container could then construct its own hierarchies with potentially
different combinations of subsystems.

>From the point of view of any given subsystem, its cgroups would still
all form a single tree, but potentially threading through multiple
hierarchies. (So there would need to be an explicit tree of pointers
running through the cgroup_subsys_state structs, as well as the tree
running through the cgroups, and a subsystem would have to only read
its own tree.)

Probably the rule for allowing this would have to be something like:
if you try to mount a cgroup filesystem with a combination of
subsystems that would normally give an EBUSY (since one or more of the
subsystems are in use but the combination requested does not exactly
match the existing combination) allow it if the cgroups of the
requesting task for the requested subsystems are all isolation roots,
and if they all contain the exact same set of tasks. At that point a
new hierarchy would be created.

There are definitely some fiddly issues to deal with in this idea,
though, and I doubt if it'll be around any time soon, but it would be
nice if we could set up the API in your isolation patches so that it
fits in with possible future ideas.

If more isolation options are likely in the future, which I think they
are, then having a separate file show up in every single cgroup for
something that's going to be relevant to very few actual cgroups seems
a bit bloated. How about making the file be called just 'isolation' or
'virtualization' and have it be a series of flags, so that it's
forward expandable.

Bit 0 could be 'root' as you have now.
Bit 1 could be 'hidden' - if the hidden bit is set, then the
subsystems in this hierarchy don't even show up in /proc/cgroup or
/proc/self/cgroup for this cgroup.

>
> I'm really not sure if the 'mount' part (patch 5) is done correctly, please
> review carefully.

It looks simple, I agree, and as though it *ought* to work. My first
worry with this was that if the parent system unmount the hierarchy,
and all the tasks in the child container died (so its namespace was
cleaned up), what would keep the root or the parent-created hierarchy
alive? But I think that since the super-block also has a reference on
the root dentry itself, it should be OK.

Paul

Re: [PATCH 1/6] cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem

[Paul Menage]

On Fri, Sep 30, 2011 at 4:36 AM, Witold Krecicki <wpk@culm.net> wrote:
> + */
> +static struct cgroup *cgroup_get_isolation_root(struct cgroup *cgrp)
> +{
> +       for (;;) {
> +               if (!cgrp)
> +                       return NULL;
> +               if (isolation_root(cgrp))
> +                       return cgrp;
> +               cgrp = cgrp->parent;
> +       }
> +       return NULL;
> +}

What are the locking requirements for cgroup_get_isolation_root?

> +
> +static int cgroup_isolation_root_write(struct cgroup *cgrp,
> +                                    struct cftype *cft,
> +                                    u64 val)
> +{
> +       if (cgrp->parent == NULL)
> +               return -EBUSY;

EPERM or EINVAL would be more appropriate here, I think.

> +       if (atomic_read(&cgrp->count))
> +               return -EBUSY;

Also need to check for child cgroups, and return -EBUSY?

> +       if (val)
> +               set_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
> +       else
> +               clear_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
> +       return 0;
> +}

Arguably we need  to take a lock in these two functions, both to guard
against racing with a creation of a child cgroup or moving in a task
while trying to set its isolation root flag, and to synchronize
readers and writers of the flag. But to be honest I think we can say
that this is one of those cases where we can say that the sysadmin is
dumb enough to have races between his container setup code and his
container population code the result is undefined, as long as we don't
actually crash or leak :-)

Paul

Re: [PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[Witold Krecicki]

Dnia czwartek, 20 października 2011 o 12:11:54 Paul Menage napisał(a):
> After talking with Eric Biederman at LPC about the virtualizability of
> containers, I was wondering whether we could go even further, and say
> that a hierarchy (in the sense of a tree of cgroups with a bound set
> of subsystems) could be broken at the point of an isolation root. The
> container could then construct its own hierarchies with potentially
> different combinations of subsystems.
I tried to make it as simple as possible - and this approach (looking at patch 
length) seemed to be the simplest (we really don't care about 'other' cgroups 
that might appear). Other approaches would probably require major rewrites of 
cgroups code.

> > I'm really not sure if the 'mount' part (patch 5) is done correctly,
> > please review carefully.
> 
> It looks simple, I agree, and as though it *ought* to work. My first
> worry with this was that if the parent system unmount the hierarchy,
> and all the tasks in the child container died (so its namespace was
> cleaned up), what would keep the root or the parent-created hierarchy
> alive? But I think that since the super-block also has a reference on
> the root dentry itself, it should be OK.
I've tested it and 'it works' so no problem there :)
I'm currently testing this setup with several containers launched under 
modified LXC (creating 'container_name' cgroup, then 'container_name/rootfs', 
then setting them both to desired values and putting init in 
'container_name/rootfs') - no problems so far.

-- 
Witold Krecicki

Re: [PATCH 0/6] cgroup: add isolation_root flag, poor man's namespaces for cgroups

[Paul Menage]

On Thu, Oct 20, 2011 at 3:25 AM, Witold Krecicki <wpk@culm.net> wrote:
> I tried to make it as simple as possible - and this approach (looking at patch
> length) seemed to be the simplest (we really don't care about 'other' cgroups
> that might appear).

Right, this is a nicely simple approach that gets rid of some nasty
incompatibilities between containers and cgroups, so it's a great
first step. But it still limits what containers can do with cgroups
(in terms of combining subsystems in hierarchies), hence the idea that
we plan what we might want to do later, and at least make the API
something that could also accommodate the possible future. (In the
same sense that the cgroups mount API has always supported the
possibility of mounting a subsystem on multiple hierarchies at once,
even if the patches to implement it didn't appear for another year or
so and are still languishing and bit-rotting).

> Other approaches would probably require major rewrites of cgroups code.

Not major rewrites, really, but definitely some fiddly replumbing.

Paul

Re: [PATCH 1/6] cgroup: add cgroup.isolation_root flag entry to the cgroup filesystem

[Witold Krecicki]

Dnia czwartek, 20 października 2011 o 12:12:37 Paul Menage napisał(a):
> On Fri, Sep 30, 2011 at 4:36 AM, Witold Krecicki <wpk@culm.net> wrote:
> > + */
> > +static struct cgroup *cgroup_get_isolation_root(struct cgroup *cgrp)
> > +{
> > +       for (;;) {
> > +               if (!cgrp)
> > +                       return NULL;
> > +               if (isolation_root(cgrp))
> > +                       return cgrp;
> > +               cgrp = cgrp->parent;
> > +       }
> > +       return NULL;
> > +}
> 
> What are the locking requirements for cgroup_get_isolation_root?

IMHO none - this function is performed only if there is a task in a cgroup, so 
cgroup cannot be removed (and a task cannot leave cgroup) - also, you cannot 
change isolation_root flag if the isolation_root is busy. 


In here we really need locking, after fixes:
static int cgroup_isolation_root_write(struct cgroup *cgrp,
                                     struct cftype *cft, 
                                     u64 val)
{
        if (cgrp->parent == NULL)
                return -EPERM; 
        cgroup_lock();
        if (atomic_read(&cgrp->count) || !list_empty(&cgrp->children)) {
                cgroup_unlock();
                return -EBUSY;
        }
        if (val)
                set_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
        else
                clear_bit(CGRP_ISOLATION_ROOT, &cgrp->flags);
        cgroup_unlock();
        return 0;
}
> Arguably we need  to take a lock in these two functions, both to guard
> against racing with a creation of a child cgroup or moving in a task
> while trying to set its isolation root flag, and to synchronize
> readers and writers of the flag. But to be honest I think we can say
> that this is one of those cases where we can say that the sysadmin is
> dumb enough to have races between his container setup code and his
> container population code the result is undefined, as long as we don't
> actually crash or leak :-)
I guess that fixes that problem?
-- 
WK