On Mon, Jun 01, 2009 at 11:02:58PM -0700, [email protected] wrote:
>
> The patch titled
> cgroups: forbid noprefix if mounting more than just cpuset subsystem
> has been added to the -mm tree. Its filename is
> cgroups-forbid-noprefix-if-mounting-more-than-just-cpuset-subsystem.patch
>
> Before you just go and hit "reply", please:
> a) Consider who else should be cc'ed
> b) Prefer to cc a suitable mailing list as well
> c) Ideally: find the original patch on the mailing list and do a
> reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>
> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> out what to do about this
>
> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>
> ------------------------------------------------------
> Subject: cgroups: forbid noprefix if mounting more than just cpuset subsystem
> From: Li Zefan <[email protected]>
>
> The 'noprefix' option was introduced for backwards-compatibility of
> cpuset, but actually it can be used when mounting other subsystems.
>
> This results in possibility of name collision, and now the collision can
> really happen, because we have 'stat' file in both memory and cpuacct
> subsystem:
>
> # mount -t cgroup -o noprefix,memory,cpuacct xxx /mnt
>
> Cgroup will happily mount the 2 subsystems, but only 'stat' file of memory
> subsys can be seen.
>
> We don't want users to use nopreifx, and also want to avoid name
> collision, so we change to allow noprefix only if mounting just the cpuset
> subsystem.
>
I am not sure if this is a good idea. For libcgroup, we would then be
adding a special case for just cpuset. I would rather that we allow it
either for all the subsystems or none of them.
thanks,
> Signed-off-by: Li Zefan <[email protected]>
> Cc: Paul Menage <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: Dhaval Giani <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> kernel/cgroup.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff -puN kernel/cgroup.c~cgroups-forbid-noprefix-if-mounting-more-than-just-cpuset-subsystem kernel/cgroup.c
> --- a/kernel/cgroup.c~cgroups-forbid-noprefix-if-mounting-more-than-just-cpuset-subsystem
> +++ a/kernel/cgroup.c
> @@ -842,6 +842,11 @@ static int parse_cgroupfs_options(char *
> struct cgroup_sb_opts *opts)
> {
> char *token, *o = data ?: "all";
> + unsigned long mask = (unsigned long)-1;
> +
> +#ifdef CONFIG_CPUSETS
> + mask = ~(1 << cpuset_subsys_id);
> +#endif
>
> opts->subsys_bits = 0;
> opts->flags = 0;
> @@ -886,6 +891,11 @@ static int parse_cgroupfs_options(char *
> }
> }
>
> + /* We allow noprefix only if mounting just the cpuset subsystem */
> + if (test_bit(ROOT_NOPREFIX, &opts->flags) &&
> + (opts->subsys_bits & mask))
> + return -EINVAL;
> +
> /* We can't have an empty hierarchy */
> if (!opts->subsys_bits)
> return -EINVAL;
> _
>
> Patches currently in -mm which might be from [email protected] are
>
> origin.patch
> linux-next.patch
> mm-add-swap-cache-interface-for-swap-reference.patch
> mm-modify-swap_map-and-add-swap_has_cache-flag.patch
> mm-reuse-unused-swap-entry-if-necessary.patch
> hexdump-remove-the-trailing-space.patch
> cgroups-make-messages-more-readable.patch
> cgroups-forbid-noprefix-if-mounting-more-than-just-cpuset-subsystem.patch
> devcgroup-skip-superfluous-checks-when-found-the-dev_all-elem.patch
> memcg-remove-some-redundant-checks.patch
> memcg-remove-unneeded-forward-declaration-from-schedh.patch
> memcg-fix-swap-accounting.patch
--
regards,
Dhaval
On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
>
> I am not sure if this is a good idea. For libcgroup, we would then be
> adding a special case for just cpuset. I would rather that we allow it
> either for all the subsystems or none of them.
>
libcgroup shouldn't be using the noprefix option. Its only intentded
use is to allow the legacy "cpuset" filesystem type to be mounted and
to see the same fileset as it had before the cgroups transition.
Paul
On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
> On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
> >
> > I am not sure if this is a good idea. For libcgroup, we would then be
> > adding a special case for just cpuset. I would rather that we allow it
> > either for all the subsystems or none of them.
> >
>
> libcgroup shouldn't be using the noprefix option. Its only intentded
> use is to allow the legacy "cpuset" filesystem type to be mounted and
> to see the same fileset as it had before the cgroups transition.
>
It does not. But if some user is using that option, we need to be in a
position to handle it.
I am quite happy not supporting the noprefix option in the library if it
is fine.
Thanks,
--
regards,
Dhaval
On Tue, 2 Jun 2009 23:10:42 +0530
Dhaval Giani <[email protected]> wrote:
> On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
> > On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
> > >
> > > I am not sure if this is a good idea. For libcgroup, we would then be
> > > adding a special case for just cpuset. I would rather that we allow it
> > > either for all the subsystems or none of them.
> > >
> >
> > libcgroup shouldn't be using the noprefix option. Its only intentded
> > use is to allow the legacy "cpuset" filesystem type to be mounted and
> > to see the same fileset as it had before the cgroups transition.
> >
>
> It does not. But if some user is using that option, we need to be in a
> position to handle it.
>
> I am quite happy not supporting the noprefix option in the library if it
> is fine.
>
fyi, the above discussion transitions akpm into the "confused" state.
I'll keep the patch on hold until akpm transitions back out of that
state.
* Dhaval Giani <[email protected]> [2009-06-02 23:10:42]:
> On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
> > On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
> > >
> > > I am not sure if this is a good idea. For libcgroup, we would then be
> > > adding a special case for just cpuset. I would rather that we allow it
> > > either for all the subsystems or none of them.
> > >
> >
> > libcgroup shouldn't be using the noprefix option. Its only intentded
> > use is to allow the legacy "cpuset" filesystem type to be mounted and
> > to see the same fileset as it had before the cgroups transition.
> >
>
> It does not. But if some user is using that option, we need to be in a
> position to handle it.
>
> I am quite happy not supporting the noprefix option in the library if it
> is fine.
>
I am not for supporting noprefix in the library, like Paul said it is
for legacy users who already have their applications working with
libcpuset or an equivalent library. libcgroup should not bother about
noprefix.
--
Balbir
On Tue, 2 Jun 2009 13:35:34 -0700
Andrew Morton <[email protected]> wrote:
> On Tue, 2 Jun 2009 23:10:42 +0530
> Dhaval Giani <[email protected]> wrote:
>
> > On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
> > > On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
> > > >
> > > > I am not sure if this is a good idea. For libcgroup, we would then be
> > > > adding a special case for just cpuset. I would rather that we allow it
> > > > either for all the subsystems or none of them.
> > > >
> > >
> > > libcgroup shouldn't be using the noprefix option. Its only intentded
> > > use is to allow the legacy "cpuset" filesystem type to be mounted and
> > > to see the same fileset as it had before the cgroups transition.
> > >
> >
> > It does not. But if some user is using that option, we need to be in a
> > position to handle it.
> >
> > I am quite happy not supporting the noprefix option in the library if it
> > is fine.
> >
>
> fyi, the above discussion transitions akpm into the "confused" state.
> I'll keep the patch on hold until akpm transitions back out of that
> state.
>
Traffic control...
[What "noprefix" is]
- When "noprefix" is used, the name of file under cgroup is..
[Without noprefix] ... (dir)/subsysname.filename
[With noprefix] ... (dir)/filename
Then, cpuset's files will be
[Without noprefix] ... (dir)/cpuset.xxx
[With noprefix] ... (dir)/xxx
This is for _backward compatibility_.
[Problem]
cpustat subsys has "stat" file.
memory subsys has "stat" file.
So, these cannot be mounted at the same mount point with "noprefix".
Considering arbitrary subsys can be mounted at the same point, allowing "noprefix"
other than cpuset just makes user-land complex, and "noprefix" itself is
troublesome, it breaks naming rule for cgroup files.
Then, I vote for that this patch should go.
Thanks,
-Kame
>
Dhaval Giani wrote:
> On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
>> On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
>>> I am not sure if this is a good idea. For libcgroup, we would then be
>>> adding a special case for just cpuset. I would rather that we allow it
>>> either for all the subsystems or none of them.
>>>
>> libcgroup shouldn't be using the noprefix option. Its only intentded
>> use is to allow the legacy "cpuset" filesystem type to be mounted and
>> to see the same fileset as it had before the cgroups transition.
>>
>
> It does not. But if some user is using that option, we need to be in a
> position to handle it.
>
> I am quite happy not supporting the noprefix option in the library if it
> is fine.
>
I don't think we need to support noprefix in libcgroup.
Even if we decide to support noprefix in the lib, it shouldn't be harder
to handle noprefix+cpuset only than handle noprefix+any combination of
subsystems.
On Tue, Jun 02, 2009 at 01:35:34PM -0700, Andrew Morton wrote:
> On Tue, 2 Jun 2009 23:10:42 +0530
> Dhaval Giani <[email protected]> wrote:
>
> > On Tue, Jun 02, 2009 at 09:08:04AM -0700, Paul Menage wrote:
> > > On Tue, Jun 2, 2009 at 2:09 AM, Dhaval Giani <[email protected]> wrote:
> > > >
> > > > I am not sure if this is a good idea. For libcgroup, we would then be
> > > > adding a special case for just cpuset. I would rather that we allow it
> > > > either for all the subsystems or none of them.
> > > >
> > >
> > > libcgroup shouldn't be using the noprefix option. Its only intentded
> > > use is to allow the legacy "cpuset" filesystem type to be mounted and
> > > to see the same fileset as it had before the cgroups transition.
> > >
> >
> > It does not. But if some user is using that option, we need to be in a
> > position to handle it.
> >
> > I am quite happy not supporting the noprefix option in the library if it
> > is fine.
> >
>
> fyi, the above discussion transitions akpm into the "confused" state.
> I'll keep the patch on hold until akpm transitions back out of that
> state.
I can help with that :).
We will not support noprefix in libcgroup and this patch should go in
because it fixes an obvious bug.
Thanks,
--
regards,
Dhaval