lkml.org 
[lkml]   [2009]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2009-01-23 18:53    [W:0.057 / U:0.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site