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@xxxxxxxxxxxx> wrote:
> > DCACHE_DISCONECTED dentries aren't always getting destroyed as soon as
> > I'd have expected in the NFSv4 case.
> >
> > I'm not sure what the right fix is; any ideas? ÂThe below at least
> > demonstrates the problem.
> >
> > --b.
> >
> > commit bb125ffbe5fc3f80ac7a5b20f51cc542c175cd49
> > Author: J. Bruce Fields <bfields@xxxxxxxxxx>
> > Date: Â Thu Nov 11 19:22:00 2010 -0500
> >
> > Â Âdput: free DCACHE_DISCONNECTED dentries sooner
> >
> > Â ÂDCACHE_DISCONECTED dentries are normally left around for the benefit of
> > Â Âfuture nfsd operations. ÂBut there's no point keeping them around once
> > Â Âthe inode has been deleted.
> >
> > Â ÂWithout this patch
> >
> > Â Â Â Âclient$ mount -tnfs4 server:/export/ /mnt/
> > Â Â Â Âclient$ tail -f /mnt/FOO
> > Â Â Â Â...
> > Â Â Â Âserver$ df -i /export
> > Â Â Â Âserver$ rm /export/FOO
> > Â Â Â Â(^C the tail -f)
> > Â Â Â Âserver$ df -i /export
> > Â Â Â Âserver$ echo 2 >/proc/sys/vm/drop_caches
> > Â Â Â Âserver$ df -i /export
> >
> > Â Âthe df's will show that the inode is not freed on the filesystem until
> > Â Âthe last step, when it could have been freed after killing the client's
> > Â Âtail -f. ÂOn-disk data won't be deallocated either, leading to possible
> > Â Âspurious ENOSPC.
> >
> > Â ÂThis occurs because when the client does the close, it arrives in a
> > Â Âcompound with a putfh and a close, processed like:
> >
> > Â Â Â Â- 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?)
> 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.
But nfsd already holds an open file in this case, and you could argue
that it should be using that from the start.
So, we could modify nfsd to add a hash mapping filehandles to the filp's
that it knows about, and have nfsd consult that hash before calling
dentry_to_fh.
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
