cgroup: fix to allow mounting a hierarchy by name

PATCH Superseded Li Zefan 2 participants 14 messages Dec 27, 2011 – Jan 6, 2012 cgroup

Share & tools

Li Zefan

author

2011-12-27 02:08 UTC #0 lore raw

PATCH 1/1 cgroup: fix to allow mounting a hierarchy by name

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 <[email protected]>

---
 kernel/cgroup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

kernel/cgroup.c +3 -3

index 1042b3c..795107f 100644
@@ -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)

Signature · 1 line

1.7.3.1

Li Zefan

author

2011-12-27 06:23 UTC #1 lore raw

PATCH v2 1/1 cgroup: fix to allow mounting a hierarchy by name

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 <[email protected]>

---

v2: fix code comment s/or/and

---

kernel/cgroup.c +3 -3

index 1042b3c..8e412fd 100644
@@ -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)

Signature · 1 line

1.7.3.1

Tejun Heo

maintainer Applied

2012-01-05 18:00 UTC #2 lore raw

Quoted 10 lines · Li Zefan

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

Quoted 6 lines

But it was broken by commit 32a8cf235e2f192eb002755076994525cdbaa35a (cgroup: make the mount options parsing more accurate)
This fixes the regression.
Signed-off-by Li Zefan <[email protected]>

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

Thank you.

Signature · 1 line

tejun

Tejun Heo

maintainer

2011-12-27 16:35 UTC #3 lore raw

In reply to Li Zefan

Hello, Li.

Quoted 7 lines · Li Zefan

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:

Quoted 7 lines

	# 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.

Signature · 1 line

tejun

Li Zefan

author

2011-12-28 06:08 UTC #4 lore raw

Quoted 16 lines · Tejun Heo

Hello, Li.
Quoted 13 lines · Li Zefan
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)

Quoted 9 lines

Quoted 1 line
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.

Li Zefan

author

2011-12-28 06:10 UTC #5 lore raw

Quoted 31 lines · Li Zefan

Quoted 16 lines · Tejun Heo
Hello, Li.
Quoted 13 lines · Li Zefan
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)
Quoted 9 lines
Quoted 1 line
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:

Quoted 6 lines

	# 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

Quoted 2 lines

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

Tejun Heo

maintainer

2011-12-28 16:36 UTC #6 lore raw

In reply to Li Zefan

Hello, Li.

Quoted 8 lines · Li Zefan

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?

Quoted 4 lines

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.

Signature · 1 line

tejun

Li Zefan

author

2011-12-29 02:47 UTC #7 lore raw

Quoted 7 lines · Tejun Heo

Hello, Li.
Quoted 4 lines · Li Zefan
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:

Quoted 8 lines

Quoted 4 lines
	# 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?

Quoted 9 lines

Quoted 4 lines
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.

Tejun Heo

maintainer

2011-12-29 16:23 UTC #8 lore raw

Hello,

Quoted 11 lines · Li Zefan

Quoted 10 lines · Tejun Heo
Hello, Li.
Quoted 7 lines · Li Zefan
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

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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.

Quoted 5 lines

Quoted 4 lines
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.

Quoted 7 lines

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.

Signature · 1 line

tejun

Li Zefan

author

2011-12-30 05:56 UTC #9 lore raw

Quoted 16 lines · Tejun Heo

Hello,
Quoted 13 lines · Li Zefan
Quoted 12 lines · Tejun Heo
Hello, Li.
Quoted 8 lines · Li Zefan
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.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 6 lines

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?

Quoted 13 lines

Quoted 5 lines
Quoted 4 lines
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.
Quoted 7 lines
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.

Quoted 8 lines

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?

Li Zefan

author

2012-01-05 02:20 UTC #10 lore raw

Any comments on this?

Quoted 98 lines · Li Zefan

Quoted 16 lines · Tejun Heo
Hello,
Quoted 13 lines · Li Zefan
Quoted 12 lines · Tejun Heo
Hello, Li.
Quoted 8 lines · Li Zefan
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.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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.
Quoted 6 lines
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?
Quoted 13 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 7 lines
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.
Quoted 8 lines
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?

Tejun Heo

maintainer

2012-01-05 17:45 UTC #11 lore raw

In reply to Li Zefan

Hello,

Quoted 19 lines · Li Zefan

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:

Quoted 8 lines

	# 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?

Quoted 6 lines

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.

Quoted 6 lines

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.

Signature · 1 line

tejun

Li Zefan

author

2012-01-06 02:22 UTC #12 lore raw

Quoted 27 lines · Tejun Heo

Hello,
Quoted 24 lines · Li Zefan
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:

Quoted 6 lines

Quoted 3 lines
	# 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.

Quoted 4 lines

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?

Tejun Heo

maintainer

2012-01-06 02:27 UTC #13 lore raw

Hello,

Quoted 9 lines · Li Zefan

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.

Quoted 8 lines

Quoted 3 lines
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.

Signature · 1 line

tejun