On Wed, Jan 02, 2019 at 04:43:39PM -0800, Andrei Vagin wrote:
> On Thu, Jan 03, 2019 at 12:26:23AM +0000, David Howells wrote:
> > Andrei Vagin <avagin@xxxxxxxxx> wrote:
> >
> > > It looks like the c6b3d5bcd67c ("cgroup: fix top cgroup refcnt leak")
> > > commit was reverted by mistake.
> > >
> > > $ mkdir /tmp/cgroup
> > > $ mkdir /tmp/cgroup2
> > > $ mount -t cgroup -o none,name=test test /tmp/cgroup
> > > $ mount -t cgroup -o none,name=test test /tmp/cgroup2
> > > $ umount /tmp/cgroup
> > > $ umount /tmp/cgroup2
> > > $ cat /proc/self/cgroup | grep test
> > > 12:name=test:/
> > >
> > > You can see the test cgroup was not freed.
> > >
> > > Cc: Li Zefan <lizefan@xxxxxxxxxx>
> > > Fixes: aea3f2676c83 ("kernfs, sysfs, cgroup, intel_rdt: Support fs_context")
> > > Signed-off-by: Andrei Vagin <avagin@xxxxxxxxx>
> > > ---
> > >
> > > v2: clean up code and add the vfs/for-next tag
> > > v3: fix a reference leak when kernfs_node_dentry fails
> > > v4: call deactivate_locked_super() in a error case
> > > v5: don't dereference fc->root after dput()
> > >
> > > kernel/cgroup/cgroup.c | 25 ++++++++++++++++++-------
> > > 1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > This patch doesn't work either.
>
> I'm sorry, but we can't say anything about this patch now, because it
> looks like recent changes in vfs-next break something else here...
I found a reason why this patch doesn't work on Al's vfs/for-next:
[avagin@laptop linux]$ git diff 40effd960becd8a355b7aafc789712afd64f5759..vfs/for-next kernel/cgroup/cgroup-v1.c | grep -B 5 -A 5 cgroup_get
/*
@@ -1280,8 +1285,8 @@ int cgroup1_get_tree(struct fs_context *fc)
mutex_lock(&cgroup_mutex);
percpu_ref_reinit(&root->cgrp.self.refcnt);
mutex_unlock(&cgroup_mutex);
+ cgroup_get(&root->cgrp);
}
- cgroup_get(&root->cgrp);
/*
* If @pinned_sb, we're reusing an existing root and holding an
40effd960becd8a355b7aafc789712afd64f5759 is the previous head of vfs/for-next
I reverted this hunk, applied my patch and all criu test passed.
>
> >
> > percpu ref (css_release) <= 0 (0) after switching to atomic
> > RIP: 0010:percpu_ref_switch_to_atomic_rcu+0x90/0x1a0
> >
> > Btw, note that the subject says "v4" but the changelog says "v5".
>
> It is v5.
>
> >
> > David
