On Fri, Jan 23, 2009 at 2:22 AM, Ingo Molnar <mingo@xxxxxxx> wrote: >> --- a/kernel/cgroup.c >> +++ b/kernel/cgroup.c >> @@ -1115,8 +1115,10 @@ static void cgroup_kill_sb(struct super_block *sb) { >> } >> write_unlock(&css_set_lock); >> >> - list_del(&root->root_list); >> - root_count--; >> + if (!list_empty(&root->root_list)) { >> + list_del(&root->root_list); >> + root_count--; >> + } > > That's ugly. It is _much_ cleaner to always keep the link head consistent > - i.e. initialize it with INIT_LIST_HEAD() It is initialized with INIT_LIST_HEAD(). > and then remove from it via > list_del_init(). There's not much point doing list_del_init() rather than list_del() here since we're about to delete the root. > > That way the error path will do the right thing automatically, and there's > no need for that ugly "if !list_empty" construct either. The important part here is avoiding decrementing root_count. So the code could equally be: if (!list_empty(&root->root_list)) { root_count--; } list_del(&root->root_list); but what I have in this patch seems more straightforward. It's actually how the code used to be before it was removed as a "redundant" check by a patch that I unfortunately didn't get a chance to read properly (or Ack) because I was too snowed under with other work. Paul _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers