lkml.org 
[lkml]   [2008]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@cn.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@cn.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
>>
>>
>



\
 
 \ /
  Last update: 2008-12-02 04:37    [W:0.046 / U:0.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site