Home » Mailing lists » Devel » [PATCH 2/3] make clone_children a flag
| [PATCH 2/3] make clone_children a flag [message #44482] |
Sun, 11 December 2011 14:45  |
Glauber Costa
Messages: 916
Registered: October 2011
|
Senior Member |
|
|
There is no reason to have a flags field, and then a separate
bool field just to indicate if the clone_children flag is set.
Make it a flag
Signed-off-by: Glauber Costa <glommer@parallels.com>
---
kernel/cgroup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e4b9d3c..fa405ee 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -231,6 +231,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
/* bits in struct cgroupfs_root flags field */
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
+ ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1062,7 +1063,6 @@ struct cgroup_sb_opts {
unsigned long subsys_bits;
unsigned long flags;
char *release_agent;
- bool clone_children;
char *name;
/* User explicitly requested empty subsystem */
bool none;
@@ -1113,7 +1113,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
continue;
}
if (!strcmp(token, "clone_children")) {
- opts->clone_children = true;
+ set_bit(ROOT_CLONE_CHILDREN, &opts->flags);
continue;
}
if (!strncmp(token, "release_agent=", 14)) {
@@ -1400,7 +1400,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
strcpy(root->release_agent_path, opts->release_agent);
if (opts->name)
strcpy(root->name, opts->name);
- if (opts->clone_children)
+ if (test_bit(ROOT_CLONE_CHILDREN, &opts->flags))
set_bit(CGRP_CLONE_CHILDREN, &root->top_cgroup.flags);
return root;
}
--
1.7.6.4
|
|
|
|
|
|
|
|
| Re: [PATCH 2/3] make clone_children a flag [message #44546 is a reply to message #44543] |
Wed, 14 December 2011 07:09  |
Glauber Costa
Messages: 916
Registered: October 2011
|
Senior Member |
|
|
On 12/14/2011 06:29 AM, Li Zefan wrote:
> Tejun Heo wrote:
>> On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
>>> There is no reason to have a flags field, and then a separate
>>> bool field just to indicate if the clone_children flag is set.
>>> Make it a flag
>>>
>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>
>> Doesn't this change how remount conditions are checked?
>>
Well, I was thinking it wouldn't, because I patched all callers. But I
forget life is not always that simple: After you mentioned, I checked
and we do test for changes in the flag field explicitly on remount. So I
missed that, indeed.
> Right. Currently we can do this:
>
> # mount -t cgroup xxx /mnt
> # mount -o remount,clone_children /mnt
>
> with this patch, the above remount will fail.
>
> But..the current bevaiour of remount is a bit confusing in that remount
> with/without "clone_children" has no effect on anything:
>
> # mount -t cgroup -o clone_children xxx /mnt
> # cat /mnt/cgroup.clone_children
> 1
> # mount -o remount xxx /mnt
> # mount | grep cgroup
> xxx on /mnt type cgroup (rw,clone_children)
> # cat /mnt/cgroup.clone_children
> 1
That's indeed confusing, and it comes from the fact that we always
inherit clone_children from the parent - which is sane, IMHO. So this
flag only has any value in establishing the initial behaviour of the top
root cgroup. I wonder then if it wouldn't better to just be explicit and
fail in this case ?
|
|
|
|
|
|
| Re: [PATCH 2/3] make clone_children a flag [message #44573 is a reply to message #44566] |
Thu, 15 December 2011 07:03 |
Glauber Costa
Messages: 916
Registered: October 2011
|
Senior Member |
|
|
On 12/14/2011 10:18 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Dec 14, 2011 at 11:09:14AM +0400, Glauber Costa wrote:
>> That's indeed confusing, and it comes from the fact that we always
>> inherit clone_children from the parent - which is sane, IMHO. So
>> this flag only has any value in establishing the initial behaviour
>> of the top root cgroup. I wonder then if it wouldn't better to just
>> be explicit and fail in this case ?
>
> I don't think all current behaviors are sane and if not let's change
> them, but those things have to be explicit with proper description and
> rationale.
>
140 % agree to that. As I said, I wrongly believed it to be functionally
equivalent when I sent it, but missed the flags remount check.
If you believe the behavior we now get is saner, I can rewrite the
Changelog and resend it.
|
|
|
|
|
|
Goto Forum:
Current Time: Tue Nov 19 11:53:14 GMT 2024
Total time taken to generate the page: 0.03235 seconds
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
