From: Linus Torvalds <torvalds@linux-foundation.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
hch@infradead.org, Trond.Myklebust@netapp.com
Subject: Re: [PATCH 1/2] vfs: dcache: fix deadlock in tree traversal
Date: Tue, 18 Sep 2012 08:56:46 -0700 [thread overview]
Message-ID: <CA+55aFxJKQMjGm00kBpj30twj1w60Mx4C0mXTJtHiJPOpu-iMA@mail.gmail.com> (raw)
In-Reply-To: <87obl3z8jn.fsf@tucsk.pomaz.szeredi.hu>
On Tue, Sep 18, 2012 at 7:53 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> Yes, with the test cases that IBM were using it is DCACHE_DISCONNECTED
> case that triggers the double-lock. Trond was misusing
> DCACHE_DISCONNECTED and this made the failure in try_to_ascend() much
> more likely (and bogus). But there is a case, which is triggered rarely
> if ever, when try_to_ascend() failure with rename_lock held is perfectly
> valid.
Ok. The whole DCACHE_DISCONNECTED logic there is clearly bogus and
results in endless loops if that case then ever triggers, but you fix
that in the second patch.
HOWEVER. Why introduce that new DCACHE_KILLED flag at all? Wouldn't it
be much better to just check whether the dentry is hashed instead of
introducing a new flag for this case? Couldn't we just check for
"d_unhashed()"?
Anyway, I don't hate the 2/2 thing, but these are clearly related, and
I'd like to understand why we had that odd DCACHE_DISCONNECTED test in
the first place when there *seems* to be more straightforward
approaches to it?
Linus
next prev parent reply other threads:[~2012-09-18 15:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-17 20:23 [PATCH 1/2] vfs: dcache: fix deadlock in tree traversal Miklos Szeredi
2012-09-17 20:31 ` [PATCH 2/2] vfs: dcache: use DCACHE_DENTRY_KILLED instead of DCACHE_DISCONNECTED in d_kill() Miklos Szeredi
2012-09-17 20:39 ` [PATCH 1/2] vfs: dcache: fix deadlock in tree traversal Al Viro
2012-09-17 22:09 ` Linus Torvalds
2012-09-18 14:53 ` Miklos Szeredi
2012-09-18 15:56 ` Linus Torvalds [this message]
2012-09-18 16:27 ` Miklos Szeredi
2012-09-18 18:22 ` Linus Torvalds
2012-09-18 20:35 ` [PATCH] trivial select_parent documentation fix J. Bruce Fields
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=CA+55aFxJKQMjGm00kBpj30twj1w60Mx4C0mXTJtHiJPOpu-iMA@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=Trond.Myklebust@netapp.com \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=viro@zeniv.linux.org.uk \
/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).