Re: [PATCH] cgroup: Fix root_count when mount fails due to busysubsystem

[Ingo Molnar]

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/