linux-kernel - Re: [PATCH 1/3] cgroups: make root_list contains active hierarchies only

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: <6599ad830812011731w75eda049tc809856041ecb707@mail.gmail.com>
Date:	Mon, 1 Dec 2008 17:31:10 -0800
From:	Paul Menage <menage@...gle.com>
To:	Li Zefan <lizf@...fujitsu.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux Containers <containers@...ts.linux-foundation.org>
Subject: Re: [PATCH 1/3] cgroups: make root_list contains active hierarchies 
	only

On Fri, Nov 28, 2008 at 2:02 AM, Li Zefan <lizf@...fujitsu.com> wrote:
> Don't link rootnode to the root list, so root_list contains active
> hierarchies only as the comment indicates. And rename for_each_root()
> to for_each_active_root().

Why is that change an improvement? If you're concerned about the
comment's accuracy then it would be better to just change it to say "A
list running through all hierarchies (including the inactive subsystem
hierarchy)"?

Also, with this change, "root_count" is no longer the length of the
"roots" list, but instead is length + 1, which doesn't seem good.

Paul

>
> Also remove redundant check in cgroup_kill_sb().
>
> Signed-off-by: Li Zefan <lizf@...fujitsu.com>
> ---
>  kernel/cgroup.c |   19 +++++++------------
>  1 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index fe00b3b..33ba756 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -84,7 +84,7 @@ struct cgroupfs_root {
>        /* Tracks how many cgroups are currently defined in hierarchy.*/
>        int number_of_cgroups;
>
> -       /* A list running through the mounted hierarchies */
> +       /* A list running through the active hierarchies */
>        struct list_head root_list;
>
>        /* Hierarchy-specific flags */
> @@ -149,8 +149,8 @@ static int notify_on_release(const struct cgroup *cgrp)
>  #define for_each_subsys(_root, _ss) \
>  list_for_each_entry(_ss, &_root->subsys_list, sibling)
>
> -/* for_each_root() allows you to iterate across the active hierarchies */
> -#define for_each_root(_root) \
> +/* for_each_active_root() allows you to iterate across the active hierarchies */
> +#define for_each_active_root(_root) \
>  list_for_each_entry(_root, &roots, root_list)
>
>  /* the list of cgroups eligible for automatic release. Protected by
> @@ -1114,10 +1114,9 @@ static void cgroup_kill_sb(struct super_block *sb) {
>        }
>        write_unlock(&css_set_lock);
>
> -       if (!list_empty(&root->root_list)) {
> -               list_del(&root->root_list);
> -               root_count--;
> -       }
> +       list_del(&root->root_list);
> +       root_count--;
> +
>        mutex_unlock(&cgroup_mutex);
>
>        kfree(root);
> @@ -2561,7 +2560,6 @@ int __init cgroup_init_early(void)
>        INIT_HLIST_NODE(&init_css_set.hlist);
>        css_set_count = 1;
>        init_cgroup_root(&rootnode);
> -       list_add(&rootnode.root_list, &roots);
>        root_count = 1;
>        init_task.cgroups = &init_css_set;
>
> @@ -2668,15 +2666,12 @@ static int proc_cgroup_show(struct seq_file *m, void *v)
>
>        mutex_lock(&cgroup_mutex);
>
> -       for_each_root(root) {
> +       for_each_active_root(root) {
>                struct cgroup_subsys *ss;
>                struct cgroup *cgrp;
>                int subsys_id;
>                int count = 0;
>
> -               /* Skip this hierarchy if it has no active subsystems */
> -               if (!root->actual_subsys_bits)
> -                       continue;
>                seq_printf(m, "%lu:", root->subsys_bits);
>                for_each_subsys(root, ss)
>                        seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
> --
> 1.5.4.rc3
>
>
--
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/