Messages in this thread | | | Date | Fri, 23 Jan 2009 18:50:28 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] cgroup: Fix root_count when mount fails due to busy subsystem |
| |
* Paul Menage <menage@google.com> wrote:
> On Fri, Jan 23, 2009 at 2:22 AM, Ingo Molnar <mingo@elte.hu> 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
| |