From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Knut Petersen <Knut_Petersen@t-online.de>,
linux-kernel@vger.kernel.org, reiserfs-devel@vger.kernel.org
Subject: Re: [Bug 3.4.5] reiserfs: mutex_destroy called with locked mutex
Date: Wed, 18 Jul 2012 22:33:55 +0100 [thread overview]
Message-ID: <20120718213355.GV31729@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFyHpgbJ9Pn5xn_ALkvzjQBOWog2qLbwwtrKbPHSGpa6pA@mail.gmail.com>
On Wed, Jul 18, 2012 at 02:25:02PM -0700, Linus Torvalds wrote:
> On Wed, Jul 18, 2012 at 2:20 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Wed, Jul 18, 2012 at 09:26:57AM -0700, Linus Torvalds wrote:
> >>
> >> So I don't think the freeing code could trigger, but a concurrent
> >> lookup then trying to look up the new directory (and taking the new
> >> directory i_semaphore lock) could happen, afaik.
> >
> > Umm... The thing is, we'd get WARN_ON() in iput_final() triggering in
> > that scenario before lockdep could complain.
>
> Not for the "look up directory in the dcache, and then lock that
> inode" case, afaik. You'd get the lock before iput_final(), no?
>
> So then "unlock_new_inode()" would run with the inode mutex held,
> which could explain the lockdep warning, no?
Umm.. Right you are, I was thinking about the "can the freeing code
actually trigger". OK; I'm still not sure this should go in before
-final, but it could be the reason behind those (false positive)
warnings from lockdep. Could probably step into something nasty
around e.g. writeback or quota, so maybe it's worth doing just in
case...
It's definitely the right thing to do wrt not giving the rest of the
VFS/VM nasty surprises - everything might work correctly with IO
coming on such still-I_NEW-and-locked inode, but that's not a case
that will be often considered when modifying code. The only questions
are "is this the WARN_ON() Knut had stepped on" (and I agree with your
scenario now) and "is it critical enough to shove it into the tree
less than a week before -final". Up to you...
next prev parent reply other threads:[~2012-07-18 21:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-18 5:26 [Bug 3.4.5] reiserfs: mutex_destroy called with locked mutex Knut Petersen
2012-07-18 16:26 ` Linus Torvalds
2012-07-18 21:20 ` Al Viro
2012-07-18 21:25 ` Linus Torvalds
2012-07-18 21:33 ` Al Viro [this message]
2012-07-18 22:37 ` Linus Torvalds
2012-07-19 4:28 ` Al Viro
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=20120718213355.GV31729@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=Knut_Petersen@t-online.de \
--cc=linux-kernel@vger.kernel.org \
--cc=reiserfs-devel@vger.kernel.org \
--cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).