On Tue, Jan 18, 2011 at 05:08:17PM -0500, J. Bruce Fields wrote:
> On Wed, Jan 19, 2011 at 09:02:59AM +1100, Nick Piggin wrote:
> > On Wed, Jan 19, 2011 at 7:45 AM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> > > @@ -1521,6 +1521,28 @@ struct dentry * d_alloc_root(struct inode * root_inode)
> > > Â}
> > > ÂEXPORT_SYMBOL(d_alloc_root);
> > >
> > > +static struct dentry * __d_find_any_alias(struct inode *inode)
> > > +{
> > > + Â Â Â struct dentry *alias;
> > > +
> > > + Â Â Â if (list_empty(&inode->i_dentry))
> > > + Â Â Â Â Â Â Â return NULL;
> > > + Â Â Â alias = list_first_entry(&inode->i_dentry, struct dentry, d_alias);
> > > + Â Â Â __dget(alias);
> > > + Â Â Â return alias;
> > > +}
> > > +
> > > +static struct dentry * d_find_any_alias(struct inode *inode)
> > > +{
> > > + Â Â Â struct dentry *de;
> > > +
> > > + Â Â Â spin_lock(&inode->i_lock);
> >
> > Yes, i_dentry/d_alias is protected by i_lock, so it looks fine.
>
> OK, thanks; I'm assuming it's OK to re-add your ack in that case. Al,
> could you apply? (Or if this should go in through an nfsd tree, or some
> other way, let me know.)
Al, do you have this in your queue to look at? Need me to resend? Or
should it take some other route?
--b.
>
> --b.
>
> From: J. Bruce Fields <bfields@xxxxxxxxxx>
> Date: Fri, 17 Dec 2010 09:05:04 -0500
> Subject: [PATCH] fs/dcache: allow __d_obtain_alias() to return unhashed dentries
>
> Without this patch, inodes are not promptly freed on last close of an
> unlinked file by an nfs client:
>
> Â Â Â Â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
> this, so it creates a new DCACHE_DISCONECTED dentry and
> returns that instead.
> - close: closes the existing filp, which is destroyed
> immediately by dput() since it's DCACHE_UNHASHED.
> - end of the compound: release the reference
> to the current filehandle, and dput() the new
> DCACHE_DISCONECTED dentry, which gets put on the
> unused list instead of being destroyed immediately.
>
> Nick Piggin suggested fixing this by allowing d_obtain_alias to return
> the unhashed dentry that is referenced by the filp, instead of making it
> create a new dentry.
>
> Leave __d_find_alias() alone to avoid changing behavior of other
> callers.
>
> Also nfsd doesn't need all the checks of __d_find_alias(); any dentry,
> hashed or unhashed, disconnected or not, should work.
>
> Acked-by: Nick Piggin <npiggin@xxxxxxxxx>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> ---
> fs/dcache.c | 26 ++++++++++++++++++++++++--
> 1 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 9f493ee..2849258 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1521,6 +1521,28 @@ struct dentry * d_alloc_root(struct inode * root_inode)
> }
> EXPORT_SYMBOL(d_alloc_root);
>
> +static struct dentry * __d_find_any_alias(struct inode *inode)
> +{
> + struct dentry *alias;
> +
> + if (list_empty(&inode->i_dentry))
> + return NULL;
> + alias = list_first_entry(&inode->i_dentry, struct dentry, d_alias);
> + __dget(alias);
> + return alias;
> +}
> +
> +static struct dentry * d_find_any_alias(struct inode *inode)
> +{
> + struct dentry *de;
> +
> + spin_lock(&inode->i_lock);
> + de = __d_find_any_alias(inode);
> + spin_unlock(&inode->i_lock);
> + return de;
> +}
> +
> +
> /**
> * d_obtain_alias - find or allocate a dentry for a given inode
> * @inode: inode to allocate the dentry for
> @@ -1550,7 +1572,7 @@ struct dentry *d_obtain_alias(struct inode *inode)
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> - res = d_find_alias(inode);
> + res = d_find_any_alias(inode);
> if (res)
> goto out_iput;
>
> @@ -1563,7 +1585,7 @@ struct dentry *d_obtain_alias(struct inode *inode)
>
>
> spin_lock(&inode->i_lock);
> - res = __d_find_alias(inode, 0);
> + res = __d_find_any_alias(inode);
> if (res) {
> spin_unlock(&inode->i_lock);
> dput(tmp);
> --
> 1.7.1
>
--
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
