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] [day] [month] [year] [list]

Message-ID: <4934ACD1.9050208@cn.fujitsu.com>
Date:	Tue, 02 Dec 2008 11:34:41 +0800
From:	Li Zefan <lizf@...fujitsu.com>
To:	Paul Menage <menage@...gle.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

Paul Menage wrote:
> 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)"?
> 

I noticed the comments for root_list and for_each_root() conflicts with
the code. But I don't have strong option whether to change the code or
change the comments. If you vote for the latter, I'll rewrite the patch.

> 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.
> 

Or rename 'roots' to 'active_roots'?

> 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/