On Thu, Jan 03, 2019 at 08:32:29AM +0000, Al Viro wrote: > On Wed, Jan 02, 2019 at 07:54:26PM -0800, Andrei Vagin wrote: > > [I'm thoroughly sick of refcounting in that thing, TBH ;-/] feel your pain... > > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > > index a19f0fec9d82..fe67b5e81f9a 100644 > > --- a/kernel/cgroup/cgroup.c > > +++ b/kernel/cgroup/cgroup.c > > @@ -2019,7 +2019,7 @@ int cgroup_do_get_tree(struct fs_context *fc) > > > > ret = kernfs_get_tree(fc); > > if (ret < 0) > > - goto out_cgrp; > > + return ret; > > Why does that case avoid needing cgroup_put()? Note, BTW, that we > also have this: The origin patch which added this cgroup_put says that it is needed because mount() and kill_sb() is not a one-to-one match. sb is created in kernfs_get_tree(), so I decided that this cgroup_put() is needed only after kernfs_get_tree(). commit c6b3d5bcd67c75961a1e8b9564d1475c0f194a84 Author: Li Zefan <lizefan@xxxxxxxxxx> Date: Fri Apr 4 17:14:41 2014 +0800 cgroup: fix top cgroup refcnt leak As mount() and kill_sb() is not a one-to-one match, If we mount the same cgroupfs in serveral mount points, and then umount all of them, kill_sb() will be called only once. > /* > * Destroy a cgroup filesystem context. > */ > static void cgroup_fs_context_free(struct fs_context *fc) > { > struct cgroup_fs_context *ctx = cgroup_fc2context(fc); > > kfree(ctx->name); > kfree(ctx->release_agent); > if (ctx->root) > cgroup_put(&ctx->root->cgrp); > put_cgroup_ns(ctx->ns); > kernfs_free_fs_context(fc); > kfree(ctx); > } > > which also needs to be taken into account. we have unconditional cgroup_get in cgroup1_get_tree() to match this put, but we need to check error paths.