From: Al Viro <viro@ZenIV.linux.org.uk>
To: Andrey Wagin <avagin@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Tejun Heo <tj@kernel.org>,
Li Zefan <lizefan@huawei.com>
Subject: Re: linux-next: cgroup_mount() falls asleep forever
Date: Wed, 24 Sep 2014 21:16:31 +0100 [thread overview]
Message-ID: <20140924201629.GH7996@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20140924185213.GB7996@ZenIV.linux.org.uk>
On Wed, Sep 24, 2014 at 07:52:14PM +0100, Al Viro wrote:
> Could somebody explain WTF is the whole construction trying to do? Not
> to mention anything else, what *does* this pinning a superblock protect
> from? Suppose we have a superblock for the same root with non-NULL ns
> and _that_ gets killed. We get hit by the same
> percpu_ref_kill(&root->cgrp.self.refcnt);
> so what's the point of pinned_sb? Might as well have just bumped the
> refcount, superblock or no superblock. And no, delaying that kernfs_kill_sb()
> does you no good whatsoever - again, pinned_sb might have nothing to do with
> the superblock we are after.
Hrm... Scratch the comments re "different superblock" (we are passing NULL
ns in that kernfs_mount() below), but... then WTF is that thing trying to
do? OK, you've got your active reference to a superblock from
kernfs_pin_sb(). You grab root->cgrp.self.refcnt. Suppose it also worked.
Now what? You drop cgroup_mutex and proceed to kernfs_mount(). Which
calls sget(), looking for exact same thing as your kernfs_pin_sb(). So it
finds the same superblock and grab it for you (with ->s_umount held).
At which point you drop root->cgrp.self.refcnt and drop the active reference
you've got from kernfs_pin_sb(). Pardon me, but what's the point of that
song and dance? Who else can make that attempt at grabbing
root->cgrp.self.refcnt fail?
BTW, what happens if kernfs_fill_super() fails? You get cgroup_kill_sb()
triggered by deactivate_locked_super(), which calls kernfs_kill_sb(), which
does kernfs_put(). Balancing the kernfs_get() we'd never got around to
in kernfs_fill_super()...
next prev parent reply other threads:[~2014-09-24 20:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-24 10:31 linux-next: cgroup_mount() falls asleep forever Andrey Wagin
2014-09-24 14:29 ` Andrey Wagin
2014-09-24 18:31 ` Cong Wang
2014-09-24 18:52 ` Al Viro
2014-09-24 19:24 ` Tejun Heo
2014-09-25 2:47 ` Al Viro
2014-09-25 3:25 ` Tejun Heo
2014-09-25 10:23 ` Zefan Li
2014-09-25 15:14 ` Tejun Heo
2014-09-24 20:16 ` Al Viro [this message]
2014-09-24 16:13 ` Andrey Wagin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140924201629.GH7996@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=avagin@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.