From: Nick Piggin <npiggin@gmail.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: lifetime of DCACHE_DISCONECTED dentries
Date: Mon, 29 Nov 2010 14:56:22 +1100 [thread overview]
Message-ID: <AANLkTinMq1W1vx-jZnf7cb1cAnh9OyoVRYtVS7zpujfs@mail.gmail.com> (raw)
In-Reply-To: <AANLkTim1QjTwOZ4SHdMy8Y6TOAnaOYtTwgwHXfH5H5Xn@mail.gmail.com>
On Tue, Nov 16, 2010 at 5:45 PM, Nick Piggin <npiggin@gmail.com> wrote:
> On Tue, Nov 16, 2010 at 4:48 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
>> On Sat, Nov 13, 2010 at 10:53:12PM +1100, Nick Piggin wrote:
>>> On Sat, Nov 13, 2010 at 5:43 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
>
>>> > - putfh: look up the filehandle. The only alias found for the
>>> > inode will be DCACHE_UNHASHED alias referenced by the filp
>>> > associated with the nfsd open. d_obtain_alias() doesn't like
>>> > this, so it creates a new DCACHE_DISCONECTED dentry and
>>> > returns that instead.
>>>
>>> This seems to be where the thing goes wrong. It isn't a hashed dentry at
>>> this point here, so d_obtain_alias should not be making one.
>>
>> Sounds sensible. (But can you think of any actual bugs that will result
>> from trying to add a new hashed dentry in this case?)
>
> Well, this one? :)
>
>
>>> I think the inode i_nlink games are much more appropriate on this side of
>>> the equation, rather than the dput side (after all, d_obtain_alias is setting
>>> up an alias for the inode).
>>>
>>> Can you even put the link check into __d_find_alias?
>>>
>>> - if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) {
>>> + if (S_ISDIR(inode->i_mode) || !inode->i_nlink ||
>>> !d_unhashed(alias)) {
>>>
>>> Something like that?
>>
>> The immediate result of that would be for the close rpc (or any rpc's
>> sent after the file was unlinked) to fail with ESTALE.
>
> Why is that? Seems like it would be a bug, because a hashed dentry may
> be unhashed at any time concurrently to nfsd operation, so it should be
> able to tolerate that so long as it has a ref on the inode?
Ping? Did you work out why nfs fails with ESTALE in that case? It seems
to work in my testing (and do the right thing with freeing the inode).
next prev parent reply other threads:[~2010-11-29 3:56 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-12 18:43 lifetime of DCACHE_DISCONECTED dentries J. Bruce Fields
2010-11-13 11:53 ` Nick Piggin
2010-11-15 17:48 ` J. Bruce Fields
2010-11-16 6:45 ` Nick Piggin
2010-11-29 3:56 ` Nick Piggin [this message]
2010-11-29 19:32 ` J. Bruce Fields
2010-11-30 1:00 ` Nick Piggin
2010-11-30 18:39 ` J. Bruce Fields
2010-12-03 22:33 ` [PATCH] nfsd4: allow __d_obtain_alias() to return unhashed dentries J. Bruce Fields
2010-12-13 5:19 ` Nick Piggin
2010-12-14 22:01 ` J. Bruce Fields
2010-12-17 17:53 ` [PATCH] fs/dcache: use standard list macro for d_find_alias J. Bruce Fields
2010-12-17 18:00 ` [PATCH 2/2] fs/dcache: allow __d_obtain_alias() to return unhashed dentries J. Bruce Fields
2010-12-18 2:01 ` Nick Piggin
2010-12-18 16:16 ` J. Bruce Fields
2010-12-19 14:53 ` Nick Piggin
2010-12-27 23:46 ` [PATCH] " J. Bruce Fields
2011-01-18 20:45 ` J. Bruce Fields
2011-01-18 22:02 ` Nick Piggin
2011-01-18 22:08 ` J. Bruce Fields
2011-03-08 18:13 ` J. Bruce Fields
2011-03-10 10:58 ` Al Viro
2011-03-11 4:07 ` NeilBrown
2012-02-14 17:03 ` J. Bruce Fields
2012-02-15 16:56 ` J. Bruce Fields
2012-02-16 3:06 ` NeilBrown
2012-02-16 11:51 ` J. Bruce Fields
2012-02-16 16:08 ` J. Bruce Fields
2012-02-16 22:30 ` J. Bruce Fields
2012-02-17 16:34 ` Peng Tao
2012-03-13 20:55 ` J. Bruce Fields
2012-03-13 20:58 ` [PATCH 1/2] vfs: stop d_splice_alias creating directory aliases J. Bruce Fields
2012-03-13 20:58 ` [PATCH 2/2] vfs: remove unused __d_splice_alias argument J. Bruce Fields
2012-02-20 2:55 ` [PATCH] fs/dcache: allow __d_obtain_alias() to return unhashed dentries NeilBrown
2012-02-29 23:10 ` J. Bruce Fields
2012-06-28 13:59 ` J. Bruce Fields
2012-06-29 20:10 ` J. Bruce Fields
2012-06-29 20:29 ` J. Bruce Fields
2012-07-01 23:15 ` NeilBrown
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=AANLkTinMq1W1vx-jZnf7cb1cAnh9OyoVRYtVS7zpujfs@mail.gmail.com \
--to=npiggin@gmail.com \
--cc=bfields@fieldses.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.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 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).