cgroup: make serial_nr_cursor available throughout cgroup.c

PATCH In review Li Zefan 2 participants 6 messages Jun 18, 2013 cgroup

lore

Share & tools

lore

Li Zefan

author

2013-06-18 10:48 UTC #0 lore raw

PATCH 1/2 cgroup: make serial_nr_cursor available throughout cgroup.c

The next patch will use it to determine if a cgroup is newly created while we're iterating the cgroup hierarchy.

Signed-off-by Li Zefan <[email protected]>

---

These two patches are based on the fixes I just sent.

---
 kernel/cgroup.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

kernel/cgroup.c +10 -2

index e8a40e3..b1d98f2 100644
@@ -198,6 +198,15 @@ static DEFINE_IDR(cgroup_hierarchy_idr);
 
 static struct cgroup_name root_cgroup_name = { .name = "/" };
 
+/*
+ * Assign a monotonically increasing serial number.  It guarantees
+ * cgroups with bigger numbers are newer than those with smaller
+ * numbers. Also with the list appending, it guarantees that sibling
+ * cgroups are always sorted in the ascending serial number order on
+ * the parent's ->children.
+ */
+static atomic64_t cgroup_serial_nr_cursor = ATOMIC64_INIT(0);
+
 /* This flag indicates whether tasks in the fork and exit paths should
  * check for fork/exit handlers to call. This avoids us having to do
  * extra work in the fork/exit path if none of the subsystems need to

@@ -4212,7 +4221,6 @@ static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 			     umode_t mode)
 {
-	static atomic64_t serial_nr_cursor = ATOMIC64_INIT(0);
 	struct cgroup *cgrp;
 	struct cgroup_name *name;
 	struct cgroupfs_root *root = parent->root;

@@ -4305,7 +4313,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	 * sorted in the ascending serial number order on the parent's
 	 * ->children.
 	 */
-	cgrp->serial_nr = atomic64_inc_return(&serial_nr_cursor);
+	cgrp->serial_nr = atomic64_inc_return(&cgroup_serial_nr_cursor);
 
 	/* allocation complete, commit to creation */
 	list_add_tail(&cgrp->allcg_node, &root->allcg_list);

Signature · 1 line

1.8.0.2

Li Zefan

author

2013-06-18 10:53 UTC #1 lore raw

PATCH 1/2 cgroup: make serial_nr_cursor available throughout cgroup.c

The next patch will use it to determine if a cgroup is newly created while we're iterating the cgroup hierarchy.

Signed-off-by Li Zefan <[email protected]>

---

forgot to remove the old comment in cgroup_create().

---
 kernel/cgroup.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

kernel/cgroup.c +10 -8

index e8a40e3..60383e8 100644
@@ -198,6 +198,15 @@ static DEFINE_IDR(cgroup_hierarchy_idr);
 
 static struct cgroup_name root_cgroup_name = { .name = "/" };
 
+/*
+ * Assign a monotonically increasing serial number.  It guarantees
+ * cgroups with bigger numbers are newer than those with smaller
+ * numbers. Also with the list appending, it guarantees that sibling
+ * cgroups are always sorted in the ascending serial number order on
+ * the parent's ->children.
+ */
+static atomic64_t cgroup_serial_nr_cursor = ATOMIC64_INIT(0);
+
 /* This flag indicates whether tasks in the fork and exit paths should
  * check for fork/exit handlers to call. This avoids us having to do
  * extra work in the fork/exit path if none of the subsystems need to

@@ -4212,7 +4221,6 @@ static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 			     umode_t mode)
 {
-	static atomic64_t serial_nr_cursor = ATOMIC64_INIT(0);
 	struct cgroup *cgrp;
 	struct cgroup_name *name;
 	struct cgroupfs_root *root = parent->root;

@@ -4299,13 +4307,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		goto err_free_all;
 	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
-	/*
-	 * Assign a monotonically increasing serial number.  With the list
-	 * appending below, it guarantees that sibling cgroups are always
-	 * sorted in the ascending serial number order on the parent's
-	 * ->children.
-	 */
-	cgrp->serial_nr = atomic64_inc_return(&serial_nr_cursor);
+	cgrp->serial_nr = atomic64_inc_return(&cgroup_serial_nr_cursor);
 
 	/* allocation complete, commit to creation */
 	list_add_tail(&cgrp->allcg_node, &root->allcg_list);

Signature · 1 line

1.8.0.2

Tejun Heo

maintainer

2013-06-18 18:16 UTC #2 lore raw

From 794611a1dfcb055d7d41ce133378dd8197d73e38 Mon Sep 17 00:00:00 2001
From: Li Zefan <[email protected]>
Date: Tue, 18 Jun 2013 18:53:53 +0800

The next patch will use it to determine if a cgroup is newly created while we're iterating the cgroup hierarchy.

tj: Rephrased the comment on top of cgroup_serial_nr_cursor.

Signed-off-by Li Zefan <[email protected]>

Signed-off-by Tejun Heo <[email protected]>

---
Applied to cgroup/for-3.11 with minor update.

Thanks.

 kernel/cgroup.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

kernel/cgroup.c +10 -8

index 1d4f471..e6571ca 100644
@@ -198,6 +198,15 @@ static DEFINE_IDR(cgroup_hierarchy_idr);
 
 static struct cgroup_name root_cgroup_name = { .name = "/" };
 
+/*
+ * Assign a monotonically increasing serial number to cgroups.  It
+ * guarantees cgroups with bigger numbers are newer than those with smaller
+ * numbers.  Also, as cgroups are always appended to the parent's
+ * ->children list, it guarantees that sibling cgroups are always sorted in
+ * the ascending serial number order on the list.
+ */
+static atomic64_t cgroup_serial_nr_cursor = ATOMIC64_INIT(0);
+
 /* This flag indicates whether tasks in the fork and exit paths should
  * check for fork/exit handlers to call. This avoids us having to do
  * extra work in the fork/exit path if none of the subsystems need to

@@ -4222,7 +4231,6 @@ static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 			     umode_t mode)
 {
-	static atomic64_t serial_nr_cursor = ATOMIC64_INIT(0);
 	struct cgroup *cgrp;
 	struct cgroup_name *name;
 	struct cgroupfs_root *root = parent->root;

@@ -4309,13 +4317,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		goto err_free_all;
 	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
-	/*
-	 * Assign a monotonically increasing serial number.  With the list
-	 * appending below, it guarantees that sibling cgroups are always
-	 * sorted in the ascending serial number order on the parent's
-	 * ->children.
-	 */
-	cgrp->serial_nr = atomic64_inc_return(&serial_nr_cursor);
+	cgrp->serial_nr = atomic64_inc_return(&cgroup_serial_nr_cursor);
 
 	/* allocation complete, commit to creation */
 	list_add_tail(&cgrp->allcg_node, &root->allcg_list);

Signature · 1 line

1.8.2.1

Li Zefan

author

2013-06-18 10:48 UTC #3 lore raw

PATCH 2/2 cgroup: convert cgroup_cft_commit() to use cgroup_for_each_descendant_pre()

In reply to Li Zefan

We used root->allcg_list to iterate cgroup hierarchy because at that time cgroup_for_each_descendant_pre() hasn't been invented.

Signed-off-by Li Zefan <[email protected]>

---
 include/linux/cgroup.h |  6 -----
 kernel/cgroup.c        | 62 +++++++++++++++++++++++++++-----------------------
 2 files changed, 33 insertions(+), 35 deletions(-)

include/linux/cgroup.h +0 -6

index 835918c..f70b116 100644
@@ -206,9 +206,6 @@ struct cgroup {
 	 */
 	struct list_head cset_links;
 
-	struct list_head allcg_node;	/* cgroupfs_root->allcg_list */
-	struct list_head cft_q_node;	/* used during cftype add/rm */
-
 	/*
 	 * Linked list running through all cgroups that can
 	 * potentially be reaped by the release agent. Protected by

@@ -317,9 +314,6 @@ struct cgroupfs_root {
 	/* A list running through the active hierarchies */
 	struct list_head root_list;
 
-	/* All cgroups on this root, cgroup_mutex protected */
-	struct list_head allcg_list;
-
 	/* Hierarchy-specific flags */
 	unsigned long flags;

kernel/cgroup.c +33 ~~-29~~

131 lines not on this page · raw message

Signature · 1 line

1.8.0.2

Tejun Heo

maintainer

2013-06-18 18:16 UTC #4 lore raw

PATCH 2/2 cgroup: convert cgroup_cft_commit() to use cgroup_for_each_descendant_pre()

From e8c82d20a9f729cf4b9f73043f7fd4e0872bebfd Mon Sep 17 00:00:00 2001
From: Li Zefan <[email protected]>
Date: Tue, 18 Jun 2013 18:48:37 +0800

We used root->allcg_list to iterate cgroup hierarchy because at that time cgroup_for_each_descendant_pre() hasn't been invented.

tj: In cgroup_cfts_commit(), s/@serial_nr/@update_upto/, move the
assignment right above releasing cgroup_mutex and explain what's going on there.

Signed-off-by Li Zefan <[email protected]>

Signed-off-by Tejun Heo <[email protected]>

---
Applied to cgroup/for-3.11 with some minor updates.

Thanks.

 include/linux/cgroup.h |  6 ----
 kernel/cgroup.c        | 80 +++++++++++++++++++++++++++-----------------------
 2 files changed, 44 insertions(+), 42 deletions(-)

include/linux/cgroup.h +0 -6

index f975227..b283658 100644
@@ -206,9 +206,6 @@ struct cgroup {
 	 */
 	struct list_head cset_links;
 
-	struct list_head allcg_node;	/* cgroupfs_root->allcg_list */
-	struct list_head cft_q_node;	/* used during cftype add/rm */
-
 	/*
 	 * Linked list running through all cgroups that can
 	 * potentially be reaped by the release agent. Protected by

@@ -313,9 +310,6 @@ struct cgroupfs_root {
 	/* A list running through the active hierarchies */
 	struct list_head root_list;
 
-	/* All cgroups on this root, cgroup_mutex protected */
-	struct list_head allcg_list;
-
 	/* Hierarchy-specific flags */
 	unsigned long flags;

kernel/cgroup.c +44 ~~-36~~

142 lines not on this page · raw message

Signature · 1 line

1.8.2.1

Tejun Heo

maintainer

2013-06-18 18:18 UTC #5 lore raw

PATCH 3/3 cgroup: clean up cgroup_serial_nr_cursor

In reply to Li Zefan

From 00356bd5f0f5e04183fb15805eb29e97c2fc20ac Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Tue, 18 Jun 2013 11:14:22 -0700

cgroup_serial_nr_cursor was created atomic64_t because I thought it was never gonna used for anything other than assigning unique numbers to cgroups and didn't want to worry about synchronization; however, now we're using it as an event-stamp to distinguish cgroups created before and after certain point which assumes that it's protected by cgroup_mutex.

Let's make it clear by making it a u64. Also, rename it to cgroup_serial_nr_next and make it point to the next nr to allocate so that where it's pointing to is clear and more conventional.

Signed-off-by Tejun Heo <[email protected]>

Cc Li Zefan <[email protected]>

---
Applied to cgroup/for-3.11 as a follow-up.

Thanks.

 kernel/cgroup.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

kernel/cgroup.c +8 -7

index 0ed7d8d..65f333e 100644
@@ -203,9 +203,10 @@ static struct cgroup_name root_cgroup_name = { .name = "/" };
  * guarantees cgroups with bigger numbers are newer than those with smaller
  * numbers.  Also, as cgroups are always appended to the parent's
  * ->children list, it guarantees that sibling cgroups are always sorted in
- * the ascending serial number order on the list.
+ * the ascending serial number order on the list.  Protected by
+ * cgroup_mutex.
  */
-static atomic64_t cgroup_serial_nr_cursor = ATOMIC64_INIT(0);
+static u64 cgroup_serial_nr_next = 1;
 
 /* This flag indicates whether tasks in the fork and exit paths should
  * check for fork/exit handlers to call. This avoids us having to do

@@ -2803,7 +2804,7 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
 	struct super_block *sb = ss->root->sb;
 	struct dentry *prev = NULL;
 	struct inode *inode;
-	u64 update_upto;
+	u64 update_before;
 
 	/* %NULL @cfts indicates abort and don't bother if @ss isn't attached */
 	if (!cfts || ss->root == &rootnode ||

@@ -2815,9 +2816,9 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
 	/*
 	 * All cgroups which are created after we drop cgroup_mutex will
 	 * have the updated set of files, so we only need to update the
-	 * cgroups created before the current @cgroup_serial_nr_cursor.
+	 * cgroups created before the current @cgroup_serial_nr_next.
 	 */
-	update_upto = atomic64_read(&cgroup_serial_nr_cursor);
+	update_before = cgroup_serial_nr_next;
 
 	mutex_unlock(&cgroup_mutex);

@@ -2844,7 +2845,7 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
 
 		mutex_lock(&inode->i_mutex);
 		mutex_lock(&cgroup_mutex);
-		if (cgrp->serial_nr <= update_upto && !cgroup_is_dead(cgrp))
+		if (cgrp->serial_nr < update_before && !cgroup_is_dead(cgrp))
 			cgroup_addrm_files(cgrp, ss, cfts, is_add);
 		mutex_unlock(&cgroup_mutex);
 		mutex_unlock(&inode->i_mutex);

@@ -4327,7 +4328,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		goto err_free_all;
 	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
-	cgrp->serial_nr = atomic64_inc_return(&cgroup_serial_nr_cursor);
+	cgrp->serial_nr = cgroup_serial_nr_next++;
 
 	/* allocation complete, commit to creation */
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);

Signature · 1 line

1.8.2.1