linux-kernel - Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4EFD531C.50300@cn.fujitsu.com>
Date:	Fri, 30 Dec 2011 13:58:52 +0800
From:	Li Zefan <lizf@...fujitsu.com>
To:	Tejun Heo <tj@...nel.org>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Cgroups <cgroups@...r.kernel.org>
Subject: Re: [PATCH] cgroup: fix to allow mounting a hierarchy by name

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/