[Devel] Re: [PATCH] cgroup: fix reverse unlock sequence in cgroup_get_sb

Zefan Li lizf.kernel at gmail.com
Tue Jul 21 04:41:23 PDT 2009


2009/7/21, Balbir Singh <balbir at linux.vnet.ibm.com>:
>
> * Xiaotian Feng <dfeng at redhat.com> [2009-07-21 18:25:26]:
>
> > In cgroup_get_sb, the lock sequence is:
> >       mutex_lock(&inode->i_mutex);
> >       mutex_lock(&cgroup->mutex);
> > so the last unlock sequence should be:
> >       mutex_unlock(&cgroup->mutex);
> >       mutex_unlock(&inode->i_mutex);
> >
> > Signed-off-by: Xiaotian Feng <dfeng at redhat.com>
> > ---
> >  kernel/cgroup.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> > index 3737a68..11ef162 100644
> > --- a/kernel/cgroup.c
> > +++ b/kernel/cgroup.c
> > @@ -1140,8 +1140,8 @@ static int cgroup_get_sb(struct file_system_type
> *fs_type,
> >               BUG_ON(root->number_of_cgroups != 1);
> >
> >               cgroup_populate_dir(root_cgrp);
> > -             mutex_unlock(&inode->i_mutex);
> >               mutex_unlock(&cgroup_mutex);
> > +             mutex_unlock(&inode->i_mutex);
> >       }
> >
>
> Seems reasonable to me. You might also want to mention that elsewhere
> the sequence is unlock cgroup_mutex followed by inode->i_mutex.
>
> Acked-by: Balbir Singh balbir at linux.vnet.ibm.com


No, the unlock order is irrelevant. It's the lock order that matters. So
this patch
fixes nothing.

Xiaotian, you didn't run into deadlock, did you?

--
Li Zefan
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers




More information about the Devel mailing list