linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cgroup: fix to allow mounting a hierarchy by name
@ 2011-12-27  2:10 Li Zefan
  2011-12-27  6:25 ` [PATCH v2] " Li Zefan
  2011-12-27 16:35 ` [PATCH] " Tejun Heo
  0 siblings, 2 replies; 14+ messages in thread
From: Li Zefan @ 2011-12-27  2:10 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

If we mount a hierarchy with a name specified, the name is unique,
and we can use it to mount the hierarchy without specifying its
set of subsystem names. This feature is documented is
Documentation/cgroups/cgroups.txt section 2.3

Here's an example:

	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
	# mount -t cgroup -o name=myhier xxx /cgroup2

But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
(cgroup: make the mount options parsing more accurate)

This fixes the regression.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cgroup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 1042b3c..795107f 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1193,10 +1193,10 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 
 	/*
 	 * If the 'all' option was specified select all the subsystems,
-	 * otherwise 'all, 'none' and a subsystem name options were not
-	 * specified, let's default to 'all'
+	 * otherwise if 'none', 'name=' or a subsystem name options
+	 * were not specified, let's default to 'all'
 	 */
-	if (all_ss || (!all_ss && !one_ss && !opts->none)) {
+	if (all_ss || (!one_ss && !opts->none && !opts->name)) {
 		for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
 			struct cgroup_subsys *ss = subsys[i];
 			if (ss == NULL)
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2] cgroup: fix to allow mounting a hierarchy by name
  2011-12-27  2:10 [PATCH] cgroup: fix to allow mounting a hierarchy by name Li Zefan
@ 2011-12-27  6:25 ` Li Zefan
  2012-01-05 18:00   ` Tejun Heo
  2011-12-27 16:35 ` [PATCH] " Tejun Heo
  1 sibling, 1 reply; 14+ messages in thread
From: Li Zefan @ 2011-12-27  6:25 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

If we mount a hierarchy with a specified name, the name is unique,
and we can use it to mount the hierarchy without specifying its
set of subsystem names. This feature is documented is
Documentation/cgroups/cgroups.txt section 2.3

Here's an example:

	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
	# mount -t cgroup -o name=myhier xxx /cgroup2

But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
(cgroup: make the mount options parsing more accurate)

This fixes the regression.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---

v2: fix code comment s/or/and

---

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 1042b3c..8e412fd 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1193,10 +1193,10 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 
 	/*
 	 * If the 'all' option was specified select all the subsystems,
-	 * otherwise 'all, 'none' and a subsystem name options were not
-	 * specified, let's default to 'all'
+	 * otherwise if 'none', 'name=' and a subsystem name options
+	 * were not specified, let's default to 'all'
 	 */
-	if (all_ss || (!all_ss && !one_ss && !opts->none)) {
+	if (all_ss || (!one_ss && !opts->none && !opts->name)) {
 		for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
 			struct cgroup_subsys *ss = subsys[i];
 			if (ss == NULL)
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-27  2:10 [PATCH] cgroup: fix to allow mounting a hierarchy by name Li Zefan
  2011-12-27  6:25 ` [PATCH v2] " Li Zefan
@ 2011-12-27 16:35 ` Tejun Heo
  2011-12-28  6:10   ` Li Zefan
  1 sibling, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2011-12-27 16:35 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

Hello, Li.

On Tue, Dec 27, 2011 at 10:10:37AM +0800, Li Zefan wrote:
> If we mount a hierarchy with a name specified, the name is unique,
> and we can use it to mount the hierarchy without specifying its
> set of subsystem names. This feature is documented is
> Documentation/cgroups/cgroups.txt section 2.3
> 
> Here's an example:
> 
> 	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
> 	# mount -t cgroup -o name=myhier xxx /cgroup2
> 
> But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
> (cgroup: make the mount options parsing more accurate)
> 
> This fixes the regression.

Hmmm... so that has been broken over a year and nobody complained?  Is
there any valid use case where specifying mount path as all other
filesystems wouldn't work?  Is this 'name' thing necessary at all?
Maybe we should rip it out instead of fixing it?  We can just leave it
as non-functional name which does nothing but being visible in mount
listing.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-27 16:35 ` [PATCH] " Tejun Heo
@ 2011-12-28  6:10   ` Li Zefan
  2011-12-28  6:12     ` Li Zefan
  2011-12-28 16:36     ` Tejun Heo
  0 siblings, 2 replies; 14+ messages in thread
From: Li Zefan @ 2011-12-28  6:10 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Tejun Heo wrote:
> Hello, Li.
> 
> On Tue, Dec 27, 2011 at 10:10:37AM +0800, Li Zefan wrote:
>> If we mount a hierarchy with a name specified, the name is unique,
>> and we can use it to mount the hierarchy without specifying its
>> set of subsystem names. This feature is documented is
>> Documentation/cgroups/cgroups.txt section 2.3
>>
>> Here's an example:
>>
>> 	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
>> 	# mount -t cgroup -o name=myhier xxx /cgroup2
>>
>> But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
>> (cgroup: make the mount options parsing more accurate)
>>
>> This fixes the regression.
> 
> Hmmm... so that has been broken over a year and nobody complained?  Is
> there any valid use case where specifying mount path as all other
> filesystems wouldn't work?  Is this 'name' thing necessary at all?
> Maybe we should rip it out instead of fixing it?  We can just leave it
> as non-functional name which does nothing but being visible in mount
> listing.
> 

The "name" option was introduced along with the "none" option, so we
can distinguish between different cgroup hierarchies which have no
bound subsystems, like this:

	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
	# mount -t cgroup -o none,name=hier2 xxx /cgroup2

As the name is unique, we have this "mount by hierarchy name" feature.
It looks reasonable, but I guess few people know this feature. We can
live with it, as it only saves us some typing when mounting an existing
hierarchy. On the other hand, removing this small feature can hardly
result in code reduction.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-28  6:10   ` Li Zefan
@ 2011-12-28  6:12     ` Li Zefan
  2011-12-28 16:36     ` Tejun Heo
  1 sibling, 0 replies; 14+ messages in thread
From: Li Zefan @ 2011-12-28  6:12 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Li Zefan wrote:
> Tejun Heo wrote:
>> Hello, Li.
>>
>> On Tue, Dec 27, 2011 at 10:10:37AM +0800, Li Zefan wrote:
>>> If we mount a hierarchy with a name specified, the name is unique,
>>> and we can use it to mount the hierarchy without specifying its
>>> set of subsystem names. This feature is documented is
>>> Documentation/cgroups/cgroups.txt section 2.3
>>>
>>> Here's an example:
>>>
>>> 	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
>>> 	# mount -t cgroup -o name=myhier xxx /cgroup2
>>>
>>> But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
>>> (cgroup: make the mount options parsing more accurate)
>>>
>>> This fixes the regression.
>>
>> Hmmm... so that has been broken over a year and nobody complained?  Is
>> there any valid use case where specifying mount path as all other
>> filesystems wouldn't work?  Is this 'name' thing necessary at all?
>> Maybe we should rip it out instead of fixing it?  We can just leave it
>> as non-functional name which does nothing but being visible in mount
>> listing.
>>
> 
> The "name" option was introduced along with the "none" option, so we
> can distinguish between different cgroup hierarchies which have no
> bound subsystems, like this:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
> 
> As the name is unique, we have this "mount by hierarchy name" feature.
> It looks reasonable, but I guess few people know this feature. We can
> live with it, as it only saves us some typing when mounting an existing

s/with/without

> hierarchy. On the other hand, removing this small feature can hardly
> result in code reduction.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-28  6:10   ` Li Zefan
  2011-12-28  6:12     ` Li Zefan
@ 2011-12-28 16:36     ` Tejun Heo
  2011-12-29  2:50       ` Li Zefan
  1 sibling, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2011-12-28 16:36 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

Hello, Li.

On Wed, Dec 28, 2011 at 02:10:42PM +0800, Li Zefan wrote:
> The "name" option was introduced along with the "none" option, so we
> can distinguish between different cgroup hierarchies which have no
> bound subsystems, like this:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
> 
> As the name is unique, we have this "mount by hierarchy name" feature.

I could be missing something but does that add anything other than
naming convenience?

> It looks reasonable, but I guess few people know this feature. We can
> live with it, as it only saves us some typing when mounting an existing
> hierarchy. On the other hand, removing this small feature can hardly
> result in code reduction.

If it's a redundant feature which has been broken over a year without
anyone complaining, it really doesn't need to exist.  It might not
save a lot of code but would save some WTH moments.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-28 16:36     ` Tejun Heo
@ 2011-12-29  2:50       ` Li Zefan
  2011-12-29 16:23         ` Tejun Heo
  0 siblings, 1 reply; 14+ messages in thread
From: Li Zefan @ 2011-12-29  2:50 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Tejun Heo wrote:
> Hello, Li.
> 
> On Wed, Dec 28, 2011 at 02:10:42PM +0800, Li Zefan wrote:
>> The "name" option was introduced along with the "none" option, so we
>> can distinguish between different cgroup hierarchies which have no
>> bound subsystems, like this:
>>
>> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
>> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
>>
>> As the name is unique, we have this "mount by hierarchy name" feature.
> 
> I could be missing something but does that add anything other than
> naming convenience?
> 

The name option is necessary, otherwise how can we mount hierarchies
as shown in the above example?

>> It looks reasonable, but I guess few people know this feature. We can
>> live with it, as it only saves us some typing when mounting an existing
>> hierarchy. On the other hand, removing this small feature can hardly
>> result in code reduction.
> 
> If it's a redundant feature which has been broken over a year without
> anyone complaining, it really doesn't need to exist.  It might not
> save a lot of code but would save some WTH moments.
> 

The redundant feature is mouting existing hierarchies by specifying name
only, and the cleanup patch I sent has this feature removed in effect.

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

This is why I'm not so keen to remove the feature.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-29  2:50       ` Li Zefan
@ 2011-12-29 16:23         ` Tejun Heo
  2011-12-30  5:58           ` Li Zefan
  0 siblings, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2011-12-29 16:23 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

Hello,

On Thu, Dec 29, 2011 at 10:50:06AM +0800, Li Zefan wrote:
> Tejun Heo wrote:
> > Hello, Li.
> > 
> > On Wed, Dec 28, 2011 at 02:10:42PM +0800, Li Zefan wrote:
> >> The "name" option was introduced along with the "none" option, so we
> >> can distinguish between different cgroup hierarchies which have no
> >> bound subsystems, like this:
> >>
> >> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
> >> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
> >>
> >> As the name is unique, we have this "mount by hierarchy name" feature.
> > 
> > I could be missing something but does that add anything other than
> > naming convenience?
> 
> The name option is necessary, otherwise how can we mount hierarchies
> as shown in the above example?

I don't think mounting itself would be a problem.  We don't need name
option to create multiple tmpfs instances, right?  The problem is
referencing to them after they're created.  Filesystems generally
don't need such identifier because, once they're created, they can be
referenced by their mount points.  I'm still not very familiar with
different corners of cgroup, so it's entirely possible that I'm
missing something.  If I am, please point me to it.

> > If it's a redundant feature which has been broken over a year without
> > anyone complaining, it really doesn't need to exist.  It might not
> > save a lot of code but would save some WTH moments.
> > 
> 
> The redundant feature is mouting existing hierarchies by specifying name
> only, and the cleanup patch I sent has this feature removed in effect.
> 
> kernel/cgroup.c |   15 +++++++--------
> 1 files changed, 7 insertions(+), 8 deletions(-)
> 
> This is why I'm not so keen to remove the feature.

Code reduction is definitely a plus and I don't want to remove a
useful feature either, but an unusual redundant feature without
necessity is confusing / misleading even if it doesn't necessasrily
add a lot of code complexity.

Also, I at least want to understand why it's actually necessary before
applying the patches.  :)

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-29 16:23         ` Tejun Heo
@ 2011-12-30  5:58           ` Li Zefan
  2012-01-05  2:20             ` Li Zefan
  2012-01-05 17:45             ` Tejun Heo
  0 siblings, 2 replies; 14+ messages in thread
From: Li Zefan @ 2011-12-30  5:58 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Tejun Heo wrote:
> Hello,
> 
> On Thu, Dec 29, 2011 at 10:50:06AM +0800, Li Zefan wrote:
>> Tejun Heo wrote:
>>> Hello, Li.
>>>
>>> On Wed, Dec 28, 2011 at 02:10:42PM +0800, Li Zefan wrote:
>>>> The "name" option was introduced along with the "none" option, so we
>>>> can distinguish between different cgroup hierarchies which have no
>>>> bound subsystems, like this:
>>>>
>>>> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
>>>> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
>>>>
>>>> As the name is unique, we have this "mount by hierarchy name" feature.
>>>
>>> I could be missing something but does that add anything other than
>>> naming convenience?
>>
>> The name option is necessary, otherwise how can we mount hierarchies
>> as shown in the above example?
> 
> I don't think mounting itself would be a problem.  We don't need name
> option to create multiple tmpfs instances, right?  The problem is

Right, but you can't mount the same tmpfs instance in more than one mount
point.

> referencing to them after they're created.  Filesystems generally
> don't need such identifier because, once they're created, they can be
> referenced by their mount points.  I'm still not very familiar with
> different corners of cgroup, so it's entirely possible that I'm
> missing something.  If I am, please point me to it.
> 

Normal filesystems can have multi mount points, and an fs instance
is identified by device name, but cgroupfs ignores device name like
other pseudo filesystems. Instead a set of subsystems is used, so
to mount the same cgroupfs instance in different mount points, we
can do this:

	# mount -t cgroup -o cpuset xxx /cgroup1
	# mount -t cgroup -o cpuset xxx /cgroup2

Now we have the "none" option, so a cgroupfs can have no subsystems
bound to it, and we allow multi instances of such cgroupfs, so we
have to assign names to each instance:

	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
	# mount -t cgroup -o none,name=hier2 xxx /cgroup2

Then we want to also mount "hier1" in another mount point, we can't
do this:

	# mount -t cgroup -o none xxx /mnt

because we have two different instances with "none" subsystem. So
we specify its name:

	# mount -t cgroup -o none,name=hier1 xxx /mnt

Hope I have made things clear to you?

>>> If it's a redundant feature which has been broken over a year without
>>> anyone complaining, it really doesn't need to exist.  It might not
>>> save a lot of code but would save some WTH moments.
>>>
>>
>> The redundant feature is mouting existing hierarchies by specifying name
>> only, and the cleanup patch I sent has this feature removed in effect.
>>
>> kernel/cgroup.c |   15 +++++++--------
>> 1 files changed, 7 insertions(+), 8 deletions(-)
>>
>> This is why I'm not so keen to remove the feature.
> 
> Code reduction is definitely a plus and I don't want to remove a
> useful feature either, but an unusual redundant feature without
> necessity is confusing / misleading even if it doesn't necessasrily
> add a lot of code complexity.
> 
> Also, I at least want to understand why it's actually necessary before
> applying the patches.  :)
> 

What I try to fix here is the behavior of "mount -t cgroup -o name=xxx ..."
(no other options are specified), so what behavior do we want?

1. find if any existing cgroupfs instance matches the name, which is
the orginal behavior.

2. the same as "mount -t cgroup -o all,name=xxx ...", which is the
current behavior due to the commit that broke (1).

3. make it invalid and fail to mount.

4. any other idea?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-30  5:58           ` Li Zefan
@ 2012-01-05  2:20             ` Li Zefan
  2012-01-05 17:45             ` Tejun Heo
  1 sibling, 0 replies; 14+ messages in thread
From: Li Zefan @ 2012-01-05  2:20 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Any comments on this?

Li Zefan wrote:
> Tejun Heo wrote:
>> Hello,
>>
>> On Thu, Dec 29, 2011 at 10:50:06AM +0800, Li Zefan wrote:
>>> Tejun Heo wrote:
>>>> Hello, Li.
>>>>
>>>> On Wed, Dec 28, 2011 at 02:10:42PM +0800, Li Zefan wrote:
>>>>> The "name" option was introduced along with the "none" option, so we
>>>>> can distinguish between different cgroup hierarchies which have no
>>>>> bound subsystems, like this:
>>>>>
>>>>> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
>>>>> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
>>>>>
>>>>> As the name is unique, we have this "mount by hierarchy name" feature.
>>>>
>>>> I could be missing something but does that add anything other than
>>>> naming convenience?
>>>
>>> The name option is necessary, otherwise how can we mount hierarchies
>>> as shown in the above example?
>>
>> I don't think mounting itself would be a problem.  We don't need name
>> option to create multiple tmpfs instances, right?  The problem is
> 
> Right, but you can't mount the same tmpfs instance in more than one mount
> point.
> 
>> referencing to them after they're created.  Filesystems generally
>> don't need such identifier because, once they're created, they can be
>> referenced by their mount points.  I'm still not very familiar with
>> different corners of cgroup, so it's entirely possible that I'm
>> missing something.  If I am, please point me to it.
>>
> 
> Normal filesystems can have multi mount points, and an fs instance
> is identified by device name, but cgroupfs ignores device name like
> other pseudo filesystems. Instead a set of subsystems is used, so
> to mount the same cgroupfs instance in different mount points, we
> can do this:
> 
> 	# mount -t cgroup -o cpuset xxx /cgroup1
> 	# mount -t cgroup -o cpuset xxx /cgroup2
> 
> Now we have the "none" option, so a cgroupfs can have no subsystems
> bound to it, and we allow multi instances of such cgroupfs, so we
> have to assign names to each instance:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
> 
> Then we want to also mount "hier1" in another mount point, we can't
> do this:
> 
> 	# mount -t cgroup -o none xxx /mnt
> 
> because we have two different instances with "none" subsystem. So
> we specify its name:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /mnt
> 
> Hope I have made things clear to you?
> 
>>>> If it's a redundant feature which has been broken over a year without
>>>> anyone complaining, it really doesn't need to exist.  It might not
>>>> save a lot of code but would save some WTH moments.
>>>>
>>>
>>> The redundant feature is mouting existing hierarchies by specifying name
>>> only, and the cleanup patch I sent has this feature removed in effect.
>>>
>>> kernel/cgroup.c |   15 +++++++--------
>>> 1 files changed, 7 insertions(+), 8 deletions(-)
>>>
>>> This is why I'm not so keen to remove the feature.
>>
>> Code reduction is definitely a plus and I don't want to remove a
>> useful feature either, but an unusual redundant feature without
>> necessity is confusing / misleading even if it doesn't necessasrily
>> add a lot of code complexity.
>>
>> Also, I at least want to understand why it's actually necessary before
>> applying the patches.  :)
>>
> 
> What I try to fix here is the behavior of "mount -t cgroup -o name=xxx ..."
> (no other options are specified), so what behavior do we want?
> 
> 1. find if any existing cgroupfs instance matches the name, which is
> the orginal behavior.
> 
> 2. the same as "mount -t cgroup -o all,name=xxx ...", which is the
> current behavior due to the commit that broke (1).
> 
> 3. make it invalid and fail to mount.
> 
> 4. any other idea?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2011-12-30  5:58           ` Li Zefan
  2012-01-05  2:20             ` Li Zefan
@ 2012-01-05 17:45             ` Tejun Heo
  2012-01-06  2:24               ` Li Zefan
  1 sibling, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2012-01-05 17:45 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

Hello,

On Fri, Dec 30, 2011 at 01:58:52PM +0800, Li Zefan wrote:
> Normal filesystems can have multi mount points, and an fs instance
> is identified by device name, but cgroupfs ignores device name like
> other pseudo filesystems. Instead a set of subsystems is used, so
> to mount the same cgroupfs instance in different mount points, we
> can do this:
> 
> 	# mount -t cgroup -o cpuset xxx /cgroup1
> 	# mount -t cgroup -o cpuset xxx /cgroup2
>
> Now we have the "none" option, so a cgroupfs can have no subsystems
> bound to it, and we allow multi instances of such cgroupfs, so we
> have to assign names to each instance:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
> 
> Then we want to also mount "hier1" in another mount point, we can't
> do this:
> 
> 	# mount -t cgroup -o none xxx /mnt
> 
> because we have two different instances with "none" subsystem. So
> we specify its name:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /mnt
> 
> Hope I have made things clear to you?

mount --bind?  It's not exactly the same thing but I don't think the
differences would matter for cgroup.  Also, what's the use case for
mounting the same cgroup directory multiple times?  Why is that
necessary?  Is it useful for some namespace-savvy setup?

> What I try to fix here is the behavior of "mount -t cgroup -o name=xxx ..."
> (no other options are specified), so what behavior do we want?
> 
> 1. find if any existing cgroupfs instance matches the name, which is
> the orginal behavior.
> 
> 2. the same as "mount -t cgroup -o all,name=xxx ...", which is the
> current behavior due to the commit that broke (1).
> 
> 3. make it invalid and fail to mount.
> 
> 4. any other idea?

I guess I'll apply the patches but it still seems like a silly
redundant feature.  If not, please enlighten me.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] cgroup: fix to allow mounting a hierarchy by name
  2011-12-27  6:25 ` [PATCH v2] " Li Zefan
@ 2012-01-05 18:00   ` Tejun Heo
  0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2012-01-05 18:00 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

On Tue, Dec 27, 2011 at 02:25:55PM +0800, Li Zefan wrote:
> If we mount a hierarchy with a specified name, the name is unique,
> and we can use it to mount the hierarchy without specifying its
> set of subsystem names. This feature is documented is
> Documentation/cgroups/cgroups.txt section 2.3
> 
> Here's an example:
> 
> 	# mount -t cgroup -o cpuset,name=myhier xxx /cgroup1
> 	# mount -t cgroup -o name=myhier xxx /cgroup2
> 
> But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a
> (cgroup: make the mount options parsing more accurate)
> 
> This fixes the regression.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Applied to cgroup/for-3.3 w/ cc to stable added.

Thank you.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2012-01-05 17:45             ` Tejun Heo
@ 2012-01-06  2:24               ` Li Zefan
  2012-01-06  2:27                 ` Tejun Heo
  0 siblings, 1 reply; 14+ messages in thread
From: Li Zefan @ 2012-01-06  2:24 UTC (permalink / raw)
  To: Tejun Heo; +Cc: LKML, Cgroups

Tejun Heo wrote:
> Hello,
> 
> On Fri, Dec 30, 2011 at 01:58:52PM +0800, Li Zefan wrote:
>> Normal filesystems can have multi mount points, and an fs instance
>> is identified by device name, but cgroupfs ignores device name like
>> other pseudo filesystems. Instead a set of subsystems is used, so
>> to mount the same cgroupfs instance in different mount points, we
>> can do this:
>>
>> 	# mount -t cgroup -o cpuset xxx /cgroup1
>> 	# mount -t cgroup -o cpuset xxx /cgroup2
>>
>> Now we have the "none" option, so a cgroupfs can have no subsystems
>> bound to it, and we allow multi instances of such cgroupfs, so we
>> have to assign names to each instance:
>>
>> 	# mount -t cgroup -o none,name=hier1 xxx /cgroup1
>> 	# mount -t cgroup -o none,name=hier2 xxx /cgroup2
>>
>> Then we want to also mount "hier1" in another mount point, we can't
>> do this:
>>
>> 	# mount -t cgroup -o none xxx /mnt
>>
>> because we have two different instances with "none" subsystem. So
>> we specify its name:
>>
>> 	# mount -t cgroup -o none,name=hier1 xxx /mnt
>>
>> Hope I have made things clear to you?
> 
> mount --bind?  It's not exactly the same thing but I don't think the
> differences would matter for cgroup.

There's a corner case where "mount --bind" can't be used:

	# mount -t cgroup -o none,name=hier1 xxx /mnt
	# mkdir /mnt/tmp
	# umount /mnt

Since there's a sub-cgroup in it, umount won't destroy the hierarchy,
but "hide" it, so the name=xxx is necessary to re-mount it.

> Also, what's the use case for
> mounting the same cgroup directory multiple times?  Why is that
> necessary?  Is it useful for some namespace-savvy setup?
> 

I don't have a use case in real life. It was made so at the very
beginning of cgroup, and we should't break it without strong reasons.
We can mount other pseudo filesystems like procfs, sysfs and debugfs
multiple times, right?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name
  2012-01-06  2:24               ` Li Zefan
@ 2012-01-06  2:27                 ` Tejun Heo
  0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2012-01-06  2:27 UTC (permalink / raw)
  To: Li Zefan; +Cc: LKML, Cgroups

Hello,

On Fri, Jan 06, 2012 at 10:24:47AM +0800, Li Zefan wrote:
> There's a corner case where "mount --bind" can't be used:
> 
> 	# mount -t cgroup -o none,name=hier1 xxx /mnt
> 	# mkdir /mnt/tmp
> 	# umount /mnt
> 
> Since there's a sub-cgroup in it, umount won't destroy the hierarchy,
> but "hide" it, so the name=xxx is necessary to re-mount it.
>
> > Also, what's the use case for
> > mounting the same cgroup directory multiple times?  Why is that
> > necessary?  Is it useful for some namespace-savvy setup?
> 
> I don't have a use case in real life. It was made so at the very
> beginning of cgroup, and we should't break it without strong reasons.
> We can mount other pseudo filesystems like procfs, sysfs and debugfs
> multiple times, right?

Yeah, we shouldn't break it so I applied the patch with a lot of
grumbling.  This is something which shouldn't have existed at all. :(

Thanks a lot for the explanation and patience.

-- 
tejun

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-01-06  2:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-27  2:10 [PATCH] cgroup: fix to allow mounting a hierarchy by name Li Zefan
2011-12-27  6:25 ` [PATCH v2] " Li Zefan
2012-01-05 18:00   ` Tejun Heo
2011-12-27 16:35 ` [PATCH] " Tejun Heo
2011-12-28  6:10   ` Li Zefan
2011-12-28  6:12     ` Li Zefan
2011-12-28 16:36     ` Tejun Heo
2011-12-29  2:50       ` Li Zefan
2011-12-29 16:23         ` Tejun Heo
2011-12-30  5:58           ` Li Zefan
2012-01-05  2:20             ` Li Zefan
2012-01-05 17:45             ` Tejun Heo
2012-01-06  2:24               ` Li Zefan
2012-01-06  2:27                 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).