[PATCH] cgroup: remove bind() method from cgroup_subsys.

[PATCH] cgroup: remove bind() method from cgroup_subsys.

[Rami Rosen]

The bind() method of cgroup_subsys is not used in any of the controllers
(cpuset, freezer, blkio, net_cls, memcg, net_prio, devices, perf, hugetlb, 
cpu and cpuacct)

Signed-off-by: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 include/linux/cgroup.h | 2 --
 kernel/cgroup.c        | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 515927e..92acf86 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -483,8 +483,6 @@ struct cgroup_subsys {
 	void (*fork)(struct task_struct *task);
 	void (*exit)(struct cgroup *cgrp, struct cgroup *old_cgrp,
 		     struct task_struct *task);
-	void (*bind)(struct cgroup *root);
-
 	int subsys_id;
 	int active;
 	int disabled;
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index ba3e24a..fd38e1c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1064,16 +1064,12 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 			cgrp->subsys[i]->cgroup = cgrp;
 			list_move(&ss->sibling, &root->subsys_list);
 			ss->root = root;
-			if (ss->bind)
-				ss->bind(cgrp);
 			/* refcount was already taken, and we're keeping it */
 		} else if (bit & removed_mask) {
 			/* We're removing this subsystem */
 			BUG_ON(ss == NULL);
 			BUG_ON(cgrp->subsys[i] != dummytop->subsys[i]);
 			BUG_ON(cgrp->subsys[i]->cgroup != cgrp);
-			if (ss->bind)
-				ss->bind(dummytop);
 			dummytop->subsys[i]->cgroup = dummytop;
 			cgrp->subsys[i] = NULL;
 			subsys[i]->root = &rootnode;
-- 
1.8.1.4

[PATCH] cgroup: remove bind() method from cgroup_subsys.

[Tejun Heo]

From 84cfb6ab484b442d5115eb3baf9db7d74a3ea626 Mon Sep 17 00:00:00 2001
From: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Wed, 10 Apr 2013 14:41:17 +0300

The bind() method of cgroup_subsys is not used in any of the
controllers (cpuset, freezer, blkio, net_cls, memcg, net_prio,
devices, perf, hugetlb, cpu and cpuacct)

tj: Removed the entry on ->bind() from
    Documentation/cgroups/cgroups.txt.  Also updated a couple
    paragraphs which were suggesting that dynamic re-binding may be
    implemented.  It's not gonna.

Signed-off-by: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Applied to cgroup/for-3.10 with documentation updated.

Thanks!

 Documentation/cgroups/cgroups.txt | 20 +++++---------------
 include/linux/cgroup.h            |  2 --
 kernel/cgroup.c                   |  4 ----
 3 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/Documentation/cgroups/cgroups.txt b/Documentation/cgroups/cgroups.txt
index 638bf17..2b51e12 100644
--- a/Documentation/cgroups/cgroups.txt
+++ b/Documentation/cgroups/cgroups.txt
@@ -211,10 +211,9 @@ matches, and any of the requested subsystems are in use in an existing
 hierarchy, the mount will fail with -EBUSY. Otherwise, a new hierarchy
 is activated, associated with the requested subsystems.
 
-It's not currently possible to bind a new subsystem to an active
-cgroup hierarchy, or to unbind a subsystem from an active cgroup
-hierarchy. This may be possible in future, but is fraught with nasty
-error-recovery issues.
+It's not possible to bind a new subsystem to an active cgroup
+hierarchy, or to unbind a subsystem from an active cgroup
+hierarchy.
 
 When a cgroup filesystem is unmounted, if there are any
 child cgroups created below the top-level cgroup, that hierarchy
@@ -382,10 +381,8 @@ To Specify a hierarchy's release_agent:
 
 Note that specifying 'release_agent' more than once will return failure.
 
-Note that changing the set of subsystems is currently only supported
-when the hierarchy consists of a single (root) cgroup. Supporting
-the ability to arbitrarily bind/unbind subsystems from an existing
-cgroup hierarchy is intended to be implemented in the future.
+Note that changing the set of subsystems is only supported when the
+hierarchy consists of a single (root) cgroup.
 
 Then under /sys/fs/cgroup/rg1 you can find a tree that corresponds to the
 tree of the cgroups in the system. For instance, /sys/fs/cgroup/rg1
@@ -643,13 +640,6 @@ void exit(struct task_struct *task)
 
 Called during task exit.
 
-void bind(struct cgroup *root)
-(cgroup_mutex held by caller)
-
-Called when a cgroup subsystem is rebound to a different hierarchy
-and root cgroup. Currently this will only involve movement between
-the default hierarchy (which never has sub-cgroups) and a hierarchy
-that is being created/destroyed (and hence has no sub-cgroups).
 
 4. Extended attribute usage
 ===========================
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 515927e..92acf86 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -483,8 +483,6 @@ struct cgroup_subsys {
 	void (*fork)(struct task_struct *task);
 	void (*exit)(struct cgroup *cgrp, struct cgroup *old_cgrp,
 		     struct task_struct *task);
-	void (*bind)(struct cgroup *root);
-
 	int subsys_id;
 	int active;
 	int disabled;
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index ba3e24a..fd38e1c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1064,16 +1064,12 @@ static int rebind_subsystems(struct cgroupfs_root *root,
 			cgrp->subsys[i]->cgroup = cgrp;
 			list_move(&ss->sibling, &root->subsys_list);
 			ss->root = root;
-			if (ss->bind)
-				ss->bind(cgrp);
 			/* refcount was already taken, and we're keeping it */
 		} else if (bit & removed_mask) {
 			/* We're removing this subsystem */
 			BUG_ON(ss == NULL);
 			BUG_ON(cgrp->subsys[i] != dummytop->subsys[i]);
 			BUG_ON(cgrp->subsys[i]->cgroup != cgrp);
-			if (ss->bind)
-				ss->bind(dummytop);
 			dummytop->subsys[i]->cgroup = dummytop;
 			cgrp->subsys[i] = NULL;
 			subsys[i]->root = &rootnode;
-- 
1.8.1.4

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Li Zefan]

Cc: Galuber

On 2013/4/11 1:50, Tejun Heo wrote:
>>From 84cfb6ab484b442d5115eb3baf9db7d74a3ea626 Mon Sep 17 00:00:00 2001
> From: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Wed, 10 Apr 2013 14:41:17 +0300
> 
> The bind() method of cgroup_subsys is not used in any of the
> controllers (cpuset, freezer, blkio, net_cls, memcg, net_prio,
> devices, perf, hugetlb, cpu and cpuacct)
> 

I remember Glauber once used the bind() callback to detect if cpuacct and
cpu are mounted together and if so do some optimization.

Then he gave up on it and just deprecated cpuacct, but then it was decided
that it can't be decprecated and removed as that will break userspace.

So mayber Glauber will want to use this callback again?

> tj: Removed the entry on ->bind() from
>     Documentation/cgroups/cgroups.txt.  Also updated a couple
>     paragraphs which were suggesting that dynamic re-binding may be
>     implemented.  It's not gonna.
> 
> Signed-off-by: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> Applied to cgroup/for-3.10 with documentation updated.
> 

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Tejun Heo]

Hello, Li.

On Wed, Apr 10, 2013 at 7:02 PM, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> I remember Glauber once used the bind() callback to detect if cpuacct and
> cpu are mounted together and if so do some optimization.
>
> Then he gave up on it and just deprecated cpuacct, but then it was decided
> that it can't be decprecated and removed as that will break userspace.
>
> So mayber Glauber will want to use this callback again?

There was a patchset which made cpu provide the same stats as cpuacct
if mounted together, which bitrotted while Peter was away. We probably
should resurrect those patches. I don't recall whether it used ->bind
or not. Glauber?

Thanks.

--
tejun

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Glauber Costa]

On 04/11/2013 06:06 AM, Tejun Heo wrote:
> Hello, Li.
> 
> On Wed, Apr 10, 2013 at 7:02 PM, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>> I remember Glauber once used the bind() callback to detect if cpuacct and
>> cpu are mounted together and if so do some optimization.
>>
>> Then he gave up on it and just deprecated cpuacct, but then it was decided
>> that it can't be decprecated and removed as that will break userspace.
>>
>> So mayber Glauber will want to use this callback again?
> 
> There was a patchset which made cpu provide the same stats as cpuacct
> if mounted together, which bitrotted while Peter was away. We probably
> should resurrect those patches. I don't recall whether it used ->bind
> or not. Glauber?
> 

The patch didn't suffer any change in its last 2 iteractions, so I was
just waiting for someone to apply =p (or well, say something)

It re-uses the infrastructure that you built in cgroup.c to detect
comounting, so it does not use bind.

I must add that I used bind just because "it was already there", and at
the time we were also quite concerned about not increasing even more the
number of callbacks. It is a horrible interface IMHO.

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Li Zefan]

On 2013/4/11 15:02, Glauber Costa wrote:
> On 04/11/2013 06:06 AM, Tejun Heo wrote:
>> Hello, Li.
>>
>> On Wed, Apr 10, 2013 at 7:02 PM, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>>> I remember Glauber once used the bind() callback to detect if cpuacct and
>>> cpu are mounted together and if so do some optimization.
>>>
>>> Then he gave up on it and just deprecated cpuacct, but then it was decided
>>> that it can't be decprecated and removed as that will break userspace.
>>>
>>> So mayber Glauber will want to use this callback again?
>>
>> There was a patchset which made cpu provide the same stats as cpuacct
>> if mounted together, which bitrotted while Peter was away. We probably
>> should resurrect those patches. I don't recall whether it used ->bind
>> or not. Glauber?
>>
> 
> The patch didn't suffer any change in its last 2 iteractions, so I was
> just waiting for someone to apply =p (or well, say something)
> 
> It re-uses the infrastructure that you built in cgroup.c to detect
> comounting, so it does not use bind.
> 
> I must add that I used bind just because "it was already there", and at
> the time we were also quite concerned about not increasing even more the
> number of callbacks. It is a horrible interface IMHO.
> 

Thanks for the clarification! Then I'm fine with this patch.

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Tejun Heo]

On Thu, Apr 11, 2013 at 11:02:39AM +0400, Glauber Costa wrote:
> The patch didn't suffer any change in its last 2 iteractions, so I was
> just waiting for someone to apply =p (or well, say something)
> 
> It re-uses the infrastructure that you built in cgroup.c to detect
> comounting, so it does not use bind.

Peter is pretty responsive these days.  Can you please rebase the
patches and repost soonish or maybe after 3.10-rc1 opens?

Thanks.

-- 
tejun

Re: [PATCH] cgroup: remove bind() method from cgroup_subsys.

[Tejun Heo]

On Wed, Apr 10, 2013 at 10:50:35AM -0700, Tejun Heo wrote:
> From 84cfb6ab484b442d5115eb3baf9db7d74a3ea626 Mon Sep 17 00:00:00 2001
> From: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Wed, 10 Apr 2013 14:41:17 +0300
> 
> The bind() method of cgroup_subsys is not used in any of the
> controllers (cpuset, freezer, blkio, net_cls, memcg, net_prio,
> devices, perf, hugetlb, cpu and cpuacct)
> 
> tj: Removed the entry on ->bind() from
>     Documentation/cgroups/cgroups.txt.  Also updated a couple
>     paragraphs which were suggesting that dynamic re-binding may be
>     implemented.  It's not gonna.
> 
> Signed-off-by: Rami Rosen <ramirose-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> Applied to cgroup/for-3.10 with documentation updated.

I'm reverting this patch for now as it's getting a new usage.  Maybe
we can try later.

Thanks.

-- 
tejun