Re: [PATCH 3/8] NFSD: always drop directory lock in nfsd_unlink()

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2022-07-06 at 14:18 +1000, NeilBrown wrote:
> Some error paths in nfsd_unlink() allow it to exit without unlocking the
> directory.  This is not a problem in practice as the directory will be
> locked with an fh_put(), but it is untidy and potentially confusing.
> 
> This allows us to remove all the fh_unlock() calls that are immediately
> after nfsd_unlink() calls.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/nfs3proc.c |    2 --
>  fs/nfsd/nfs4proc.c |    4 +---
>  fs/nfsd/vfs.c      |    7 +++++--
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 38255365ef71..ad7941001106 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -478,7 +478,6 @@ nfsd3_proc_remove(struct svc_rqst *rqstp)
>  	fh_copy(&resp->fh, &argp->fh);
>  	resp->status = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR,
>  				   argp->name, argp->len);
> -	fh_unlock(&resp->fh);
>  	return rpc_success;
>  }
>  
> @@ -499,7 +498,6 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp)
>  	fh_copy(&resp->fh, &argp->fh);
>  	resp->status = nfsd_unlink(rqstp, &resp->fh, S_IFDIR,
>  				   argp->name, argp->len);
> -	fh_unlock(&resp->fh);
>  	return rpc_success;
>  }
>  
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3279daab909d..4737019738ab 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1052,10 +1052,8 @@ nfsd4_remove(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		return nfserr_grace;
>  	status = nfsd_unlink(rqstp, &cstate->current_fh, 0,
>  			     remove->rm_name, remove->rm_namelen);
> -	if (!status) {
> -		fh_unlock(&cstate->current_fh);
> +	if (!status)
>  		set_change_info(&remove->rm_cinfo, &cstate->current_fh);
> -	}
>  	return status;
>  }
>  
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 1e7ca39e8a49..3f4579f5775c 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1762,12 +1762,12 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
>  	rdentry = lookup_one_len(fname, dentry, flen);
>  	host_err = PTR_ERR(rdentry);
>  	if (IS_ERR(rdentry))
> -		goto out_drop_write;
> +		goto out_unlock;
>  
>  	if (d_really_is_negative(rdentry)) {
>  		dput(rdentry);
>  		host_err = -ENOENT;
> -		goto out_drop_write;
> +		goto out_unlock;
>  	}
>  	rinode = d_inode(rdentry);
>  	ihold(rinode);
> @@ -1805,6 +1805,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
>  	}
>  out:
>  	return err;
> +out_unlock:
> +	fh_unlock(fhp);
> +	goto out_drop_write;
>  }
>  
>  /*
> 
> 

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