All of lore.kernel.org
 help / color / mirror / Atom feed
From: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
To: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jbottomley-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bsingharora-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org
Subject: Re: [PATCH 2/3] make clone_children a flag
Date: Wed, 14 Dec 2011 11:09:14 +0400	[thread overview]
Message-ID: <4EE84B9A.90901@parallels.com> (raw)
In-Reply-To: <4EE80A0D.7090808-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

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-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> 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 ?

--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Glauber Costa <glommer@parallels.com>
To: Li Zefan <lizf@cn.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>, <linux-kernel@vger.kernel.org>,
	<jbottomley@parallels.com>, <cgroups@vger.kernel.org>,
	<bsingharora@gmail.com>, <devel@openvz.org>,
	<kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [PATCH 2/3] make clone_children a flag
Date: Wed, 14 Dec 2011 11:09:14 +0400	[thread overview]
Message-ID: <4EE84B9A.90901@parallels.com> (raw)
In-Reply-To: <4EE80A0D.7090808@cn.fujitsu.com>

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 ?


  parent reply	other threads:[~2011-12-14  7:09 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-11 14:45 [PATCH 0/3] Simple cleanups for cgroups Glauber Costa
2011-12-11 14:45 ` Glauber Costa
     [not found] ` <1323614738-7405-1-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-11 14:45   ` [PATCH] make clone_children a flag Glauber Costa
2011-12-11 14:45     ` Glauber Costa
2011-12-11 14:48     ` Glauber Costa
2011-12-11 14:48       ` Glauber Costa
2011-12-11 14:45   ` [PATCH 1/3] nitpick: make simple functions inline Glauber Costa
2011-12-11 14:45     ` Glauber Costa
     [not found]     ` <1323614738-7405-3-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-11 18:55       ` KOSAKI Motohiro
2011-12-11 18:55         ` KOSAKI Motohiro
     [not found]         ` <4EE4FCBB.9050600-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2011-12-11 20:44           ` Glauber Costa
2011-12-11 20:44             ` Glauber Costa
     [not found]             ` <4EE51646.3030900-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-12 17:27               ` Tejun Heo
2011-12-12 17:27                 ` Tejun Heo
2011-12-14  1:09       ` Li Zefan
2011-12-14  1:09         ` Li Zefan
2011-12-11 14:45   ` [PATCH 2/3] make clone_children a flag Glauber Costa
2011-12-11 14:45     ` Glauber Costa
     [not found]     ` <1323614738-7405-4-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-11 18:58       ` KOSAKI Motohiro
2011-12-11 18:58         ` KOSAKI Motohiro
2011-12-13 15:39       ` Tejun Heo
2011-12-13 15:39         ` Tejun Heo
     [not found]         ` <20111213153921.GE25802-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-14  2:29           ` Li Zefan
2011-12-14  2:29             ` Li Zefan
     [not found]             ` <4EE80A0D.7090808-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2011-12-14  7:09               ` Glauber Costa [this message]
2011-12-14  7:09                 ` Glauber Costa
     [not found]                 ` <4EE84B9A.90901-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-14 18:18                   ` Tejun Heo
2011-12-14 18:18                     ` Tejun Heo
2011-12-15  7:03                     ` Glauber Costa
2011-12-15  7:03                       ` Glauber Costa
2011-12-11 14:45   ` [PATCH 3/3] make 'none' field " Glauber Costa
2011-12-11 14:45     ` Glauber Costa
2011-12-11 18:59     ` KOSAKI Motohiro
     [not found]     ` <1323614738-7405-5-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-12-13 15:41       ` Tejun Heo
2011-12-13 15:41         ` Tejun Heo
     [not found]         ` <20111213154108.GF25802-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-14  2:09           ` Li Zefan
2011-12-14  2:09             ` Li Zefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EE84B9A.90901@parallels.com \
    --to=glommer-bzqdu9zft3wakbo8gow8eq@public.gmane.org \
    --cc=bsingharora-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    --cc=jbottomley-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org \
    --cc=kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org \
    --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.