On Sun, Nov 18, 2018 at 08:23:42PM -0800, Andrei Vagin wrote:
> On Fri, Sep 21, 2018 at 05:33:01PM +0100, David Howells wrote:
> > @@ -1993,57 +2009,53 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask, int ref_flags)
> > return ret;
> > }
> >
> > -struct dentry *cgroup_do_mount(struct file_system_type *fs_type, int flags,
> > - struct cgroup_root *root, unsigned long magic,
> > - struct cgroup_namespace *ns)
> > +int cgroup_do_get_tree(struct fs_context *fc)
> > {
> > - struct dentry *dentry;
> > - bool new_sb;
> > + struct cgroup_fs_context *ctx = cgroup_fc2context(fc);
> > + int ret;
> >
> > - dentry = kernfs_mount(fs_type, flags, root->kf_root, magic, &new_sb);
> > + ctx->kfc.root = ctx->root->kf_root;
> > +
> > + ret = kernfs_get_tree(fc);
> > + if (ret < 0)
> > + goto out_cgrp;
> >
> > /*
> > * In non-init cgroup namespace, instead of root cgroup's dentry,
> > * we return the dentry corresponding to the cgroupns->root_cgrp.
> > */
> > - if (!IS_ERR(dentry) && ns != &init_cgroup_ns) {
> > + if (ctx->ns != &init_cgroup_ns) {
> > struct dentry *nsdentry;
> > struct cgroup *cgrp;
> >
> > mutex_lock(&cgroup_mutex);
> > spin_lock_irq(&css_set_lock);
> >
> > - cgrp = cset_cgroup_from_root(ns->root_cset, root);
> > + cgrp = cset_cgroup_from_root(ctx->ns->root_cset, ctx->root);
> >
> > spin_unlock_irq(&css_set_lock);
> > mutex_unlock(&cgroup_mutex);
> >
> > - nsdentry = kernfs_node_dentry(cgrp->kn, dentry->d_sb);
> > - dput(dentry);
> > - dentry = nsdentry;
> > + nsdentry = kernfs_node_dentry(cgrp->kn, fc->root->d_sb);
> > + if (IS_ERR(nsdentry))
> > + return PTR_ERR(nsdentry);
> > + dput(fc->root);
> > + fc->root = nsdentry;
> > }
> >
> > - if (IS_ERR(dentry) || !new_sb)
> > - cgroup_put(&root->cgrp);
>
> I don't see where this cgroup_put() has been moved.
David, have you looked at this problem? It isn't fixed in linux-next
yet.
https://travis-ci.org/avagin/linux/jobs/463960763
Thanks,
Andrei
>
> With this patch, the next script works only once, on the second attempt
> it hangs up on mounting a cgroup file system.
>
> This is the only suspicious place in this patch what I have found.
>
> [root@fc24 ~]# cat fs-vs-cg
> d=$(mktemp -d /tmp/cg.XXXXXX)
> mkdir $d/a
> mkdir $d/b
> mount -t cgroup -o none,name=xxxx xxx $d/a
> mount -t cgroup -o none,name=xxxx xxx $d/b
> umount $d/a
> umount $d/b
>
> [root@fc24 ~]# unshare -m --propagation private bash -x fs-vs-cg
> ++ mktemp -d /tmp/cg.XXXXXX
> + d=/tmp/cg.yUfagS
> + mkdir /tmp/cg.yUfagS/a
> + mkdir /tmp/cg.yUfagS/b
> + mount -t cgroup -o none,name=xxxx xxx /tmp/cg.yUfagS/a
> + mount -t cgroup -o none,name=xxxx xxx /tmp/cg.yUfagS/b
> + umount /tmp/cg.yUfagS/a
> + umount /tmp/cg.yUfagS/b
> [root@fc24 ~]# unshare -m --propagation private bash -x fs-vs-cg
> ++ mktemp -d /tmp/cg.XXXXXX
> + d=/tmp/cg.ippWUn
> + mkdir /tmp/cg.ippWUn/a
> + mkdir /tmp/cg.ippWUn/b
> + mount -t cgroup -o none,name=xxxx xxx /tmp/cg.ippWUn/a
> ^Z
> [1]+ Stopped unshare -m --propagation private bash -x fs-vs-cg
>
> [root@fc24 ~]# ps
> PID TTY TIME CMD
> 556 pts/0 00:00:00 bash
> 591 pts/0 00:00:00 bash
> 595 pts/0 00:00:00 mount
> 596 pts/0 00:00:00 ps
>
> [root@fc24 ~]# bg
> [1]+ unshare -m --propagation private bash -x fs-vs-cg &
>
> [root@fc24 ~]# cat /proc/595/stack
> [<0>] msleep+0x38/0x40
> [<0>] cgroup1_get_tree+0x4e1/0x72c
> [<0>] vfs_get_tree+0x5e/0x140
> [<0>] do_mount+0x326/0xc70
> [<0>] ksys_mount+0xba/0xd0
> [<0>] __x64_sys_mount+0x21/0x30
> [<0>] do_syscall_64+0x60/0x210
> [<0>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [<0>] 0xffffffffffffffff
>
