[PATCH 01/17] cgroup: remove incorrect dget/dput() pair in cgroup_create_dir()

[PATCH 01/17] cgroup: remove incorrect dget/dput() pair in cgroup_create_dir()

[Tejun Heo]

cgroup_create_dir() does weird dancing with dentry refcnt.  On
success, it gets and then puts it achieving nothing.  On failure, it
puts but there isn't no matching get anywhere leading to the following
oops if cgroup_create_file() fails for whatever reason.

  ------------[ cut here ]------------
  kernel BUG at /work/os/work/fs/dcache.c:552!
  invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
  Modules linked in:
  CPU 2
  Pid: 697, comm: mkdir Not tainted 3.7.0-rc4-work+ #3 Bochs Bochs
  RIP: 0010:[<ffffffff811d9c0c>]  [<ffffffff811d9c0c>] dput+0x1dc/0x1e0
  RSP: 0018:ffff88001a3ebef8  EFLAGS: 00010246
  RAX: 0000000000000000 RBX: ffff88000e5b1ef8 RCX: 0000000000000403
  RDX: 0000000000000303 RSI: 2000000000000000 RDI: ffff88000e5b1f58
  RBP: ffff88001a3ebf18 R08: ffffffff82c76960 R09: 0000000000000001
  R10: ffff880015022080 R11: ffd9bed70f48a041 R12: 00000000ffffffea
  R13: 0000000000000001 R14: ffff88000e5b1f58 R15: 00007fff57656d60
  FS:  00007ff05fcb3800(0000) GS:ffff88001fd00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00000000004046f0 CR3: 000000001315f000 CR4: 00000000000006e0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
  Process mkdir (pid: 697, threadinfo ffff88001a3ea000, task ffff880015022080)
  Stack:
   ffff88001a3ebf48 00000000ffffffea 0000000000000001 0000000000000000
   ffff88001a3ebf38 ffffffff811cc889 0000000000000001 ffff88000e5b1ef8
   ffff88001a3ebf68 ffffffff811d1fc9 ffff8800198d7f18 ffff880019106ef8
  Call Trace:
   [<ffffffff811cc889>] done_path_create+0x19/0x50
   [<ffffffff811d1fc9>] sys_mkdirat+0x59/0x80
   [<ffffffff811d2009>] sys_mkdir+0x19/0x20
   [<ffffffff81be1e02>] system_call_fastpath+0x16/0x1b
  Code: 00 48 8d 90 18 01 00 00 48 89 93 c0 00 00 00 4c 89 a0 18 01 00 00 48 8b 83 a0 00 00 00 83 80 28 01 00 00 01 e8 e6 6f a0 00 eb 92 <0f> 0b 66 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41
  RIP  [<ffffffff811d9c0c>] dput+0x1dc/0x1e0
   RSP <ffff88001a3ebef8>
  ---[ end trace 1277bcfd9561ddb0 ]---

Fix it by dropping the unnecessary dget/dput() pair.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 kernel/cgroup.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 0f8fa6a..d0803f0 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2684,9 +2684,7 @@ static int cgroup_create_dir(struct cgroup *cgrp, struct dentry *dentry,
 		dentry->d_fsdata = cgrp;
 		inc_nlink(parent->d_inode);
 		rcu_assign_pointer(cgrp->dentry, dentry);
-		dget(dentry);
 	}
-	dput(dentry);
 
 	return error;
 }
-- 
1.7.11.7

Re: [PATCH 01/17] cgroup: remove incorrect dget/dput() pair in cgroup_create_dir()

[Li Zefan]

On 2012/11/13 11:01, Tejun Heo wrote:
> cgroup_create_dir() does weird dancing with dentry refcnt.  On
> success, it gets and then puts it achieving nothing.  On failure, it
> puts but there isn't no matching get anywhere leading to the following
> oops if cgroup_create_file() fails for whatever reason.
> 
>   ------------[ cut here ]------------
>   kernel BUG at /work/os/work/fs/dcache.c:552!
>   invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>   Modules linked in:
>   CPU 2
>   Pid: 697, comm: mkdir Not tainted 3.7.0-rc4-work+ #3 Bochs Bochs
>   RIP: 0010:[<ffffffff811d9c0c>]  [<ffffffff811d9c0c>] dput+0x1dc/0x1e0
>   RSP: 0018:ffff88001a3ebef8  EFLAGS: 00010246
>   RAX: 0000000000000000 RBX: ffff88000e5b1ef8 RCX: 0000000000000403
>   RDX: 0000000000000303 RSI: 2000000000000000 RDI: ffff88000e5b1f58
>   RBP: ffff88001a3ebf18 R08: ffffffff82c76960 R09: 0000000000000001
>   R10: ffff880015022080 R11: ffd9bed70f48a041 R12: 00000000ffffffea
>   R13: 0000000000000001 R14: ffff88000e5b1f58 R15: 00007fff57656d60
>   FS:  00007ff05fcb3800(0000) GS:ffff88001fd00000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 00000000004046f0 CR3: 000000001315f000 CR4: 00000000000006e0
>   DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>   DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>   Process mkdir (pid: 697, threadinfo ffff88001a3ea000, task ffff880015022080)
>   Stack:
>    ffff88001a3ebf48 00000000ffffffea 0000000000000001 0000000000000000
>    ffff88001a3ebf38 ffffffff811cc889 0000000000000001 ffff88000e5b1ef8
>    ffff88001a3ebf68 ffffffff811d1fc9 ffff8800198d7f18 ffff880019106ef8
>   Call Trace:
>    [<ffffffff811cc889>] done_path_create+0x19/0x50
>    [<ffffffff811d1fc9>] sys_mkdirat+0x59/0x80
>    [<ffffffff811d2009>] sys_mkdir+0x19/0x20
>    [<ffffffff81be1e02>] system_call_fastpath+0x16/0x1b
>   Code: 00 48 8d 90 18 01 00 00 48 89 93 c0 00 00 00 4c 89 a0 18 01 00 00 48 8b 83 a0 00 00 00 83 80 28 01 00 00 01 e8 e6 6f a0 00 eb 92 <0f> 0b 66 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41
>   RIP  [<ffffffff811d9c0c>] dput+0x1dc/0x1e0
>    RSP <ffff88001a3ebef8>
>   ---[ end trace 1277bcfd9561ddb0 ]---
> 
> Fix it by dropping the unnecessary dget/dput() pair.
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  kernel/cgroup.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 0f8fa6a..d0803f0 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -2684,9 +2684,7 @@ static int cgroup_create_dir(struct cgroup *cgrp, struct dentry *dentry,
>  		dentry->d_fsdata = cgrp;
>  		inc_nlink(parent->d_inode);
>  		rcu_assign_pointer(cgrp->dentry, dentry);
> -		dget(dentry);
>  	}
> -	dput(dentry);
>  

The code has been there since cgroup was merged into mainline!

>  	return error;
>  }
> 

Re: [PATCH 01/17] cgroup: remove incorrect dget/dput() pair in cgroup_create_dir()

[Tejun Heo]

On Mon, Nov 19, 2012 at 04:08:04PM +0800, Li Zefan wrote:
> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 0f8fa6a..d0803f0 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -2684,9 +2684,7 @@ static int cgroup_create_dir(struct cgroup *cgrp, struct dentry *dentry,
> >  		dentry->d_fsdata = cgrp;
> >  		inc_nlink(parent->d_inode);
> >  		rcu_assign_pointer(cgrp->dentry, dentry);
> > -		dget(dentry);
> >  	}
> > -	dput(dentry);
> >  
> 
> The code has been there since cgroup was merged into mainline!

I know.  Sometimes it's scary how many latent bugs we have in the
kernel and how long many of them have been around.  At other times,
it's comforting.  I mean, there's a pretty good chance that other
people don't notice my screw ups, right?  :P

-- 
tejun

[PATCH 02/17] cgroup: initialize cgrp->allcg_node in init_cgroup_housekeeping()

[Tejun Heo]

Not strictly necessary but it's annoying to have uninitialized
list_head around.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d0803f0..ed0e177 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1381,6 +1381,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
 	INIT_LIST_HEAD(&cgrp->children);
 	INIT_LIST_HEAD(&cgrp->files);
 	INIT_LIST_HEAD(&cgrp->css_sets);
+	INIT_LIST_HEAD(&cgrp->allcg_node);
 	INIT_LIST_HEAD(&cgrp->release_list);
 	INIT_LIST_HEAD(&cgrp->pidlists);
 	mutex_init(&cgrp->pidlist_mutex);
-- 
1.7.11.7

[PATCH 03/17] cgroup: open-code cgroup_create_dir()

[Tejun Heo]

The operation order of cgroup creation is about to change and
cgroup_create_dir() is more of a hindrance than a proper abstraction.
Open-code it by moving the parent nlink adjustment next to self nlink
adjustment in cgroup_create_file() and the rest to cgroup_create().

This patch doesn't introduce any behavior change.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index ed0e177..b042673 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2652,6 +2652,7 @@ static int cgroup_create_file(struct dentry *dentry, umode_t mode,
 
 		/* start off with i_nlink == 2 (for "." entry) */
 		inc_nlink(inode);
+		inc_nlink(dentry->d_parent->d_inode);
 
 		/* start with the directory inode held, so that we can
 		 * populate it without racing with another mkdir */
@@ -2666,30 +2667,6 @@ static int cgroup_create_file(struct dentry *dentry, umode_t mode,
 	return 0;
 }
 
-/*
- * cgroup_create_dir - create a directory for an object.
- * @cgrp: the cgroup we create the directory for. It must have a valid
- *        ->parent field. And we are going to fill its ->dentry field.
- * @dentry: dentry of the new cgroup
- * @mode: mode to set on new directory.
- */
-static int cgroup_create_dir(struct cgroup *cgrp, struct dentry *dentry,
-				umode_t mode)
-{
-	struct dentry *parent;
-	int error = 0;
-
-	parent = cgrp->parent->dentry;
-	error = cgroup_create_file(dentry, S_IFDIR | mode, cgrp->root->sb);
-	if (!error) {
-		dentry->d_fsdata = cgrp;
-		inc_nlink(parent->d_inode);
-		rcu_assign_pointer(cgrp->dentry, dentry);
-	}
-
-	return error;
-}
-
 /**
  * cgroup_file_mode - deduce file mode of a control file
  * @cft: the control file in question
@@ -4138,10 +4115,13 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
 
-	err = cgroup_create_dir(cgrp, dentry, mode);
+	err = cgroup_create_file(dentry, S_IFDIR | mode, sb);
 	if (err < 0)
 		goto err_remove;
 
+	dentry->d_fsdata = cgrp;
+	rcu_assign_pointer(cgrp->dentry, dentry);
+
 	for_each_subsys(root, ss) {
 		/* each css holds a ref to the cgroup's dentry */
 		dget(dentry);
-- 
1.7.11.7

[PATCH 04/17] cgroup: create directory before linking while creating a new cgroup

[Tejun Heo]

While creating a new cgroup, cgroup_create() links the newly allocated
cgroup into various places before trying to create its directory.
Because cgroup life-cycle is tied to the vfs objects, this makes it
impossible to use cgroup_rmdir() for rolling back creation - the
removal logic depends on having full vfs objects.

This patch moves directory creation above linking and collect linking
operations to one place.  This allows directory creation failure to
share error exit path with css allocation failures and any failure
sites afterwards (to be added later) can use cgroup_rmdir() logic to
undo creation.

It also removes the need to check whether cgroup->dentry is %NULL in
cgroup_path.  If a cgroup is visible, its dentry is guaranteed to be
visible too.

Note that this also makes the memory barriers around cgroup->dentry,
which currently is misleadingly using RCU operations, unnecessary.
This will be handled in the next patch.

While at it, locking BUG_ON() on i_mutex is converted to
lockdep_assert_held().

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b042673..40707cd 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1760,7 +1760,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 	struct dentry *dentry = rcu_dereference_check(cgrp->dentry,
 						      cgroup_lock_is_held());
 
-	if (!dentry || cgrp == dummytop) {
+	if (cgrp == dummytop) {
 		/*
 		 * Inactive subsystems have no dentry for their root
 		 * cgroup
@@ -4112,15 +4112,22 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		}
 	}
 
-	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
-	root->number_of_cgroups++;
-
+	/*
+	 * Create directory.  cgroup_create_file() returns with the new
+	 * directory locked on success so that it can be populated without
+	 * dropping cgroup_mutex.
+	 */
 	err = cgroup_create_file(dentry, S_IFDIR | mode, sb);
 	if (err < 0)
-		goto err_remove;
+		goto err_destroy;
+	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
+	/* allocation complete, commit to creation */
 	dentry->d_fsdata = cgrp;
 	rcu_assign_pointer(cgrp->dentry, dentry);
+	list_add_tail(&cgrp->allcg_node, &root->allcg_list);
+	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
+	root->number_of_cgroups++;
 
 	for_each_subsys(root, ss) {
 		/* each css holds a ref to the cgroup's dentry */
@@ -4131,11 +4138,6 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 			ss->post_create(cgrp);
 	}
 
-	/* The cgroup directory was pre-locked for us */
-	BUG_ON(!mutex_is_locked(&cgrp->dentry->d_inode->i_mutex));
-
-	list_add_tail(&cgrp->allcg_node, &root->allcg_list);
-
 	err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
 	/* If err < 0, we have a half-filled directory - oh well ;) */
 
@@ -4144,20 +4146,12 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 
 	return 0;
 
- err_remove:
-
-	list_del_rcu(&cgrp->sibling);
-	root->number_of_cgroups--;
-
- err_destroy:
-
+err_destroy:
 	for_each_subsys(root, ss) {
 		if (cgrp->subsys[ss->subsys_id])
 			ss->destroy(cgrp);
 	}
-
 	mutex_unlock(&cgroup_mutex);
-
 	/* Release the reference count that we took on the superblock */
 	deactivate_super(sb);
 err_free:
-- 
1.7.11.7

Re: [PATCH 04/17] cgroup: create directory before linking while creating a new cgroup

[Li Zefan]

On 2012/11/13 11:01, Tejun Heo wrote:
> While creating a new cgroup, cgroup_create() links the newly allocated
> cgroup into various places before trying to create its directory.
> Because cgroup life-cycle is tied to the vfs objects, this makes it
> impossible to use cgroup_rmdir() for rolling back creation - the
> removal logic depends on having full vfs objects.
> 
> This patch moves directory creation above linking and collect linking
> operations to one place.  This allows directory creation failure to
> share error exit path with css allocation failures and any failure
> sites afterwards (to be added later) can use cgroup_rmdir() logic to
> undo creation.
> 

> It also removes the need to check whether cgroup->dentry is %NULL in
> cgroup_path.  If a cgroup is visible, its dentry is guaranteed to be
> visible too.
> 

I'm afraid this isn't safe.

The cgroup is visible to a controller soon after ss->create(), and then
the controller might call cgroup_path() while cgrp->dentry is still NULL.

This can happen for cpu cgroup I think, and in fact the NULL check was
added for this reason IIRC.

> Note that this also makes the memory barriers around cgroup->dentry,
> which currently is misleadingly using RCU operations, unnecessary.
> This will be handled in the next patch.
> 
> While at it, locking BUG_ON() on i_mutex is converted to
> lockdep_assert_held().
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  kernel/cgroup.c | 32 +++++++++++++-------------------
>  1 file changed, 13 insertions(+), 19 deletions(-)

Re: [PATCH 04/17] cgroup: create directory before linking while creating a new cgroup

[Tejun Heo]

Hello, Li.

On Wed, Nov 14, 2012 at 11:20:47AM +0800, Li Zefan wrote:
> > It also removes the need to check whether cgroup->dentry is %NULL in
> > cgroup_path.  If a cgroup is visible, its dentry is guaranteed to be
> > visible too.
> 
> I'm afraid this isn't safe.
> 
> The cgroup is visible to a controller soon after ss->create(), and then
> the controller might call cgroup_path() while cgrp->dentry is still NULL.

Hmmm... I can't find any case where ss->create() is calling
cgroup_path().  Do you remember which one that was?

> This can happen for cpu cgroup I think, and in fact the NULL check was
> added for this reason IIRC.

Then, I think we better move the dentry assignment further up.  It's
really silly to be unable to answer "what's this new cgroup's name?"
for any callback really.  Will update accordingly.

Thanks.

-- 
tejun

Re: [PATCH 04/17] cgroup: create directory before linking while creating a new cgroup

[Li Zefan]

On 2012/11/15 3:04, Tejun Heo wrote:
> Hello, Li.
> 
> On Wed, Nov 14, 2012 at 11:20:47AM +0800, Li Zefan wrote:
>>> It also removes the need to check whether cgroup->dentry is %NULL in
>>> cgroup_path.  If a cgroup is visible, its dentry is guaranteed to be
>>> visible too.
>>
>> I'm afraid this isn't safe.
>>
>> The cgroup is visible to a controller soon after ss->create(), and then
>> the controller might call cgroup_path() while cgrp->dentry is still NULL.
> 
> Hmmm... I can't find any case where ss->create() is calling
> cgroup_path().  Do you remember which one that was?
> 

I meant cgroup_create() may race with cgroup_path(). We used to have
this race:

(cpu0)                          (cpu1)
mkdir                           cat /proc/sched_debug

cgroup_create()
  cpu_cgroup_create()
    register_fair_sched_group()
      list_add_rcu(...)
				print_cfs_stats()
				  for_each_leaf_cfs_rq()
				    print_cfs_rq()
				      cgroup_path()
  css->cgroup = cgroup;
  cgroup->dentry = dentry;

In this case, cgroup_path() might access NULL cgroup or NULL dentry.

But I've looked the current scheduler code, no race anymore, because now the
cfs_rq won't be added to the gloal list until there's task in the cgroup.
(in the old time it was added to the list when creating a cgroup)

[PATCH v2 04/17] cgroup: create directory before linking while creating a new cgroup

[Tejun Heo]

While creating a new cgroup, cgroup_create() links the newly allocated
cgroup into various places before trying to create its directory.
Because cgroup life-cycle is tied to the vfs objects, this makes it
impossible to use cgroup_rmdir() for rolling back creation - the
removal logic depends on having full vfs objects.

This patch moves directory creation above linking and collect linking
operations to one place.  This allows directory creation failure to
share error exit path with css allocation failures and any failure
sites afterwards (to be added later) can use cgroup_rmdir() logic to
undo creation.

Note that this also makes the memory barriers around cgroup->dentry,
which currently is misleadingly using RCU operations, unnecessary.
This will be handled in the next patch.

While at it, locking BUG_ON() on i_mutex is converted to
lockdep_assert_held().

v2: Patch originally removed %NULL dentry check in cgroup_path();
    however, Li pointed out that this patch doesn't make it
    unnecessary as ->create() may call cgroup_path().  Drop the
    change for now.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 kernel/cgroup.c |   30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4112,15 +4112,22 @@ static long cgroup_create(struct cgroup
 		}
 	}
 
-	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
-	root->number_of_cgroups++;
-
+	/*
+	 * Create directory.  cgroup_create_file() returns with the new
+	 * directory locked on success so that it can be populated without
+	 * dropping cgroup_mutex.
+	 */
 	err = cgroup_create_file(dentry, S_IFDIR | mode, sb);
 	if (err < 0)
-		goto err_remove;
+		goto err_destroy;
+	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
+	/* allocation complete, commit to creation */
 	dentry->d_fsdata = cgrp;
 	rcu_assign_pointer(cgrp->dentry, dentry);
+	list_add_tail(&cgrp->allcg_node, &root->allcg_list);
+	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
+	root->number_of_cgroups++;
 
 	for_each_subsys(root, ss) {
 		/* each css holds a ref to the cgroup's dentry */
@@ -4131,11 +4138,6 @@ static long cgroup_create(struct cgroup
 			ss->post_create(cgrp);
 	}
 
-	/* The cgroup directory was pre-locked for us */
-	BUG_ON(!mutex_is_locked(&cgrp->dentry->d_inode->i_mutex));
-
-	list_add_tail(&cgrp->allcg_node, &root->allcg_list);
-
 	err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
 	/* If err < 0, we have a half-filled directory - oh well ;) */
 
@@ -4144,20 +4146,12 @@ static long cgroup_create(struct cgroup
 
 	return 0;
 
- err_remove:
-
-	list_del_rcu(&cgrp->sibling);
-	root->number_of_cgroups--;
-
- err_destroy:
-
+err_destroy:
 	for_each_subsys(root, ss) {
 		if (cgrp->subsys[ss->subsys_id])
 			ss->destroy(cgrp);
 	}
-
 	mutex_unlock(&cgroup_mutex);
-
 	/* Release the reference count that we took on the superblock */
 	deactivate_super(sb);
 err_free:

[PATCH 05/17] cgroup: cgroup->dentry isn't a RCU pointer

[Tejun Heo]

cgroup->dentry is marked and used as a RCU pointer; however, it isn't
one - the final dentry put doesn't go through call_rcu().  cgroup and
dentry share the same RCU freeing rule via synchronize_rcu() in
cgroup_diput() (kfree_rcu() used on cgrp is unnecessary).  If cgrp is
accessible under RCU read lock, so is its dentry and dereferencing
cgrp->dentry doesn't need any further RCU protection or annotation.

While not being accurate, before the previous patch, the RCU accessors
served a purpose as memory barriers - cgroup->dentry used to be
assigned after the cgroup was made visible to cgroup_path(), so the
assignment and dereferencing in cgroup_path() needed the memory
barrier pair.  Now that list_add_tail_rcu() happens after
cgroup->dentry is assigned, this no longer is necessary.

Remove the now unnecessary and misleading RCU annotations from
cgroup->dentry.  To make up for the removal of rcu_dereference_check()
in cgroup_path(), add an explicit rcu_lockdep_assert(), which asserts
the dereference rule of @cgrp, not cgrp->dentry.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/linux/cgroup.h |  2 +-
 kernel/cgroup.c        | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 8f64b45..d605857 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -165,7 +165,7 @@ struct cgroup {
 	struct list_head files;		/* my files */
 
 	struct cgroup *parent;		/* my parent */
-	struct dentry __rcu *dentry;	/* cgroup fs entry, RCU protected */
+	struct dentry *dentry;		/* cgroup fs entry, RCU protected */
 
 	/* Private pointers for each registered subsystem */
 	struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 40707cd..278752e 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1756,9 +1756,11 @@ static struct kobject *cgroup_kobj;
  */
 int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 {
+	struct dentry *dentry = cgrp->dentry;
 	char *start;
-	struct dentry *dentry = rcu_dereference_check(cgrp->dentry,
-						      cgroup_lock_is_held());
+
+	rcu_lockdep_assert(rcu_read_lock_held() || cgroup_lock_is_held(),
+			   "cgroup_path() called without proper locking");
 
 	if (cgrp == dummytop) {
 		/*
@@ -1782,8 +1784,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 		if (!cgrp)
 			break;
 
-		dentry = rcu_dereference_check(cgrp->dentry,
-					       cgroup_lock_is_held());
+		dentry = cgrp->dentry;
 		if (!cgrp->parent)
 			continue;
 		if (--start < buf)
@@ -4124,7 +4125,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 
 	/* allocation complete, commit to creation */
 	dentry->d_fsdata = cgrp;
-	rcu_assign_pointer(cgrp->dentry, dentry);
+	cgrp->dentry = dentry;
 	list_add_tail(&cgrp->allcg_node, &root->allcg_list);
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
-- 
1.7.11.7

Re: [PATCH 05/17] cgroup: cgroup->dentry isn't a RCU pointer

[Glauber Costa]

On 11/13/2012 07:01 AM, Tejun Heo wrote:
> cgroup->dentry is marked and used as a RCU pointer; however, it isn't
> one - the final dentry put doesn't go through call_rcu().  cgroup and
> dentry share the same RCU freeing rule via synchronize_rcu() in
> cgroup_diput() (kfree_rcu() used on cgrp is unnecessary).  If cgrp is
> accessible under RCU read lock, so is its dentry and dereferencing
> cgrp->dentry doesn't need any further RCU protection or annotation.
> 
> While not being accurate, before the previous patch, the RCU accessors
> served a purpose as memory barriers - cgroup->dentry used to be
> assigned after the cgroup was made visible to cgroup_path(), so the
> assignment and dereferencing in cgroup_path() needed the memory
> barrier pair.  Now that list_add_tail_rcu() happens after
> cgroup->dentry is assigned, this no longer is necessary.
> 
> Remove the now unnecessary and misleading RCU annotations from
> cgroup->dentry.  To make up for the removal of rcu_dereference_check()
> in cgroup_path(), add an explicit rcu_lockdep_assert(), which asserts
> the dereference rule of @cgrp, not cgrp->dentry.
> 

Will memcontrol.c need similar amendments?

The code that lives in -mm and includes kmemcg includes the following
excerpt:

        rcu_read_lock();
        dentry = rcu_dereference(memcg->css.cgroup->dentry);
        rcu_read_unlock();

Re: [PATCH 05/17] cgroup: cgroup->dentry isn't a RCU pointer

[Tejun Heo]

Hello, Glauber.

On Wed, Nov 14, 2012 at 03:05:46PM +0400, Glauber Costa wrote:
> Will memcontrol.c need similar amendments?
> 
> The code that lives in -mm and includes kmemcg includes the following
> excerpt:
> 
>         rcu_read_lock();
>         dentry = rcu_dereference(memcg->css.cgroup->dentry);
>         rcu_read_unlock();

Ooh, yeah, hmmm... that should happen after this series gets merged.
I'll mark it as pending.

Thanks.

-- 
tejun

Re: [PATCH 05/17] cgroup: cgroup->dentry isn't a RCU pointer

[Tejun Heo]

Hello, Glauber.

On Thu, Nov 15, 2012 at 07:00:11AM +0400, Glauber Costa wrote:
> > Ooh, yeah, hmmm... that should happen after this series gets merged.
> > I'll mark it as pending.
> > 
> Btw: I did this because there is no other facility to figure out a
> memcg's "name".
> 
> I wonder if there is anyone else using this, so we could just abstract
> that in cgroup.c?

We have cgroup_path() after all, so I don't think it's crazy to have
cgroup_name().  Please feel free to add.

Thanks.

-- 
tejun

Re: [PATCH 05/17] cgroup: cgroup->dentry isn't a RCU pointer

[Glauber Costa]

On 11/14/2012 10:55 PM, Tejun Heo wrote:
> Hello, Glauber.
> 
> On Wed, Nov 14, 2012 at 03:05:46PM +0400, Glauber Costa wrote:
>> Will memcontrol.c need similar amendments?
>>
>> The code that lives in -mm and includes kmemcg includes the following
>> excerpt:
>>
>>         rcu_read_lock();
>>         dentry = rcu_dereference(memcg->css.cgroup->dentry);
>>         rcu_read_unlock();
> 
> Ooh, yeah, hmmm... that should happen after this series gets merged.
> I'll mark it as pending.
> 
> Thanks.
> 
Btw: I did this because there is no other facility to figure out a
memcg's "name".

I wonder if there is anyone else using this, so we could just abstract
that in cgroup.c?

[PATCH 06/17] cgroup: remove duplicate RCU free on struct cgroup

[Tejun Heo]

struct cgroup is made RCU-safe by synchronize_rcu() in cgroup_diput().
There is no reason to use RCU safe kfree on it after it.  Remove
cgroup->rcu_head and use kfree() instead of kfree_rcu() in
cgroup_diput().

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/linux/cgroup.h | 3 ---
 kernel/cgroup.c        | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index d605857..9dcbfa1 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -196,9 +196,6 @@ struct cgroup {
 	struct list_head pidlists;
 	struct mutex pidlist_mutex;
 
-	/* For RCU-protected deletion */
-	struct rcu_head rcu_head;
-
 	/* List of events which userspace want to receive */
 	struct list_head event_list;
 	spinlock_t event_list_lock;
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 278752e..a91e7ad 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -893,7 +893,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
 
 		simple_xattrs_free(&cgrp->xattrs);
 
-		kfree_rcu(cgrp, rcu_head);
+		kfree(cgrp);
 	} else {
 		struct cfent *cfe = __d_cfe(dentry);
 		struct cgroup *cgrp = dentry->d_parent->d_fsdata;
-- 
1.7.11.7

Re: [PATCH 06/17] cgroup: remove duplicate RCU free on struct cgroup

[Li Zefan]

On 2012/11/13 11:01, Tejun Heo wrote:
> struct cgroup is made RCU-safe by synchronize_rcu() in cgroup_diput().

but synchronize_rcu() is called before ss->destroy().

rcu_read_lock();
for_each_leaf_cfs_rq(cpu_rq(cpu), cfs_rq)
	print_cfs_rq(m, cpu, cfs_rq);
	-> call cgroup_path(task_group->css.cgroup);
rcu_read_unlock();

With this patch, if the above code race with cgroup_diput(), we might
end up accessing a cgroup which has been freed.

> There is no reason to use RCU safe kfree on it after it.  Remove
> cgroup->rcu_head and use kfree() instead of kfree_rcu() in
> cgroup_diput().
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  include/linux/cgroup.h | 3 ---
>  kernel/cgroup.c        | 2 +-
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index d605857..9dcbfa1 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -196,9 +196,6 @@ struct cgroup {
>  	struct list_head pidlists;
>  	struct mutex pidlist_mutex;
>  
> -	/* For RCU-protected deletion */
> -	struct rcu_head rcu_head;
> -
>  	/* List of events which userspace want to receive */
>  	struct list_head event_list;
>  	spinlock_t event_list_lock;
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 278752e..a91e7ad 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -893,7 +893,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
>  
>  		simple_xattrs_free(&cgrp->xattrs);
>  
> -		kfree_rcu(cgrp, rcu_head);
> +		kfree(cgrp);

This was also added to prevent a race in group scheduling code, and I think the race still
exists.



>  	} else {
>  		struct cfent *cfe = __d_cfe(dentry);
>  		struct cgroup *cgrp = dentry->d_parent->d_fsdata;
> 

Re: [PATCH 06/17] cgroup: remove duplicate RCU free on struct cgroup

[Tejun Heo]

Hello, Li.

On Mon, Nov 19, 2012 at 05:02:42PM +0800, Li Zefan wrote:
> On 2012/11/13 11:01, Tejun Heo wrote:
> > struct cgroup is made RCU-safe by synchronize_rcu() in cgroup_diput().
> 
> but synchronize_rcu() is called before ss->destroy().
> 
> rcu_read_lock();
> for_each_leaf_cfs_rq(cpu_rq(cpu), cfs_rq)
> 	print_cfs_rq(m, cpu, cfs_rq);
> 	-> call cgroup_path(task_group->css.cgroup);
> rcu_read_unlock();
> 
> With this patch, if the above code race with cgroup_diput(), we might
> end up accessing a cgroup which has been freed.

Ah, okay.  So, the problem here is that sched is using ->css_free() as
a de-registration point rather than freeing and may end up walking it
after ->css_free() is complete inside RCU period.

I think the correct solution is using ->css_offline() for that.  It's
ugly to require double RCU grace periods.

> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 278752e..a91e7ad 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -893,7 +893,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
> >  
> >  		simple_xattrs_free(&cgrp->xattrs);
> >  
> > -		kfree_rcu(cgrp, rcu_head);
> > +		kfree(cgrp);
> 
> This was also added to prevent a race in group scheduling code, and I think the race still
> exists.

Care to point out which one?  I don't think the double-RCU workaround
is a good idea.  We really should sort it out by following object
lifecycle rules consistently.

Thanks.

-- 
tejun

[PATCH 07/17] cgroup: make CSS_* flags bit masks instead of bit positions

[Tejun Heo]

Currently, CSS_* flags are defined as bit positions and manipulated
using atomic bitops.  There's no reason to use atomic bitops for them
and bit positions are clunkier to deal with than bit masks.  Make
CSS_* bit masks instead and use the usual C bitwise operators to
access them.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/linux/cgroup.h | 8 ++++----
 kernel/cgroup.c        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 9dcbfa1..e9f07ef 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -81,7 +81,7 @@ struct cgroup_subsys_state {
 
 /* bits in struct cgroup_subsys_state flags field */
 enum {
-	CSS_ROOT, /* This CSS is the root of the subsystem */
+	CSS_ROOT	= (1 << 0), /* this CSS is the root of the subsystem */
 };
 
 /* Caller must verify that the css is not for root cgroup */
@@ -100,7 +100,7 @@ static inline void __css_get(struct cgroup_subsys_state *css, int count)
 static inline void css_get(struct cgroup_subsys_state *css)
 {
 	/* We don't need to reference count the root state */
-	if (!test_bit(CSS_ROOT, &css->flags))
+	if (!(css->flags & CSS_ROOT))
 		__css_get(css, 1);
 }
 
@@ -113,7 +113,7 @@ static inline void css_get(struct cgroup_subsys_state *css)
 extern bool __css_tryget(struct cgroup_subsys_state *css);
 static inline bool css_tryget(struct cgroup_subsys_state *css)
 {
-	if (test_bit(CSS_ROOT, &css->flags))
+	if (css->flags & CSS_ROOT)
 		return true;
 	return __css_tryget(css);
 }
@@ -126,7 +126,7 @@ static inline bool css_tryget(struct cgroup_subsys_state *css)
 extern void __css_put(struct cgroup_subsys_state *css);
 static inline void css_put(struct cgroup_subsys_state *css)
 {
-	if (!test_bit(CSS_ROOT, &css->flags))
+	if (!(css->flags & CSS_ROOT))
 		__css_put(css);
 }
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index a91e7ad..d849efd 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4020,7 +4020,7 @@ static void init_cgroup_css(struct cgroup_subsys_state *css,
 	css->flags = 0;
 	css->id = NULL;
 	if (cgrp == dummytop)
-		set_bit(CSS_ROOT, &css->flags);
+		css->flags |= CSS_ROOT;
 	BUG_ON(cgrp->subsys[ss->subsys_id]);
 	cgrp->subsys[ss->subsys_id] = css;
 
-- 
1.7.11.7

[PATCH 08/17] cgroup: trivial cleanup for cgroup_init/load_subsys()

[Tejun Heo]

Consistently use @css and @dummytop in these two functions instead of
referring to them indirectly.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d849efd..6df17cb 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4332,7 +4332,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	 * pointer to this state - since the subsystem is
 	 * newly registered, all tasks and hence the
 	 * init_css_set is in the subsystem's top cgroup. */
-	init_css_set.subsys[ss->subsys_id] = dummytop->subsys[ss->subsys_id];
+	init_css_set.subsys[ss->subsys_id] = css;
 
 	need_forkexit_callback |= ss->fork || ss->exit;
 
@@ -4344,7 +4344,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	ss->active = 1;
 
 	if (ss->post_create)
-		ss->post_create(&ss->root->top_cgroup);
+		ss->post_create(dummytop);
 
 	/* this function shouldn't be used with modular subsystems, since they
 	 * need to register a subsys_id, among other things */
@@ -4456,7 +4456,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	ss->active = 1;
 
 	if (ss->post_create)
-		ss->post_create(&ss->root->top_cgroup);
+		ss->post_create(dummytop);
 
 	/* success! */
 	mutex_unlock(&cgroup_mutex);
-- 
1.7.11.7

[PATCH 09/17] cgroup: lock cgroup_mutex in cgroup_init_subsys()

[Tejun Heo]

Make cgroup_init_subsys() grab cgroup_mutex while initializing a
subsystem so that all helpers and callbacks are called under the
context they expect.  This isn't strictly necessary as
cgroup_init_subsys() doesn't race with anybody but will allow adding
lockdep assertions.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 6df17cb..76f1a01 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4317,6 +4317,8 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 
 	printk(KERN_INFO "Initializing cgroup subsys %s\n", ss->name);
 
+	mutex_lock(&cgroup_mutex);
+
 	/* init base cftset */
 	cgroup_init_cftsets(ss);
 
@@ -4346,6 +4348,8 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	if (ss->post_create)
 		ss->post_create(dummytop);
 
+	mutex_unlock(&cgroup_mutex);
+
 	/* this function shouldn't be used with modular subsystems, since they
 	 * need to register a subsys_id, among other things */
 	BUG_ON(ss->module);
-- 
1.7.11.7

[PATCH 10/17] cgroup: fix harmless bugs in cgroup_load_subsys() fail path and cgroup_unload_subsys()

[Tejun Heo]

* If idr init fails, cgroup_load_subsys() cleared dummytop->subsys[]
  before calilng ->destroy() making CSS inaccessible to the callback,
  and didn't unlink ss->sibling.  As no modular controller uses
  ->use_id, this doesn't cause any actual problems.

* cgroup_unload_subsys() was forgetting to free idr, call
  ->pre_destroy() and clear ->active.  As there currently is no
  modular controller which uses ->use_id, ->pre_destroy() or ->active,
  this doesn't cause any actual problems.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 76f1a01..c5368c0 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4420,9 +4420,10 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	if (ss->use_id) {
 		int ret = cgroup_init_idr(ss, css);
 		if (ret) {
-			dummytop->subsys[ss->subsys_id] = NULL;
 			ss->destroy(dummytop);
+			dummytop->subsys[ss->subsys_id] = NULL;
 			subsys[ss->subsys_id] = NULL;
+			list_del_init(&ss->sibling);
 			mutex_unlock(&cgroup_mutex);
 			return ret;
 		}
@@ -4490,7 +4491,19 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
 	 */
 	BUG_ON(ss->root != &rootnode);
 
+	/* ->pre_destroy() should be called outside cgroup_mutex for now */
+	if (ss->pre_destroy)
+		ss->pre_destroy(dummytop);
+
 	mutex_lock(&cgroup_mutex);
+
+	ss->active = 0;
+
+	if (ss->use_id) {
+		idr_remove_all(&ss->idr);
+		idr_destroy(&ss->idr);
+	}
+
 	/* deassign the subsys_id */
 	subsys[ss->subsys_id] = NULL;
 
-- 
1.7.11.7

[PATCH 11/17] cgroup: separate out cgroup_destroy_locked()

[Tejun Heo]

Separate out cgroup_destroy_locked() from cgroup_destroy().  This will
be later used in cgroup_create() failure path.

While at it, add lockdep asserts on i_mutex and cgroup_mutex, and move
@d and @parent assignments to their declarations.

This patch doesn't introduce any functional difference.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index c5368c0..afc47d6 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -242,6 +242,8 @@ static DEFINE_SPINLOCK(hierarchy_id_lock);
  */
 static int need_forkexit_callback __read_mostly;
 
+static int cgroup_destroy_locked(struct cgroup *cgrp);
+
 #ifdef CONFIG_PROVE_LOCKING
 int cgroup_lock_is_held(void)
 {
@@ -4209,22 +4211,20 @@ static int cgroup_has_css_refs(struct cgroup *cgrp)
 	return 0;
 }
 
-static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
+static int cgroup_destroy_locked(struct cgroup *cgrp)
+	__releases(&cgroup_mutex) __acquires(&cgroup_mutex)
 {
-	struct cgroup *cgrp = dentry->d_fsdata;
-	struct dentry *d;
-	struct cgroup *parent;
+	struct dentry *d = cgrp->dentry;
+	struct cgroup *parent = cgrp->parent;
 	DEFINE_WAIT(wait);
 	struct cgroup_event *event, *tmp;
 	struct cgroup_subsys *ss;
 
-	/* the vfs holds both inode->i_mutex already */
-	mutex_lock(&cgroup_mutex);
-	parent = cgrp->parent;
-	if (atomic_read(&cgrp->count) || !list_empty(&cgrp->children)) {
-		mutex_unlock(&cgroup_mutex);
+	lockdep_assert_held(&d->d_inode->i_mutex);
+	lockdep_assert_held(&cgroup_mutex);
+
+	if (atomic_read(&cgrp->count) || !list_empty(&cgrp->children))
 		return -EBUSY;
-	}
 
 	/*
 	 * Block new css_tryget() by deactivating refcnt and mark @cgrp
@@ -4243,7 +4243,9 @@ static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
 	/*
 	 * Tell subsystems to initate destruction.  pre_destroy() should be
 	 * called with cgroup_mutex unlocked.  See 3fa59dfbc3 ("cgroup: fix
-	 * potential deadlock in pre_destroy") for details.
+	 * potential deadlock in pre_destroy") for details.  This temporary
+	 * unlocking should go away once cgroup_mutex is unexported from
+	 * controllers.
 	 */
 	mutex_unlock(&cgroup_mutex);
 	for_each_subsys(cgrp->root, ss)
@@ -4268,11 +4270,9 @@ static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
 
 	/* delete this cgroup from parent->children */
 	list_del_rcu(&cgrp->sibling);
-
 	list_del_init(&cgrp->allcg_node);
 
-	d = dget(cgrp->dentry);
-
+	dget(d);
 	cgroup_d_remove_dir(d);
 	dput(d);
 
@@ -4293,10 +4293,20 @@ static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
 	}
 	spin_unlock(&cgrp->event_list_lock);
 
-	mutex_unlock(&cgroup_mutex);
 	return 0;
 }
 
+static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
+{
+	int ret;
+
+	mutex_lock(&cgroup_mutex);
+	ret = cgroup_destroy_locked(dentry->d_fsdata);
+	mutex_unlock(&cgroup_mutex);
+
+	return ret;
+}
+
 static void __init_or_module cgroup_init_cftsets(struct cgroup_subsys *ss)
 {
 	INIT_LIST_HEAD(&ss->cftsets);
-- 
1.7.11.7

[PATCH 12/17] cgroup: introduce CSS_ONLINE flag and on/offline_css() helpers

[Tejun Heo]

New helpers on/offline_css() respectively wrap ->post_create() and
->pre_destroy() invocations.  online_css() sets CSS_ONLINE after
->post_create() is complete and offline_css() invokes ->pre_destroy()
iff CSS_ONLINE is set and clears it while also handling the temporary
dropping of cgroup_mutex.

This patch doesn't introduce any behavior change at the moment but
will be used to improve cgroup_create() failure path and allow
->post_create() to fail.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/linux/cgroup.h |  1 +
 kernel/cgroup.c        | 65 ++++++++++++++++++++++++++++++++------------------
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index e9f07ef..c487315 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -82,6 +82,7 @@ struct cgroup_subsys_state {
 /* bits in struct cgroup_subsys_state flags field */
 enum {
 	CSS_ROOT	= (1 << 0), /* this CSS is the root of the subsystem */
+	CSS_ONLINE	= (1 << 1), /* between ->post_create() and ->pre_destroy() */
 };
 
 /* Caller must verify that the css is not for root cgroup */
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index afc47d6..c3087f5 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4035,6 +4035,42 @@ static void init_cgroup_css(struct cgroup_subsys_state *css,
 	INIT_WORK(&css->dput_work, css_dput_fn);
 }
 
+/* invoke ->post_create() on a new CSS and mark it online */
+static void online_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
+{
+	lockdep_assert_held(&cgroup_mutex);
+
+	if (ss->post_create)
+		ss->post_create(cgrp);
+	cgrp->subsys[ss->subsys_id]->flags |= CSS_ONLINE;
+}
+
+/* if the CSS is online, invoke ->pre_destory() on it and mark it offline */
+static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
+	__releases(&cgroup_mutex) __acquires(&cgroup_mutex)
+{
+	struct cgroup_subsys_state *css = cgrp->subsys[ss->subsys_id];
+
+	lockdep_assert_held(&cgroup_mutex);
+
+	if (!(css->flags & CSS_ONLINE))
+		return;
+
+	/*
+	 * pre_destroy() should be called with cgroup_mutex unlocked.  See
+	 * 3fa59dfbc3 ("cgroup: fix potential deadlock in pre_destroy") for
+	 * details.  This temporary unlocking should go away once
+	 * cgroup_mutex is unexported from controllers.
+	 */
+	if (ss->pre_destroy) {
+		mutex_unlock(&cgroup_mutex);
+		ss->pre_destroy(cgrp);
+		mutex_lock(&cgroup_mutex);
+	}
+
+	cgrp->subsys[ss->subsys_id]->flags &= ~CSS_ONLINE;
+}
+
 /*
  * cgroup_create - create a cgroup
  * @parent: cgroup that will be parent of the new cgroup
@@ -4137,8 +4173,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		dget(dentry);
 
 		/* creation succeeded, notify subsystems */
-		if (ss->post_create)
-			ss->post_create(cgrp);
+		online_css(ss, cgrp);
 	}
 
 	err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
@@ -4240,18 +4275,9 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
 	}
 	set_bit(CGRP_REMOVED, &cgrp->flags);
 
-	/*
-	 * Tell subsystems to initate destruction.  pre_destroy() should be
-	 * called with cgroup_mutex unlocked.  See 3fa59dfbc3 ("cgroup: fix
-	 * potential deadlock in pre_destroy") for details.  This temporary
-	 * unlocking should go away once cgroup_mutex is unexported from
-	 * controllers.
-	 */
-	mutex_unlock(&cgroup_mutex);
+	/* tell subsystems to initate destruction */
 	for_each_subsys(cgrp->root, ss)
-		if (ss->pre_destroy)
-			ss->pre_destroy(cgrp);
-	mutex_lock(&cgroup_mutex);
+		offline_css(ss, cgrp);
 
 	/*
 	 * Put all the base refs.  Each css holds an extra reference to the
@@ -4354,9 +4380,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	BUG_ON(!list_empty(&init_task.tasks));
 
 	ss->active = 1;
-
-	if (ss->post_create)
-		ss->post_create(dummytop);
+	online_css(ss, dummytop);
 
 	mutex_unlock(&cgroup_mutex);
 
@@ -4469,9 +4493,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	write_unlock(&css_set_lock);
 
 	ss->active = 1;
-
-	if (ss->post_create)
-		ss->post_create(dummytop);
+	online_css(ss, dummytop);
 
 	/* success! */
 	mutex_unlock(&cgroup_mutex);
@@ -4501,12 +4523,9 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
 	 */
 	BUG_ON(ss->root != &rootnode);
 
-	/* ->pre_destroy() should be called outside cgroup_mutex for now */
-	if (ss->pre_destroy)
-		ss->pre_destroy(dummytop);
-
 	mutex_lock(&cgroup_mutex);
 
+	offline_css(ss, dummytop);
 	ss->active = 0;
 
 	if (ss->use_id) {
-- 
1.7.11.7

[PATCH 13/17] cgroup: simplify cgroup_load_subsys() failure path

[Tejun Heo]

Now that cgroup_unload_subsys() can tell whether the root css is
online or not, we can safely call cgroup_unload_subsys() after idr
init failure in cgroup_load_subsys().

Replace the manual unrolling and invoke cgroup_unload_subsys() on
failure.  This drops cgroup_mutex inbetween but should be safe as the
subsystem will fail try_module_get() and thus can't be mounted
inbetween.  As this means that cgroup_unload_subsys() can be called
before css_sets are rehashed, remove BUG_ON() on %NULL
css_set->subsys[] from cgroup_unload_subsys().

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index c3087f5..b42f63f 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4400,8 +4400,8 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
  */
 int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 {
-	int i;
 	struct cgroup_subsys_state *css;
+	int i, ret;
 
 	/* check name and function validity */
 	if (ss->name == NULL || strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN ||
@@ -4452,15 +4452,9 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	init_cgroup_css(css, ss, dummytop);
 	/* init_idr must be after init_cgroup_css because it sets css->id. */
 	if (ss->use_id) {
-		int ret = cgroup_init_idr(ss, css);
-		if (ret) {
-			ss->destroy(dummytop);
-			dummytop->subsys[ss->subsys_id] = NULL;
-			subsys[ss->subsys_id] = NULL;
-			list_del_init(&ss->sibling);
-			mutex_unlock(&cgroup_mutex);
-			return ret;
-		}
+		ret = cgroup_init_idr(ss, css);
+		if (ret)
+			goto err_unload;
 	}
 
 	/*
@@ -4498,6 +4492,12 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	/* success! */
 	mutex_unlock(&cgroup_mutex);
 	return 0;
+
+err_unload:
+	mutex_unlock(&cgroup_mutex);
+	/* @ss can't be mounted here as try_module_get() would fail */
+	cgroup_unload_subsys(ss);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(cgroup_load_subsys);
 
@@ -4548,7 +4548,6 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
 		struct css_set *cg = link->cg;
 
 		hlist_del(&cg->hlist);
-		BUG_ON(!cg->subsys[ss->subsys_id]);
 		cg->subsys[ss->subsys_id] = NULL;
 		hhead = css_set_hash(cg->subsys);
 		hlist_add_head(&cg->hlist, hhead);
-- 
1.7.11.7

[PATCH 14/17] cgroup: use mutex_trylock() when grabbing i_mutex of a new cgroup directory

[Tejun Heo]

All cgroup directory i_mutexes nest outside cgroup_mutex; however, new
directory creation is a special case.  A new cgroup directory is
created while holding cgroup_mutex.  Populating the new directory
requires both the new directory's i_mutex and cgroup_mutex.  Because
all directory i_mutexes nest outside cgroup_mutex, grabbing both
requires releasing cgroup_mutex first, which isn't a good idea as the
new cgroup isn't yet ready to be manipulated by other cgroup
opreations.

This is worked around by grabbing the new directory's i_mutex while
holding cgroup_mutex before making it visible.  As there's no other
user at that point, grabbing the i_mutex under cgroup_mutex can't lead
to deadlock.

cgroup_create_file() was using I_MUTEX_CHILD to tell lockdep not to
worry about the reverse locking order; however, this creates pseudo
locking dependency cgroup_mutex -> I_MUTEX_CHILD, which isn't true -
all directory i_mutexes are still nested outside cgroup_mutex.  This
pseudo locking dependency can lead to spurious lockdep warnings.

Use mutex_trylock() instead.  This will always succeed and lockdep
doesn't create any locking dependency for it.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b42f63f..8ad5e76 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2657,9 +2657,15 @@ static int cgroup_create_file(struct dentry *dentry, umode_t mode,
 		inc_nlink(inode);
 		inc_nlink(dentry->d_parent->d_inode);
 
-		/* start with the directory inode held, so that we can
-		 * populate it without racing with another mkdir */
-		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
+		/*
+		 * Control reaches here with cgroup_mutex held.
+		 * @inode->i_mutex should nest outside cgroup_mutex but we
+		 * want to populate it immediately without releasing
+		 * cgroup_mutex.  As @inode isn't visible to anyone else
+		 * yet, trylock will always succeed without affecting
+		 * lockdep checks.
+		 */
+		WARN_ON_ONCE(!mutex_trylock(&inode->i_mutex));
 	} else if (S_ISREG(mode)) {
 		inode->i_size = 0;
 		inode->i_fop = &cgroup_file_operations;
-- 
1.7.11.7

[PATCH 15/17] cgroup: update cgroup_create() failure path

[Tejun Heo]

cgroup_create() was ignoring failure of cgroupfs files.  Update it
such that, if file creation fails, it rolls back by calling
cgroup_destroy_locked() and returns failure.

Note that error out goto labels are renamed.  The labels are a bit
confusing but will become better w/ later cgroup operation renames.

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

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 8ad5e76..63d83d0 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4107,7 +4107,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	 */
 	if (!cgroup_lock_live_group(parent)) {
 		err = -ENODEV;
-		goto err_free;
+		goto err_free_cgrp;
 	}
 
 	/* Grab a reference on the superblock so the hierarchy doesn't
@@ -4135,13 +4135,13 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 		css = ss->create(cgrp);
 		if (IS_ERR(css)) {
 			err = PTR_ERR(css);
-			goto err_destroy;
+			goto err_free_all;
 		}
 		init_cgroup_css(css, ss, cgrp);
 		if (ss->use_id) {
 			err = alloc_css_id(ss, parent, cgrp);
 			if (err)
-				goto err_destroy;
+				goto err_free_all;
 		}
 		/* At error, ->destroy() callback has to free assigned ID. */
 		if (clone_children(parent) && ss->post_clone)
@@ -4164,7 +4164,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	 */
 	err = cgroup_create_file(dentry, S_IFDIR | mode, sb);
 	if (err < 0)
-		goto err_destroy;
+		goto err_free_all;
 	lockdep_assert_held(&dentry->d_inode->i_mutex);
 
 	/* allocation complete, commit to creation */
@@ -4183,14 +4183,15 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	}
 
 	err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
-	/* If err < 0, we have a half-filled directory - oh well ;) */
+	if (err)
+		goto err_destroy;
 
 	mutex_unlock(&cgroup_mutex);
 	mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
 
 	return 0;
 
-err_destroy:
+err_free_all:
 	for_each_subsys(root, ss) {
 		if (cgrp->subsys[ss->subsys_id])
 			ss->destroy(cgrp);
@@ -4198,9 +4199,15 @@ err_destroy:
 	mutex_unlock(&cgroup_mutex);
 	/* Release the reference count that we took on the superblock */
 	deactivate_super(sb);
-err_free:
+err_free_cgrp:
 	kfree(cgrp);
 	return err;
+
+err_destroy:
+	cgroup_destroy_locked(cgrp);
+	mutex_unlock(&cgroup_mutex);
+	mutex_unlock(&dentry->d_inode->i_mutex);
+	return err;
 }
 
 static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
-- 
1.7.11.7

[PATCH 16/17] cgroup: allow ->post_create() to fail

[Tejun Heo]

There could be cases where controllers want to do initialization
operations which may fail from ->post_create().  This patch makes
->post_create() return -errno to indicate failure and online_css()
relay such failures.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
 include/linux/cgroup.h  |  2 +-
 kernel/cgroup.c         | 29 +++++++++++++++++++----------
 kernel/cgroup_freezer.c |  4 +++-
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c487315..fb79d7a 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -437,7 +437,7 @@ int cgroup_taskset_size(struct cgroup_taskset *tset);
 
 struct cgroup_subsys {
 	struct cgroup_subsys_state *(*create)(struct cgroup *cgrp);
-	void (*post_create)(struct cgroup *cgrp);
+	int (*post_create)(struct cgroup *cgrp);
 	void (*pre_destroy)(struct cgroup *cgrp);
 	void (*destroy)(struct cgroup *cgrp);
 	int (*can_attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 63d83d0..de97cf2 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4041,14 +4041,18 @@ static void init_cgroup_css(struct cgroup_subsys_state *css,
 	INIT_WORK(&css->dput_work, css_dput_fn);
 }
 
-/* invoke ->post_create() on a new CSS and mark it online */
-static void online_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
+/* invoke ->post_create() on a new CSS and mark it online if successful */
+static int online_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 {
+	int ret = 0;
+
 	lockdep_assert_held(&cgroup_mutex);
 
 	if (ss->post_create)
-		ss->post_create(cgrp);
-	cgrp->subsys[ss->subsys_id]->flags |= CSS_ONLINE;
+		ret = ss->post_create(cgrp);
+	if (!ret)
+		cgrp->subsys[ss->subsys_id]->flags |= CSS_ONLINE;
+	return ret;
 }
 
 /* if the CSS is online, invoke ->pre_destory() on it and mark it offline */
@@ -4174,12 +4178,15 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
 
-	for_each_subsys(root, ss) {
-		/* each css holds a ref to the cgroup's dentry */
+	/* each css holds a ref to the cgroup's dentry */
+	for_each_subsys(root, ss)
 		dget(dentry);
 
-		/* creation succeeded, notify subsystems */
-		online_css(ss, cgrp);
+	/* creation succeeded, notify subsystems */
+	for_each_subsys(root, ss) {
+		err = online_css(ss, cgrp);
+		if (err)
+			goto err_destroy;
 	}
 
 	err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
@@ -4393,7 +4400,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	BUG_ON(!list_empty(&init_task.tasks));
 
 	ss->active = 1;
-	online_css(ss, dummytop);
+	BUG_ON(online_css(ss, dummytop));
 
 	mutex_unlock(&cgroup_mutex);
 
@@ -4500,7 +4507,9 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	write_unlock(&css_set_lock);
 
 	ss->active = 1;
-	online_css(ss, dummytop);
+	ret = online_css(ss, dummytop);
+	if (ret)
+		goto err_unload;
 
 	/* success! */
 	mutex_unlock(&cgroup_mutex);
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 670a4af..ee8bb67 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -112,7 +112,7 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
  * parent's freezing state while holding both parent's and our
  * freezer->lock.
  */
-static void freezer_post_create(struct cgroup *cgroup)
+static int freezer_post_create(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
 	struct freezer *parent = parent_freezer(freezer);
@@ -136,6 +136,8 @@ static void freezer_post_create(struct cgroup *cgroup)
 	spin_unlock(&freezer->lock);
 	if (parent)
 		spin_unlock_irq(&parent->lock);
+
+	return 0;
 }
 
 /**
-- 
1.7.11.7

[PATCH 17/17] cgroup: rename ->create/post_create/pre_destroy/destroy() to ->css_alloc/online/offline/free()

[Tejun Heo]

Rename cgroup_subsys css lifetime related callbacks to better describe
what their roles are.  Also, update documentation.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/cgroups/cgroups.txt | 49 ++++++++++++++++++++++---------------
 block/blk-cgroup.c                | 14 +++++------
 include/linux/cgroup.h            | 35 ++++++++++++++-------------
 kernel/cgroup.c                   | 51 ++++++++++++++++++++-------------------
 kernel/cgroup_freezer.c           | 20 +++++++--------
 kernel/cpuset.c                   | 10 ++++----
 kernel/events/core.c              |  8 +++---
 kernel/sched/core.c               | 16 ++++++------
 mm/hugetlb_cgroup.c               | 14 +++++------
 mm/memcontrol.c                   | 12 ++++-----
 net/core/netprio_cgroup.c         |  8 +++---
 net/sched/cls_cgroup.c            |  8 +++---
 security/device_cgroup.c          |  8 +++---
 13 files changed, 132 insertions(+), 121 deletions(-)

diff --git a/Documentation/cgroups/cgroups.txt b/Documentation/cgroups/cgroups.txt
index 9e04196..b06eea2 100644
--- a/Documentation/cgroups/cgroups.txt
+++ b/Documentation/cgroups/cgroups.txt
@@ -553,16 +553,16 @@ call to cgroup_unload_subsys(). It should also set its_subsys.module =
 THIS_MODULE in its .c file.
 
 Each subsystem may export the following methods. The only mandatory
-methods are create/destroy. Any others that are null are presumed to
+methods are css_alloc/free. Any others that are null are presumed to
 be successful no-ops.
 
-struct cgroup_subsys_state *create(struct cgroup *cgrp)
+struct cgroup_subsys_state *css_alloc(struct cgroup *cgrp)
 (cgroup_mutex held by caller)
 
-Called to create a subsystem state object for a cgroup. The
+Called to allocate a subsystem state object for a cgroup. The
 subsystem should allocate its subsystem state object for the passed
 cgroup, returning a pointer to the new object on success or a
-negative error code. On success, the subsystem pointer should point to
+ERR_PTR() value. On success, the subsystem pointer should point to
 a structure of type cgroup_subsys_state (typically embedded in a
 larger subsystem-specific object), which will be initialized by the
 cgroup system. Note that this will be called at initialization to
@@ -571,24 +571,33 @@ identified by the passed cgroup object having a NULL parent (since
 it's the root of the hierarchy) and may be an appropriate place for
 initialization code.
 
-void destroy(struct cgroup *cgrp)
+int css_online(struct cgroup *cgrp)
 (cgroup_mutex held by caller)
 
-The cgroup system is about to destroy the passed cgroup; the subsystem
-should do any necessary cleanup and free its subsystem state
-object. By the time this method is called, the cgroup has already been
-unlinked from the file system and from the child list of its parent;
-cgroup->parent is still valid. (Note - can also be called for a
-newly-created cgroup if an error occurs after this subsystem's
-create() method has been called for the new cgroup).
-
-int pre_destroy(struct cgroup *cgrp);
-
-Called before checking the reference count on each subsystem. This may
-be useful for subsystems which have some extra references even if
-there are not tasks in the cgroup. If pre_destroy() returns error code,
-rmdir() will fail with it. From this behavior, pre_destroy() can be
-called multiple times against a cgroup.
+Called after @cgrp successfully completed all allocations and made
+visible to cgroup_for_each_child/descendant_*() iterators. The
+subsystem may choose to fail creation by returning -errno. This
+callback can be used to implement reliable state sharing and
+propagation along the hierarchy. See the comment on
+cgroup_for_each_descendant_pre() for details.
+
+void css_offline(struct cgroup *cgrp);
+
+This is the counterpart of css_online() and called iff css_online()
+has succeeded on @cgrp. This signifies the beginning of the end of
+@cgrp. @cgrp is being removed and the subsystem should start dropping
+all references it's holding on @cgrp. When all references are dropped,
+cgroup removal will proceed to the next step - css_free(). After this
+callback, @cgrp should be considered dead to the subsystem.
+
+void css_free(struct cgroup *cgrp)
+(cgroup_mutex held by caller)
+
+The cgroup system is about to free @cgrp; the subsystem should free
+its subsystem state object. By the time this method is called, @cgrp
+is completely unused; @cgrp->parent is still valid. (Note - can also
+be called for a newly-created cgroup if an error occurs after this
+subsystem's create() method has been called for the new cgroup).
 
 int can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
 (cgroup_mutex held by caller)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 3dc60fc..3f6d39d 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -600,7 +600,7 @@ struct cftype blkcg_files[] = {
 };
 
 /**
- * blkcg_pre_destroy - cgroup pre_destroy callback
+ * blkcg_css_offline - cgroup css_offline callback
  * @cgroup: cgroup of interest
  *
  * This function is called when @cgroup is about to go away and responsible
@@ -610,7 +610,7 @@ struct cftype blkcg_files[] = {
  *
  * This is the blkcg counterpart of ioc_release_fn().
  */
-static void blkcg_pre_destroy(struct cgroup *cgroup)
+static void blkcg_css_offline(struct cgroup *cgroup)
 {
 	struct blkcg *blkcg = cgroup_to_blkcg(cgroup);
 
@@ -634,7 +634,7 @@ static void blkcg_pre_destroy(struct cgroup *cgroup)
 	spin_unlock_irq(&blkcg->lock);
 }
 
-static void blkcg_destroy(struct cgroup *cgroup)
+static void blkcg_css_free(struct cgroup *cgroup)
 {
 	struct blkcg *blkcg = cgroup_to_blkcg(cgroup);
 
@@ -642,7 +642,7 @@ static void blkcg_destroy(struct cgroup *cgroup)
 		kfree(blkcg);
 }
 
-static struct cgroup_subsys_state *blkcg_create(struct cgroup *cgroup)
+static struct cgroup_subsys_state *blkcg_css_alloc(struct cgroup *cgroup)
 {
 	static atomic64_t id_seq = ATOMIC64_INIT(0);
 	struct blkcg *blkcg;
@@ -739,10 +739,10 @@ static int blkcg_can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
 
 struct cgroup_subsys blkio_subsys = {
 	.name = "blkio",
-	.create = blkcg_create,
+	.css_alloc = blkcg_css_alloc,
+	.css_offline = blkcg_css_offline,
+	.css_free = blkcg_css_free,
 	.can_attach = blkcg_can_attach,
-	.pre_destroy = blkcg_pre_destroy,
-	.destroy = blkcg_destroy,
 	.subsys_id = blkio_subsys_id,
 	.base_cftypes = blkcg_files,
 	.module = THIS_MODULE,
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index fb79d7a..89c631d 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -82,7 +82,7 @@ struct cgroup_subsys_state {
 /* bits in struct cgroup_subsys_state flags field */
 enum {
 	CSS_ROOT	= (1 << 0), /* this CSS is the root of the subsystem */
-	CSS_ONLINE	= (1 << 1), /* between ->post_create() and ->pre_destroy() */
+	CSS_ONLINE	= (1 << 1), /* between ->css_online() and ->css_offline() */
 };
 
 /* Caller must verify that the css is not for root cgroup */
@@ -436,10 +436,11 @@ int cgroup_taskset_size(struct cgroup_taskset *tset);
  */
 
 struct cgroup_subsys {
-	struct cgroup_subsys_state *(*create)(struct cgroup *cgrp);
-	int (*post_create)(struct cgroup *cgrp);
-	void (*pre_destroy)(struct cgroup *cgrp);
-	void (*destroy)(struct cgroup *cgrp);
+	struct cgroup_subsys_state *(*css_alloc)(struct cgroup *cgrp);
+	int (*css_online)(struct cgroup *cgrp);
+	void (*css_offline)(struct cgroup *cgrp);
+	void (*css_free)(struct cgroup *cgrp);
+
 	int (*can_attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
 	void (*cancel_attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
 	void (*attach)(struct cgroup *cgrp, struct cgroup_taskset *tset);
@@ -538,13 +539,13 @@ static inline struct cgroup* task_cgroup(struct task_struct *task,
  * @cgroup: cgroup whose children to walk
  *
  * Walk @cgroup's children.  Must be called under rcu_read_lock().  A child
- * cgroup which hasn't finished ->post_create() or already has finished
- * ->pre_destroy() may show up during traversal and it's each subsystem's
+ * cgroup which hasn't finished ->css_online() or already has finished
+ * ->css_offline() may show up during traversal and it's each subsystem's
  * responsibility to verify that each @pos is alive.
  *
- * If a subsystem synchronizes against the parent in its ->post_create()
- * and before starting iterating, a cgroup which finished ->post_create()
- * is guaranteed to be visible in the future iterations.
+ * If a subsystem synchronizes against the parent in its ->css_online() and
+ * before starting iterating, a cgroup which finished ->css_online() is
+ * guaranteed to be visible in the future iterations.
  */
 #define cgroup_for_each_child(pos, cgroup)				\
 	list_for_each_entry_rcu(pos, &(cgroup)->children, sibling)
@@ -558,19 +559,19 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos,
  * @cgroup: cgroup whose descendants to walk
  *
  * Walk @cgroup's descendants.  Must be called under rcu_read_lock().  A
- * descendant cgroup which hasn't finished ->post_create() or already has
- * finished ->pre_destroy() may show up during traversal and it's each
+ * descendant cgroup which hasn't finished ->css_online() or already has
+ * finished ->css_offline() may show up during traversal and it's each
  * subsystem's responsibility to verify that each @pos is alive.
  *
- * If a subsystem synchronizes against the parent in its ->post_create()
- * and before starting iterating, and synchronizes against @pos on each
- * iteration, any descendant cgroup which finished ->post_create() is
+ * If a subsystem synchronizes against the parent in its ->css_online() and
+ * before starting iterating, and synchronizes against @pos on each
+ * iteration, any descendant cgroup which finished ->css_offline() is
  * guaranteed to be visible in the future iterations.
  *
  * In other words, the following guarantees that a descendant can't escape
  * state updates of its ancestors.
  *
- * my_post_create(@cgrp)
+ * my_online(@cgrp)
  * {
  *	Lock @cgrp->parent and @cgrp;
  *	Inherit state from @cgrp->parent;
@@ -603,7 +604,7 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos,
  * iteration should lock and unlock both @pos->parent and @pos.
  *
  * Alternatively, a subsystem may choose to use a single global lock to
- * synchronize ->post_create() and ->pre_destroy() against tree-walking
+ * synchronize ->css_online() and ->css_offline() against tree-walking
  * operations.
  */
 #define cgroup_for_each_descendant_pre(pos, cgroup)			\
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index de97cf2..46c5119 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -876,7 +876,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
 		 * Release the subsystem state objects.
 		 */
 		for_each_subsys(cgrp->root, ss)
-			ss->destroy(cgrp);
+			ss->css_free(cgrp);
 
 		cgrp->root->number_of_cgroups--;
 		mutex_unlock(&cgroup_mutex);
@@ -4048,8 +4048,8 @@ static int online_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 
 	lockdep_assert_held(&cgroup_mutex);
 
-	if (ss->post_create)
-		ret = ss->post_create(cgrp);
+	if (ss->css_online)
+		ret = ss->css_online(cgrp);
 	if (!ret)
 		cgrp->subsys[ss->subsys_id]->flags |= CSS_ONLINE;
 	return ret;
@@ -4067,14 +4067,14 @@ static void offline_css(struct cgroup_subsys *ss, struct cgroup *cgrp)
 		return;
 
 	/*
-	 * pre_destroy() should be called with cgroup_mutex unlocked.  See
+	 * css_offline() should be called with cgroup_mutex unlocked.  See
 	 * 3fa59dfbc3 ("cgroup: fix potential deadlock in pre_destroy") for
 	 * details.  This temporary unlocking should go away once
 	 * cgroup_mutex is unexported from controllers.
 	 */
-	if (ss->pre_destroy) {
+	if (ss->css_offline) {
 		mutex_unlock(&cgroup_mutex);
-		ss->pre_destroy(cgrp);
+		ss->css_offline(cgrp);
 		mutex_lock(&cgroup_mutex);
 	}
 
@@ -4136,7 +4136,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 	for_each_subsys(root, ss) {
 		struct cgroup_subsys_state *css;
 
-		css = ss->create(cgrp);
+		css = ss->css_alloc(cgrp);
 		if (IS_ERR(css)) {
 			err = PTR_ERR(css);
 			goto err_free_all;
@@ -4147,7 +4147,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 			if (err)
 				goto err_free_all;
 		}
-		/* At error, ->destroy() callback has to free assigned ID. */
+		/* At error, ->css_free() callback has to free assigned ID. */
 		if (clone_children(parent) && ss->post_clone)
 			ss->post_clone(cgrp);
 
@@ -4201,7 +4201,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 err_free_all:
 	for_each_subsys(root, ss) {
 		if (cgrp->subsys[ss->subsys_id])
-			ss->destroy(cgrp);
+			ss->css_free(cgrp);
 	}
 	mutex_unlock(&cgroup_mutex);
 	/* Release the reference count that we took on the superblock */
@@ -4381,7 +4381,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	/* Create the top cgroup state for this subsystem */
 	list_add(&ss->sibling, &rootnode.subsys_list);
 	ss->root = &rootnode;
-	css = ss->create(dummytop);
+	css = ss->css_alloc(dummytop);
 	/* We don't handle early failures gracefully */
 	BUG_ON(IS_ERR(css));
 	init_cgroup_css(css, ss, dummytop);
@@ -4425,7 +4425,7 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 
 	/* check name and function validity */
 	if (ss->name == NULL || strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN ||
-	    ss->create == NULL || ss->destroy == NULL)
+	    ss->css_alloc == NULL || ss->css_free == NULL)
 		return -EINVAL;
 
 	/*
@@ -4454,10 +4454,11 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	subsys[ss->subsys_id] = ss;
 
 	/*
-	 * no ss->create seems to need anything important in the ss struct, so
-	 * this can happen first (i.e. before the rootnode attachment).
+	 * no ss->css_alloc seems to need anything important in the ss
+	 * struct, so this can happen first (i.e. before the rootnode
+	 * attachment).
 	 */
-	css = ss->create(dummytop);
+	css = ss->css_alloc(dummytop);
 	if (IS_ERR(css)) {
 		/* failure case - need to deassign the subsys[] slot. */
 		subsys[ss->subsys_id] = NULL;
@@ -4577,12 +4578,12 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
 	write_unlock(&css_set_lock);
 
 	/*
-	 * remove subsystem's css from the dummytop and free it - need to free
-	 * before marking as null because ss->destroy needs the cgrp->subsys
-	 * pointer to find their state. note that this also takes care of
-	 * freeing the css_id.
+	 * remove subsystem's css from the dummytop and free it - need to
+	 * free before marking as null because ss->css_free needs the
+	 * cgrp->subsys pointer to find their state. note that this also
+	 * takes care of freeing the css_id.
 	 */
-	ss->destroy(dummytop);
+	ss->css_free(dummytop);
 	dummytop->subsys[ss->subsys_id] = NULL;
 
 	mutex_unlock(&cgroup_mutex);
@@ -4626,8 +4627,8 @@ int __init cgroup_init_early(void)
 
 		BUG_ON(!ss->name);
 		BUG_ON(strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN);
-		BUG_ON(!ss->create);
-		BUG_ON(!ss->destroy);
+		BUG_ON(!ss->css_alloc);
+		BUG_ON(!ss->css_free);
 		if (ss->subsys_id != i) {
 			printk(KERN_ERR "cgroup: Subsys %s id == %d\n",
 			       ss->name, ss->subsys_id);
@@ -5439,7 +5440,7 @@ struct cgroup_subsys_state *cgroup_css_from_dir(struct file *f, int id)
 }
 
 #ifdef CONFIG_CGROUP_DEBUG
-static struct cgroup_subsys_state *debug_create(struct cgroup *cont)
+static struct cgroup_subsys_state *debug_css_alloc(struct cgroup *cont)
 {
 	struct cgroup_subsys_state *css = kzalloc(sizeof(*css), GFP_KERNEL);
 
@@ -5449,7 +5450,7 @@ static struct cgroup_subsys_state *debug_create(struct cgroup *cont)
 	return css;
 }
 
-static void debug_destroy(struct cgroup *cont)
+static void debug_css_free(struct cgroup *cont)
 {
 	kfree(cont->subsys[debug_subsys_id]);
 }
@@ -5578,8 +5579,8 @@ static struct cftype debug_files[] =  {
 
 struct cgroup_subsys debug_subsys = {
 	.name = "debug",
-	.create = debug_create,
-	.destroy = debug_destroy,
+	.css_alloc = debug_css_alloc,
+	.css_free = debug_css_free,
 	.subsys_id = debug_subsys_id,
 	.base_cftypes = debug_files,
 };
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index ee8bb67..75dda1e 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -92,7 +92,7 @@ static const char *freezer_state_strs(unsigned int state)
 
 struct cgroup_subsys freezer_subsys;
 
-static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
+static struct cgroup_subsys_state *freezer_css_alloc(struct cgroup *cgroup)
 {
 	struct freezer *freezer;
 
@@ -105,14 +105,14 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup *cgroup)
 }
 
 /**
- * freezer_post_create - commit creation of a freezer cgroup
+ * freezer_css_online - commit creation of a freezer cgroup
  * @cgroup: cgroup being created
  *
  * We're committing to creation of @cgroup.  Mark it online and inherit
  * parent's freezing state while holding both parent's and our
  * freezer->lock.
  */
-static int freezer_post_create(struct cgroup *cgroup)
+static int freezer_css_online(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
 	struct freezer *parent = parent_freezer(freezer);
@@ -141,13 +141,13 @@ static int freezer_post_create(struct cgroup *cgroup)
 }
 
 /**
- * freezer_pre_destroy - initiate destruction of @cgroup
+ * freezer_css_offline - initiate destruction of @cgroup
  * @cgroup: cgroup being destroyed
  *
  * @cgroup is going away.  Mark it dead and decrement system_freezing_count
  * if it was holding one.
  */
-static void freezer_pre_destroy(struct cgroup *cgroup)
+static void freezer_css_offline(struct cgroup *cgroup)
 {
 	struct freezer *freezer = cgroup_freezer(cgroup);
 
@@ -161,7 +161,7 @@ static void freezer_pre_destroy(struct cgroup *cgroup)
 	spin_unlock_irq(&freezer->lock);
 }
 
-static void freezer_destroy(struct cgroup *cgroup)
+static void freezer_css_free(struct cgroup *cgroup)
 {
 	kfree(cgroup_freezer(cgroup));
 }
@@ -477,10 +477,10 @@ static struct cftype files[] = {
 
 struct cgroup_subsys freezer_subsys = {
 	.name		= "freezer",
-	.create		= freezer_create,
-	.post_create	= freezer_post_create,
-	.pre_destroy	= freezer_pre_destroy,
-	.destroy	= freezer_destroy,
+	.css_alloc	= freezer_css_alloc,
+	.css_online	= freezer_css_online,
+	.css_offline	= freezer_css_offline,
+	.css_free	= freezer_css_free,
 	.subsys_id	= freezer_subsys_id,
 	.attach		= freezer_attach,
 	.fork		= freezer_fork,
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index f33c715..0693133 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1821,11 +1821,11 @@ static void cpuset_post_clone(struct cgroup *cgroup)
 }
 
 /*
- *	cpuset_create - create a cpuset
+ *	cpuset_css_alloc - allocate a cpuset css
  *	cont:	control group that the new cpuset will be part of
  */
 
-static struct cgroup_subsys_state *cpuset_create(struct cgroup *cont)
+static struct cgroup_subsys_state *cpuset_css_alloc(struct cgroup *cont)
 {
 	struct cpuset *cs;
 	struct cpuset *parent;
@@ -1864,7 +1864,7 @@ static struct cgroup_subsys_state *cpuset_create(struct cgroup *cont)
  * will call async_rebuild_sched_domains().
  */
 
-static void cpuset_destroy(struct cgroup *cont)
+static void cpuset_css_free(struct cgroup *cont)
 {
 	struct cpuset *cs = cgroup_cs(cont);
 
@@ -1878,8 +1878,8 @@ static void cpuset_destroy(struct cgroup *cont)
 
 struct cgroup_subsys cpuset_subsys = {
 	.name = "cpuset",
-	.create = cpuset_create,
-	.destroy = cpuset_destroy,
+	.css_alloc = cpuset_css_alloc,
+	.css_free = cpuset_css_free,
 	.can_attach = cpuset_can_attach,
 	.attach = cpuset_attach,
 	.post_clone = cpuset_post_clone,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index dbccf83..f9ff549 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7434,7 +7434,7 @@ unlock:
 device_initcall(perf_event_sysfs_init);
 
 #ifdef CONFIG_CGROUP_PERF
-static struct cgroup_subsys_state *perf_cgroup_create(struct cgroup *cont)
+static struct cgroup_subsys_state *perf_cgroup_css_alloc(struct cgroup *cont)
 {
 	struct perf_cgroup *jc;
 
@@ -7451,7 +7451,7 @@ static struct cgroup_subsys_state *perf_cgroup_create(struct cgroup *cont)
 	return &jc->css;
 }
 
-static void perf_cgroup_destroy(struct cgroup *cont)
+static void perf_cgroup_css_free(struct cgroup *cont)
 {
 	struct perf_cgroup *jc;
 	jc = container_of(cgroup_subsys_state(cont, perf_subsys_id),
@@ -7492,8 +7492,8 @@ static void perf_cgroup_exit(struct cgroup *cgrp, struct cgroup *old_cgrp,
 struct cgroup_subsys perf_subsys = {
 	.name		= "perf_event",
 	.subsys_id	= perf_subsys_id,
-	.create		= perf_cgroup_create,
-	.destroy	= perf_cgroup_destroy,
+	.css_alloc	= perf_cgroup_css_alloc,
+	.css_free	= perf_cgroup_css_free,
 	.exit		= perf_cgroup_exit,
 	.attach		= perf_cgroup_attach,
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2d8927f..6f20c8f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7468,7 +7468,7 @@ static inline struct task_group *cgroup_tg(struct cgroup *cgrp)
 			    struct task_group, css);
 }
 
-static struct cgroup_subsys_state *cpu_cgroup_create(struct cgroup *cgrp)
+static struct cgroup_subsys_state *cpu_cgroup_css_alloc(struct cgroup *cgrp)
 {
 	struct task_group *tg, *parent;
 
@@ -7485,7 +7485,7 @@ static struct cgroup_subsys_state *cpu_cgroup_create(struct cgroup *cgrp)
 	return &tg->css;
 }
 
-static void cpu_cgroup_destroy(struct cgroup *cgrp)
+static void cpu_cgroup_css_free(struct cgroup *cgrp)
 {
 	struct task_group *tg = cgroup_tg(cgrp);
 
@@ -7845,8 +7845,8 @@ static struct cftype cpu_files[] = {
 
 struct cgroup_subsys cpu_cgroup_subsys = {
 	.name		= "cpu",
-	.create		= cpu_cgroup_create,
-	.destroy	= cpu_cgroup_destroy,
+	.css_alloc	= cpu_cgroup_css_alloc,
+	.css_free	= cpu_cgroup_css_free,
 	.can_attach	= cpu_cgroup_can_attach,
 	.attach		= cpu_cgroup_attach,
 	.exit		= cpu_cgroup_exit,
@@ -7869,7 +7869,7 @@ struct cgroup_subsys cpu_cgroup_subsys = {
 struct cpuacct root_cpuacct;
 
 /* create a new cpu accounting group */
-static struct cgroup_subsys_state *cpuacct_create(struct cgroup *cgrp)
+static struct cgroup_subsys_state *cpuacct_css_alloc(struct cgroup *cgrp)
 {
 	struct cpuacct *ca;
 
@@ -7899,7 +7899,7 @@ out:
 }
 
 /* destroy an existing cpu accounting group */
-static void cpuacct_destroy(struct cgroup *cgrp)
+static void cpuacct_css_free(struct cgroup *cgrp)
 {
 	struct cpuacct *ca = cgroup_ca(cgrp);
 
@@ -8070,8 +8070,8 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 
 struct cgroup_subsys cpuacct_subsys = {
 	.name = "cpuacct",
-	.create = cpuacct_create,
-	.destroy = cpuacct_destroy,
+	.css_alloc = cpuacct_css_alloc,
+	.css_free = cpuacct_css_free,
 	.subsys_id = cpuacct_subsys_id,
 	.base_cftypes = files,
 };
diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index 0d3a1a3..b5bde7a 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -77,7 +77,7 @@ static inline bool hugetlb_cgroup_have_usage(struct cgroup *cg)
 	return false;
 }
 
-static struct cgroup_subsys_state *hugetlb_cgroup_create(struct cgroup *cgroup)
+static struct cgroup_subsys_state *hugetlb_cgroup_css_alloc(struct cgroup *cgroup)
 {
 	int idx;
 	struct cgroup *parent_cgroup;
@@ -101,7 +101,7 @@ static struct cgroup_subsys_state *hugetlb_cgroup_create(struct cgroup *cgroup)
 	return &h_cgroup->css;
 }
 
-static void hugetlb_cgroup_destroy(struct cgroup *cgroup)
+static void hugetlb_cgroup_css_free(struct cgroup *cgroup)
 {
 	struct hugetlb_cgroup *h_cgroup;
 
@@ -155,7 +155,7 @@ out:
  * Force the hugetlb cgroup to empty the hugetlb resources by moving them to
  * the parent cgroup.
  */
-static void hugetlb_cgroup_pre_destroy(struct cgroup *cgroup)
+static void hugetlb_cgroup_css_offline(struct cgroup *cgroup)
 {
 	struct hstate *h;
 	struct page *page;
@@ -404,8 +404,8 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 
 struct cgroup_subsys hugetlb_subsys = {
 	.name = "hugetlb",
-	.create     = hugetlb_cgroup_create,
-	.pre_destroy = hugetlb_cgroup_pre_destroy,
-	.destroy    = hugetlb_cgroup_destroy,
-	.subsys_id  = hugetlb_subsys_id,
+	.css_alloc	= hugetlb_cgroup_css_alloc,
+	.css_offline	= hugetlb_cgroup_css_offline,
+	.css_free	= hugetlb_cgroup_css_free,
+	.subsys_id	= hugetlb_subsys_id,
 };
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 08adaaa..8b0b2b0 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4922,7 +4922,7 @@ err_cleanup:
 }
 
 static struct cgroup_subsys_state * __ref
-mem_cgroup_create(struct cgroup *cont)
+mem_cgroup_css_alloc(struct cgroup *cont)
 {
 	struct mem_cgroup *memcg, *parent;
 	long error = -ENOMEM;
@@ -5003,14 +5003,14 @@ free_out:
 	return ERR_PTR(error);
 }
 
-static void mem_cgroup_pre_destroy(struct cgroup *cont)
+static void mem_cgroup_css_offline(struct cgroup *cont)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
 
 	mem_cgroup_reparent_charges(memcg);
 }
 
-static void mem_cgroup_destroy(struct cgroup *cont)
+static void mem_cgroup_css_free(struct cgroup *cont)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
 
@@ -5600,9 +5600,9 @@ static void mem_cgroup_move_task(struct cgroup *cont,
 struct cgroup_subsys mem_cgroup_subsys = {
 	.name = "memory",
 	.subsys_id = mem_cgroup_subsys_id,
-	.create = mem_cgroup_create,
-	.pre_destroy = mem_cgroup_pre_destroy,
-	.destroy = mem_cgroup_destroy,
+	.css_alloc = mem_cgroup_css_alloc,
+	.css_offline = mem_cgroup_css_offline,
+	.css_free = mem_cgroup_css_free,
 	.can_attach = mem_cgroup_can_attach,
 	.cancel_attach = mem_cgroup_cancel_attach,
 	.attach = mem_cgroup_move_task,
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 79285a3..f0b6b0d 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -108,7 +108,7 @@ static int write_update_netdev_table(struct net_device *dev)
 	return ret;
 }
 
-static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
+static struct cgroup_subsys_state *cgrp_css_alloc(struct cgroup *cgrp)
 {
 	struct cgroup_netprio_state *cs;
 	int ret = -EINVAL;
@@ -132,7 +132,7 @@ out:
 	return ERR_PTR(ret);
 }
 
-static void cgrp_destroy(struct cgroup *cgrp)
+static void cgrp_css_free(struct cgroup *cgrp)
 {
 	struct cgroup_netprio_state *cs;
 	struct net_device *dev;
@@ -276,8 +276,8 @@ static struct cftype ss_files[] = {
 
 struct cgroup_subsys net_prio_subsys = {
 	.name		= "net_prio",
-	.create		= cgrp_create,
-	.destroy	= cgrp_destroy,
+	.css_alloc	= cgrp_css_alloc,
+	.css_free	= cgrp_css_free,
 	.attach		= net_prio_attach,
 	.subsys_id	= net_prio_subsys_id,
 	.base_cftypes	= ss_files,
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 2ecde22..8cdc18e 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -34,7 +34,7 @@ static inline struct cgroup_cls_state *task_cls_state(struct task_struct *p)
 			    struct cgroup_cls_state, css);
 }
 
-static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
+static struct cgroup_subsys_state *cgrp_css_alloc(struct cgroup *cgrp)
 {
 	struct cgroup_cls_state *cs;
 
@@ -48,7 +48,7 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
 	return &cs->css;
 }
 
-static void cgrp_destroy(struct cgroup *cgrp)
+static void cgrp_css_free(struct cgroup *cgrp)
 {
 	kfree(cgrp_cls_state(cgrp));
 }
@@ -75,8 +75,8 @@ static struct cftype ss_files[] = {
 
 struct cgroup_subsys net_cls_subsys = {
 	.name		= "net_cls",
-	.create		= cgrp_create,
-	.destroy	= cgrp_destroy,
+	.css_alloc	= cgrp_css_alloc,
+	.css_free	= cgrp_css_free,
 	.subsys_id	= net_cls_subsys_id,
 	.base_cftypes	= ss_files,
 	.module		= THIS_MODULE,
diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index 78a16f5..19ecc8d 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -180,7 +180,7 @@ static void dev_exception_clean(struct dev_cgroup *dev_cgroup)
 /*
  * called from kernel/cgroup.c with cgroup_lock() held.
  */
-static struct cgroup_subsys_state *devcgroup_create(struct cgroup *cgroup)
+static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup)
 {
 	struct dev_cgroup *dev_cgroup, *parent_dev_cgroup;
 	struct cgroup *parent_cgroup;
@@ -210,7 +210,7 @@ static struct cgroup_subsys_state *devcgroup_create(struct cgroup *cgroup)
 	return &dev_cgroup->css;
 }
 
-static void devcgroup_destroy(struct cgroup *cgroup)
+static void devcgroup_css_free(struct cgroup *cgroup)
 {
 	struct dev_cgroup *dev_cgroup;
 
@@ -564,8 +564,8 @@ static struct cftype dev_cgroup_files[] = {
 struct cgroup_subsys devices_subsys = {
 	.name = "devices",
 	.can_attach = devcgroup_can_attach,
-	.create = devcgroup_create,
-	.destroy = devcgroup_destroy,
+	.css_alloc = devcgroup_css_alloc,
+	.css_free = devcgroup_css_free,
 	.subsys_id = devices_subsys_id,
 	.base_cftypes = dev_cgroup_files,
 
-- 
1.7.11.7

Re: [PATCHSET cgroup/for-3.8] cgroup: allow ->post_create() to fail

[Li Zefan]

On 2012/11/13 11:01, Tejun Heo wrote:
> Hello,
> 
> This became a rather large patchset mostly because there are a lot of
> weirdities and subtle breakages in create/destroy/load/unload paths.
> The goal of the patchset is allowing ->post_create() to fail and
> renaming ->create/post_create/pre_destroy/destroy() to
> ->css_alloc/css_online/css_offline/css_free().
> 
> This patchset contains the following seventeen patches.
> 
>  0001-cgroup-remove-incorrect-dget-dput-pair-in-cgroup_cre.patch
>  0002-cgroup-initialize-cgrp-allcg_node-in-init_cgroup_hou.patch
>  0003-cgroup-open-code-cgroup_create_dir.patch
>  0004-cgroup-create-directory-before-linking-while-creatin.patch
>  0005-cgroup-cgroup-dentry-isn-t-a-RCU-pointer.patch
>  0006-cgroup-remove-duplicate-RCU-free-on-struct-cgroup.patch
>  0007-cgroup-make-CSS_-flags-bit-masks-instead-of-bit-posi.patch
>  0008-cgroup-trivial-cleanup-for-cgroup_init-load_subsys.patch
>  0009-cgroup-lock-cgroup_mutex-in-cgroup_init_subsys.patch
>  0010-cgroup-fix-harmless-bugs-in-cgroup_load_subsys-fail-.patch
>  0011-cgroup-separate-out-cgroup_destroy_locked.patch
>  0012-cgroup-introduce-CSS_ONLINE-flag-and-on-offline_css-.patch
>  0013-cgroup-simplify-cgroup_load_subsys-failure-path.patch
>  0014-cgroup-use-mutex_trylock-when-grabbing-i_mutex-of-a-.patch
>  0015-cgroup-update-cgroup_create-failure-path.patch
>  0016-cgroup-allow-post_create-to-fail.patch
>  0017-cgroup-rename-create-post_create-pre_destroy-destroy.patch
> 

Acked-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

except 0006-cgroup-remove-duplicate-RCU-free-on-struct-cgroup.patch

> 0001 is a bugfix.  Tagged for -stable.  I don't I'll push it through
> for-3.7-fixes tho.  This has been broken forever and nobody reported
> anything yet.
> 
> 0002-0011 are various fixes and cleanups to prepare for the following
> changes.  Each should be self-explanatory.
> 
> 0012-0015 are preparation patches more directly related to allowing
> failure of ->post_create().
> 
> 0016 allows ->post_create() to fail.
> 
> 0017 renames ops and updates documentation.
> 
> This patchst is on top of cgroup/for-3.8 and available on the
> following git branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-cgroup-online-may-fail

Re: [PATCHSET cgroup/for-3.8] cgroup: allow ->post_create() to fail

[Tejun Heo]

On Mon, Nov 19, 2012 at 04:54:11PM +0800, Li Zefan wrote:
> Acked-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> 
> except 0006-cgroup-remove-duplicate-RCU-free-on-struct-cgroup.patch

Patchset applied to cgroup/for-3.8 sans 0006.

Thanks.

-- 
tejun