Re: [PATCH 1/2] nfs/vfs: discard d_exact_alias()

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-02-26 at 17:18 +1100, NeilBrown wrote:
> d_exact_alias() is a descendent of d_add_unique() which was introduced
> 20 years ago mostly likely to work around problems with NFS servers of
> the time.  It is now not used in several situations were it was
> originally needed and there have been no reports of problems -
> presumably the old NFS servers have been improved.  This only place it
> is now use is in NFSv4 code and the old problematic servers are thought
> to have been v2/v3 only.
> 
> There is no clear benefit in reusing a unhashed() dentry which happens
> to have the same name as the dentry we are adding.
> 
> So this patch removes d_exact_alias() and the one place that it is used.
> 
> Cc: Trond Myklebust <trond.myklebust@hammerspace.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/dcache.c            | 46 ------------------------------------------
>  fs/nfs/nfs4proc.c      |  4 +---
>  include/linux/dcache.h |  1 -
>  3 files changed, 1 insertion(+), 50 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index e3634916ffb9..726a5be2747b 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2687,52 +2687,6 @@ void d_add(struct dentry *entry, struct inode *inode)
>  }
>  EXPORT_SYMBOL(d_add);
>  
> -/**
> - * d_exact_alias - find and hash an exact unhashed alias
> - * @entry: dentry to add
> - * @inode: The inode to go with this dentry
> - *
> - * If an unhashed dentry with the same name/parent and desired
> - * inode already exists, hash and return it.  Otherwise, return
> - * NULL.
> - *
> - * Parent directory should be locked.
> - */
> -struct dentry *d_exact_alias(struct dentry *entry, struct inode *inode)
> -{
> -	struct dentry *alias;
> -	unsigned int hash = entry->d_name.hash;
> -
> -	spin_lock(&inode->i_lock);
> -	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
> -		/*
> -		 * Don't need alias->d_lock here, because aliases with
> -		 * d_parent == entry->d_parent are not subject to name or
> -		 * parent changes, because the parent inode i_mutex is held.
> -		 */
> -		if (alias->d_name.hash != hash)
> -			continue;
> -		if (alias->d_parent != entry->d_parent)
> -			continue;
> -		if (!d_same_name(alias, entry->d_parent, &entry->d_name))
> -			continue;
> -		spin_lock(&alias->d_lock);
> -		if (!d_unhashed(alias)) {
> -			spin_unlock(&alias->d_lock);
> -			alias = NULL;
> -		} else {
> -			dget_dlock(alias);
> -			__d_rehash(alias);
> -			spin_unlock(&alias->d_lock);
> -		}
> -		spin_unlock(&inode->i_lock);
> -		return alias;
> -	}
> -	spin_unlock(&inode->i_lock);
> -	return NULL;
> -}
> -EXPORT_SYMBOL(d_exact_alias);
> -
>  static void swap_names(struct dentry *dentry, struct dentry *target)
>  {
>  	if (unlikely(dname_external(target))) {
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index df9669d4ded7..0a46b193f18e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3153,9 +3153,7 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
>  	if (d_really_is_negative(dentry)) {
>  		struct dentry *alias;
>  		d_drop(dentry);
> -		alias = d_exact_alias(dentry, state->inode);
> -		if (!alias)
> -			alias = d_splice_alias(igrab(state->inode), dentry);
> +		alias = d_splice_alias(igrab(state->inode), dentry);
>  		/* d_splice_alias() can't fail here - it's a non-directory */
>  		if (alias) {
>  			dput(ctx->dentry);
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 4afb60365675..8a63978187a4 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -253,7 +253,6 @@ extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
>  extern struct dentry * d_add_ci(struct dentry *, struct inode *, struct qstr *);
>  extern bool d_same_name(const struct dentry *dentry, const struct dentry *parent,
>  			const struct qstr *name);
> -extern struct dentry * d_exact_alias(struct dentry *, struct inode *);
>  extern struct dentry *d_find_any_alias(struct inode *inode);
>  extern struct dentry * d_obtain_alias(struct inode *);
>  extern struct dentry * d_obtain_root(struct inode *);

Reviewed-by: Jeff Layton <jlayton@kernel.org>