Skip to contentSkip to conversation

cgroup: substitude per-cgroup id with per-subsys id

PATCH In review Jianyu Zhan 4 participants 14 messages Apr 22, 2014 – Apr 23, 2014 1 message not in the archive

Subsystems (5)
Networking Network drivers Memory management cgroup +1 more subsystems
lore
Share & tools
lore

Jianyu Zhan

author
#0 lore raw

cover cgroup: substitude per-cgroup id with per-subsys id

Currently, cgrp->id is only used to look up css's. As cgroup and css's lifetimes is now decoupled, it should be made per-subsystem and moved to css->css_id so that lookups are successful until the target css is released.

Patch 1-3 are prep patches.
Patch 4 do the coverting job.

Thanks!

Jianyu Zhan (4):
cgroup: introduce helper css_to_id() mm/memcontrol.c: use accessor to get id from css
netprio_cgroup: use accessor to get id from css
cgroup: convert from per-cgroup id to per-subsys id

 include/linux/cgroup.h    | 27 ++++++-------
 kernel/cgroup.c           | 96 +++++++++++++++++++++++++----------------------
 mm/memcontrol.c           |  8 ++--
 net/core/netprio_cgroup.c |  8 ++--
 4 files changed, 74 insertions(+), 65 deletions(-)
Signature · 1 line
2.0.0-rc0
author
#1 lore raw

PATCH 1/4 cgroup: introduce helper css_to_id()

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

Some subsystems dereference the per-cgrpu id directly, but this is implementation-specific, so it should be transparent for subsystems.

Use this accessor instead.

Signed-off-by Jianyu Zhan <[email protected]>
---
 include/linux/cgroup.h |  1 +
 kernel/cgroup.c        | 11 +++++++++++
 2 files changed, 12 insertions(+)
include/linux/cgroup.h +1 -0
index 4b38e2d..de31b2a 100644
@@ -754,6 +754,7 @@ struct cgroup_subsys_state *css_next_child(struct cgroup_subsys_state *pos,
 					   struct cgroup_subsys_state *parent);
 
 struct cgroup_subsys_state *css_from_id(int id, struct cgroup_subsys *ss);
+int css_to_id(struct cgroup_subsys_state *css);
 
 /**
  * css_for_each_child - iterate through children of a css
kernel/cgroup.c +11 -0
index 930569c..80c1cf3 100644
@@ -5153,6 +5153,17 @@ struct cgroup_subsys_state *css_tryget_from_dir(struct dentry *dentry,
 }
 
 /**
+ * css_to_id - get the id of a css
+ * @css: the css of interest
+ *
+ * Return the corresponding id for the @css.
+ */
+int css_to_id(struct cgroup_subsys_state *css)
+{
+	return css->cgroup->id;
+}
+
+/**
  * css_from_id - lookup css by id
  * @id: the cgroup id
  * @ss: cgroup subsys to be looked into
Signature · 1 line
2.0.0-rc0
author
#3 lore raw

PATCH 2/4 mm/memcontrol.c: use accessor to get id from css

In reply to Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail. Use the accessor css_from_id() instead.

This patch has no functional change.

Signed-off-by Jianyu Zhan <[email protected]>
---
 mm/memcontrol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
mm/memcontrol.c +4 -4
index 80d9e38..46333cb 100644
@@ -528,10 +528,10 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
 	/*
-	 * The ID of the root cgroup is 0, but memcg treat 0 as an
-	 * invalid ID, so we return (cgroup_id + 1).
+	 * The ID of css for the root cgroup is 0, but memcg treat 0 as an
+	 * invalid ID, so we return (id + 1).
 	 */
-	return memcg->css.cgroup->id + 1;
+	return css_to_id(&memcg->css) + 1;
 }
 
 static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)

@@ -6407,7 +6407,7 @@ mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 	struct mem_cgroup *parent = mem_cgroup_from_css(css_parent(css));
 
-	if (css->cgroup->id > MEM_CGROUP_ID_MAX)
+	if (css_to_id(css) > MEM_CGROUP_ID_MAX)
 		return -ENOSPC;
 
 	if (!parent)
Signature · 7 lines
2.0.0-rc0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#9 lore raw

PATCH 3/4 netprio_cgroup: use accessor to get id from css

In reply to Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail.

Use the accessor css_from_id() instead.

This patch has no functional change.

Signed-off-by Jianyu Zhan <[email protected]>
---
 net/core/netprio_cgroup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
net/core/netprio_cgroup.c +4 -4
index ce285c6..fc21035 100644
@@ -89,7 +89,7 @@ static int extend_netdev_table(struct net_device *dev, u32 target_idx)
 static u32 netprio_prio(struct cgroup_subsys_state *css, struct net_device *dev)
 {
 	struct netprio_map *map = rcu_dereference_rtnl(dev->priomap);
-	int id = css->cgroup->id;
+	int id = css_to_id(css);
 
 	if (map && id < map->priomap_len)
 		return map->priomap[id];

@@ -109,7 +109,7 @@ static int netprio_set_prio(struct cgroup_subsys_state *css,
 			    struct net_device *dev, u32 prio)
 {
 	struct netprio_map *map;
-	int id = css->cgroup->id;
+	int id = css_to_id(css);
 	int ret;
 
 	/* avoid extending priomap for zero writes */

@@ -170,7 +170,7 @@ static void cgrp_css_free(struct cgroup_subsys_state *css)
 
 static u64 read_prioidx(struct cgroup_subsys_state *css, struct cftype *cft)
 {
-	return css->cgroup->id;
+	return css_to_id(css);
 }
 
 static int read_priomap(struct seq_file *sf, void *v)

@@ -222,7 +222,7 @@ static void net_prio_attach(struct cgroup_subsys_state *css,
 			    struct cgroup_taskset *tset)
 {
 	struct task_struct *p;
-	void *v = (void *)(unsigned long)css->cgroup->id;
+	void *v = (void *)(unsigned long)css_to_id(css);
 
 	cgroup_taskset_for_each(p, tset) {
 		task_lock(p);
Signature · 1 line
2.0.0-rc0
author
#12 lore raw

PATCH 4/4 cgroup: convert from per-cgroup id to per-subsys id

In reply to Jianyu Zhan

Currently, cgrp->id is only used to look up css's. As cgroup and css's lifetimes is now decoupled, it should be made per-subsystem and moved to css->css_id so that lookups are successful until the target css is released.

Signed-off-by Jianyu Zhan <[email protected]>
---
 include/linux/cgroup.h | 26 +++++++--------
 kernel/cgroup.c        | 87 ++++++++++++++++++++++++--------------------------
 2 files changed, 55 insertions(+), 58 deletions(-)
include/linux/cgroup.h +13 -13
index de31b2a..66085bd 100644
@@ -62,6 +62,16 @@ struct cgroup_subsys_state {
 	/* the parent css */
 	struct cgroup_subsys_state *parent;
 
+	/*
+	 * per css id
+	 *
+	 * The css id of cgrp_dfl_root for each subsys is always 0, and
+	 * a new css will be assigned with a smallest available ID.
+	 *
+	 * Allocating/Removing ID must be protected by cgroup_mutex.
+	 */
+	int css_id;
+
 	unsigned long flags;
 
 	/* percpu_ref killing and RCU release */

@@ -141,16 +151,6 @@ enum {
 struct cgroup {
 	unsigned long flags;		/* "unsigned long" so bitops work */
 
-	/*
-	 * idr allocated in-hierarchy ID.
-	 *
-	 * The ID of the root cgroup is always 0, and a new cgroup
-	 * will be assigned with a smallest available ID.
-	 *
-	 * Allocating/Removing ID must be protected by cgroup_mutex.
-	 */
-	int id;
-
 	/* the number of attached css's */
 	int nr_css;
 

@@ -329,9 +329,6 @@ struct cgroup_root {
 	/* Hierarchy-specific flags */
 	unsigned long flags;
 
-	/* IDs for cgroups in this hierarchy */
-	struct idr cgroup_idr;
-
 	/* The path to use for release notifications. */
 	char release_agent_path[PATH_MAX];
 

@@ -655,6 +652,9 @@ struct cgroup_subsys {
 	/* link to parent, protected by cgroup_lock() */
 	struct cgroup_root *root;
 
+	/* IDs for css'es of this subsys */
+	struct idr css_idr;
+
 	/*
 	 * List of cftypes.  Each entry is the first entry of an array
 	 * terminated by zero length name.
kernel/cgroup.c +42 -45

190 lines not on this page · raw message

Signature · 1 line
2.0.0-rc0
author
#4 lore raw

Re: [PATCH 2/4] mm/memcontrol.c: use accessor to get id from css

In reply to Jianyu Zhan

Cc Andrew.

Thanks,
Jianyu Zhan

Quoted 41 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail. Use the accessor css_from_id() instead.

This patch has no functional change.

Signed-off-by Jianyu Zhan <[email protected]>
---
 mm/memcontrol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
mm/memcontrol.c +4 -5
index 80d9e38..46333cb 100644
@@ -528,10 +528,10 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
        /*
-        * The ID of the root cgroup is 0, but memcg treat 0 as an
-        * invalid ID, so we return (cgroup_id + 1).
+        * The ID of css for the root cgroup is 0, but memcg treat 0 as an
+        * invalid ID, so we return (id + 1).
         */
-       return memcg->css.cgroup->id + 1;
+       return css_to_id(&memcg->css) + 1;
 }
 
 static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)

@@ -6407,7 +6407,7 @@ mem_cgroup_css_online(struct cgroup_subsys_state *css)
        struct mem_cgroup *memcg = mem_cgroup_from_css(css);
        struct mem_cgroup *parent = mem_cgroup_from_css(css_parent(css));
 
-       if (css->cgroup->id > MEM_CGROUP_ID_MAX)
+       if (css_to_id(css) > MEM_CGROUP_ID_MAX)
                return -ENOSPC;
 
        if (!parent)
--
Quoted 2 lines

2.0.0-rc0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#13 lore raw

Re: [PATCH 4/4] cgroup: convert from per-cgroup id to per-subsys id

In reply to Jianyu Zhan

Hi, all.

Sorry, previous patch has a minor fault, and cause a unitialized variable warning. I've fixed it up in this.

Renewed patch:
---

Currently, cgrp->id is only used to look up css's. As cgroup and css's lifetimes is now decoupled, it should be made per-subsystem and moved to css->css_id so that lookups are successful until the target css is released.

Signed-off-by Jianyu Zhan <[email protected]>
---
 include/linux/cgroup.h | 26 +++++++--------
 kernel/cgroup.c        | 88 ++++++++++++++++++++++++--------------------------
 2 files changed, 56 insertions(+), 58 deletions(-)
include/linux/cgroup.h +13 -13
index de31b2a..66085bd 100644
@@ -62,6 +62,16 @@ struct cgroup_subsys_state {
 	/* the parent css */
 	struct cgroup_subsys_state *parent;
 
+	/*
+	 * per css id
+	 *
+	 * The css id of cgrp_dfl_root for each subsys is always 0, and
+	 * a new css will be assigned with a smallest available ID.
+	 *
+	 * Allocating/Removing ID must be protected by cgroup_mutex.
+	 */
+	int css_id;
+
 	unsigned long flags;
 
 	/* percpu_ref killing and RCU release */

@@ -141,16 +151,6 @@ enum {
 struct cgroup {
 	unsigned long flags;		/* "unsigned long" so bitops work */
 
-	/*
-	 * idr allocated in-hierarchy ID.
-	 *
-	 * The ID of the root cgroup is always 0, and a new cgroup
-	 * will be assigned with a smallest available ID.
-	 *
-	 * Allocating/Removing ID must be protected by cgroup_mutex.
-	 */
-	int id;
-
 	/* the number of attached css's */
 	int nr_css;
 

@@ -329,9 +329,6 @@ struct cgroup_root {
 	/* Hierarchy-specific flags */
 	unsigned long flags;
 
-	/* IDs for cgroups in this hierarchy */
-	struct idr cgroup_idr;
-
 	/* The path to use for release notifications. */
 	char release_agent_path[PATH_MAX];
 

@@ -655,6 +652,9 @@ struct cgroup_subsys {
 	/* link to parent, protected by cgroup_lock() */
 	struct cgroup_root *root;
 
+	/* IDs for css'es of this subsys */
+	struct idr css_idr;
+
 	/*
 	 * List of cftypes.  Each entry is the first entry of an array
 	 * terminated by zero length name.
kernel/cgroup.c +43 -45

191 lines not on this page · raw message

Signature · 1 line
2.0.0-rc0
author
#5 lore raw

Re: [PATCH 2/4] mm/memcontrol.c: use accessor to get id from css

In reply to Jianyu Zhan
Quoted 7 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail. Use the accessor css_from_id() instead.

This patch has no functional change.

Hi, I'm sorry. This patch should be applied on top of its previous patch:
https://lkml.org/lkml/2014/4/22/54

Sorry for my fault , not cc'ing this mail-list in that patch.

Thanks,
Jianyu Zhan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Acked-by on v1 2/4
#6 lore raw

Re: [PATCH 2/4] mm/memcontrol.c: use accessor to get id from css

In reply to Jianyu Zhan
Quoted 9 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail. Use the accessor css_from_id() instead.

This patch has no functional change.

Signed-off-by Jianyu Zhan <[email protected]>
Acked-by Michal Hocko <[email protected]>

Thanks!

Quoted 34 lines
---
 mm/memcontrol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
mm/memcontrol.c +4 -4
index 80d9e38..46333cb 100644
@@ -528,10 +528,10 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
 	/*
-	 * The ID of the root cgroup is 0, but memcg treat 0 as an
-	 * invalid ID, so we return (cgroup_id + 1).
+	 * The ID of css for the root cgroup is 0, but memcg treat 0 as an
+	 * invalid ID, so we return (id + 1).
 	 */
-	return memcg->css.cgroup->id + 1;
+	return css_to_id(&memcg->css) + 1;
 }
 
 static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)

@@ -6407,7 +6407,7 @@ mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 	struct mem_cgroup *parent = mem_cgroup_from_css(css_parent(css));
 
-	if (css->cgroup->id > MEM_CGROUP_ID_MAX)
+	if (css_to_id(css) > MEM_CGROUP_ID_MAX)
 		return -ENOSPC;
 
 	if (!parent)
Signature · 1 line
2.0.0-rc0
Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Acked-by on v1 3/4
#10 lore raw

Re: [PATCH 3/4] netprio_cgroup: use accessor to get id from css

In reply to Jianyu Zhan
Quoted 64 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail.

Use the accessor css_from_id() instead.

This patch has no functional change.

Signed-off-by Jianyu Zhan <[email protected]>
---
 net/core/netprio_cgroup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
net/core/netprio_cgroup.c +4 -4
index ce285c6..fc21035 100644
@@ -89,7 +89,7 @@ static int extend_netdev_table(struct net_device *dev, u32 target_idx)
 static u32 netprio_prio(struct cgroup_subsys_state *css, struct net_device *dev)
 {
 	struct netprio_map *map = rcu_dereference_rtnl(dev->priomap);
-	int id = css->cgroup->id;
+	int id = css_to_id(css);
 
 	if (map && id < map->priomap_len)
 		return map->priomap[id];

@@ -109,7 +109,7 @@ static int netprio_set_prio(struct cgroup_subsys_state *css,
 			    struct net_device *dev, u32 prio)
 {
 	struct netprio_map *map;
-	int id = css->cgroup->id;
+	int id = css_to_id(css);
 	int ret;
 
 	/* avoid extending priomap for zero writes */

@@ -170,7 +170,7 @@ static void cgrp_css_free(struct cgroup_subsys_state *css)
 
 static u64 read_prioidx(struct cgroup_subsys_state *css, struct cftype *cft)
 {
-	return css->cgroup->id;
+	return css_to_id(css);
 }
 
 static int read_priomap(struct seq_file *sf, void *v)

@@ -222,7 +222,7 @@ static void net_prio_attach(struct cgroup_subsys_state *css,
 			    struct cgroup_taskset *tset)
 {
 	struct task_struct *p;
-	void *v = (void *)(unsigned long)css->cgroup->id;
+	void *v = (void *)(unsigned long)css_to_id(css);
 
 	cgroup_taskset_for_each(p, tset) {
 		task_lock(p);
Signature · 7 lines
2.0.0-rc0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Acked-by Neil Horman <[email protected]>
maintainer
#2 lore raw

Re: [PATCH 1/4] cgroup: introduce helper css_to_id()

In reply to Jianyu Zhan
Quoted 5 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

Some subsystems dereference the per-cgrpu id directly, but this is implementation-specific, so it should be transparent for subsystems.

Hmm... why would cgroup ID be implementation-specific? It's a published field

Quoted 1 line

Use this accessor instead.

I'm not a big believer of trivial accessors. They tend to obfuscate things more than helping anything. Here, we need to switch from cgrp->id to css->id. Wrapping cgrp->id by css_to_id() doesn't really help anything especially because there will be cases where we'd actually want cgroup IDs instead of css IDs too.

Thanks.

Signature · 1 line
tejun
maintainer
#11 lore raw

Re: [PATCH 3/4] netprio_cgroup: use accessor to get id from css

In reply to Jianyu Zhan
Quoted 9 lines · Jianyu Zhan

This is a prepared patch for converting from per-cgroup id to per-subsystem id.

We should not access per-cgroup id directly, since this is implemetation detail.

Use the accessor css_from_id() instead.

This patch has no functional change.

netprio_cgroup is a cgroup identifying controller. We *do* want to use per-cgroup IDs for it.

Thanks.

Signature · 1 line
tejun
maintainer
#14 lore raw

Re: [PATCH 4/4] cgroup: convert from per-cgroup id to per-subsys id

In reply to Jianyu Zhan
Quoted 5 lines · Jianyu Zhan

Hi, all.

Sorry, previous patch has a minor fault, and cause a unitialized variable warning. I've fixed it up in this.

Quoted 7 lines

Renewed patch:
---

Currently, cgrp->id is only used to look up css's. As cgroup and css's lifetimes is now decoupled, it should be made per-subsystem and moved to css->css_id so that lookups are successful until the target css is released.

So, css ID can't replace cgroup ID as whole. It's just that there are cases where we're currently using cgroup IDs where we should be using css IDs. Also, we need to update css creation error path RCU safe so that idr lookup is always RCU safe. I already had patches queued for posting. Will post them soon.

Thanks.

Signature · 1 line
tejun
author
#8 lore raw

Re: [PATCH 2/4] mm/memcontrol.c: use accessor to get id from css

In reply to a message not in the archive
Quoted 1 line · Andrew Morton

I'd expect Tejun to process this series, but you didn't cc him on 2/4.

Oh, I reposed too much confidence in get_maintainer.pl. I thought it would cc tj as usual. :-)

Tj said he has a patchset queued for addressing this problem and will be sent out soon,
so just forget this patch and wait.

Thanks,
Jianyu Zhan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>