cgroup: initialize cgrp->dentry before css_alloc()

This is a preparaton for later patches.

- What do we gain from cpu_cgroup_css_online():

After ss->css_alloc() and before ss->css_online(), there's a small window that tg->css.cgroup is NULL. With this change, tg won't be seen before ss->css_online(), where it's added to the global list, so we're guaranteed we'll never see NULL tg->css.cgroup.

- What do we gain from cpu_cgroup_css_offline():

tg is freed via RCU, so is cgroup. Without this change, This is how synchronization works:

cgroup_rmdir()
no ss->css_offline()
diput()
syncornize_rcu()
ss->css_free() <-- unregister tg, and free it via call_rcu()
kfree_rcu(cgroup) <-- wait possible refs to cgroup, and free cgroup

We can't just kfree(cgroup), because tg might access tg->css.cgroup.

With this change:

cgroup_rmdir()
ss->css_offline() <-- unregister tg
diput()

  synchronize_rcu()    <-- wait possible refs to tg and cgroup
  ss->css_free()       <-- free tg
  kfree_rcu(cgroup)    <-- free cgroup

As you see, kfree_rcu() is redundant now.

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

Acked-by Ingo Molnar <[email protected]>

Quoted 1 line · Ingo Molnar

Acked-by Ingo Molnar <[email protected]>

Ingo, if you don't objecdt, I'll take these through cgroup/for-3.9 branch with other cgroup changes. Pleaes holler if you object.

include/linux/sched.h extern void normalize_rt_tasks(void); · line 2756

Quoted 5 lines · Li Zefan

extern struct task_group *sched_create_group(struct task_group *parent);

include/linux/sched.h +3 -0

+extern void sched_online_group(struct task_group *tg,
+			       struct task_group *parent);
 extern void sched_destroy_group(struct task_group *tg);
+extern void sched_offline_group(struct task_group *tg);

Btw., a rename of these APIs might be in order, along the usual patterns:

sched_task_group_create()
sched_task_group_online()
sched_task_group_offline()
sched_task_group_destroy()

etc.

that way the naming is more hierarchical and there's no clash with the sched_group concept which is about something else.

(In a separate patch.)

Since commit 48ddbe194623ae089cc0576e60363f2d2e85662a ("cgroup: make css->refcnt clearing on cgroup removal optional"), each css holds a ref on cgroup's dentry, so cgroup_diput() won't be called until all css' refs go down to 0, which invalids the comments.

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

---
 kernel/cgroup.c | 6 ------
 1 file changed, 6 deletions(-)

kernel/cgroup.c +0 -0

index f18b0d9..368cff1 100644

Quoted 17 lines

Since commit 48ddbe194623ae089cc0576e60363f2d2e85662a ("cgroup: make css->refcnt clearing on cgroup removal optional"), each css holds a ref on cgroup's dentry, so cgroup_diput() won't be called until all css' refs go down to 0, which invalids the comments.

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

---
 kernel/cgroup.c | 6 ------
 1 file changed, 6 deletions(-)

kernel/cgroup.c +0 -0

index f18b0d9..368cff1 100644
@@ -899,12 +899,6 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
 		struct cgroup *cgrp = dentry->d_fsdata;
 

kernel/cgroup.c static void cgroup_diput(struct dentry *dentry, struct inode *inode) · line 907

Quoted 7 lines

kernel/cgroup.c +0 -6

 		BUG_ON(!(cgroup_is_removed(cgrp)));
-		/* It's possible for external users to be holding css
-		 * reference counts on a cgroup; css_put() needs to
-		 * be able to access the cgroup after decrementing
-		 * the reference count in order to know if it needs to
-		 * queue the cgroup to be handled by the release
-		 * agent */

The latter half is still true tho. I applied the patch anyway as the comment isn't all that useful without covering everything intended to be covered with RCU. Can you please write up another patch documenting what's covered by RCU (stuff that we intend to keep guaranteeing to be safe to access only w/ RCU)?

With this change, we're guaranteed that cgroup_path() won't see NULL cgrp->dentry, and thus we can remove the NULL check in it.

(Well, it's not true, because dummptop.dentry is always NULL)

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