* Paul Menage <menage@xxxxxxxxxx> wrote:
> 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.
that should be done via proper placement of err_* labels. An assymetric
exit path like the one you did is always the sign of bad code structure.
Ingo
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
