From: Anna Schumaker <Anna.Schumaker@netapp.com>
Subject: Re: [PATCH 6/9] nfs: move nfs4_set_lock_state call into caller
To: Jeff Layton <jlayton@redhat.com>, <trond.myklebust@primarydata.com>
References: <1473174760-29859-1-git-send-email-jlayton@redhat.com>
 <1473174760-29859-7-git-send-email-jlayton@redhat.com>
CC: <linux-nfs@vger.kernel.org>
Message-ID: <6e584c1a-605c-2b3f-07dc-305c5287cd5c@Netapp.com>
Date: Thu, 8 Sep 2016 15:47:37 -0400
MIME-Version: 1.0
In-Reply-To: <1473174760-29859-7-git-send-email-jlayton@redhat.com>
Content-Type: text/plain; charset="utf-8"
Sender: linux-nfs-owner@vger.kernel.org

Hi Jeff,

On 09/06/2016 11:12 AM, Jeff Layton wrote:
> We need to have this info set up before adding the waiter to the
> waitqueue, so move this out of the _nfs4_proc_setlk and into the
> caller. That's more efficient anyway since we don't need to do
> this more than once if we end up waiting on the lock.

Looks like you're moving this outside of the state recovery retry loop, too.  Do we need to worry about lock state changing during state recovery?

Thanks,
Anna

> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/nfs/nfs4proc.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index e9232d71bc64..5573f19539a6 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6134,14 +6134,8 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
>  	struct nfs_inode *nfsi = NFS_I(state->inode);
>  	struct nfs4_state_owner *sp = state->owner;
>  	unsigned char fl_flags = request->fl_flags;
> -	int status = -ENOLCK;
> +	int status;
>  
> -	if (!test_bit(NFS_STATE_POSIX_LOCKS, &state->flags))
> -		goto out;
> -	/* Is this a delegated open? */
> -	status = nfs4_set_lock_state(state, request);
> -	if (status != 0)
> -		goto out;
>  	request->fl_flags |= FL_ACCESS;
>  	status = locks_lock_inode_wait(state->inode, request);
>  	if (status < 0)
> @@ -6215,6 +6209,10 @@ nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
>  
>  	if (state == NULL)
>  		return -ENOLCK;
> +
> +	if (!test_bit(NFS_STATE_POSIX_LOCKS, &state->flags))
> +		return -ENOLCK;
> +
>  	/*
>  	 * Don't rely on the VFS having checked the file open mode,
>  	 * since it won't do this for flock() locks.
> @@ -6229,6 +6227,10 @@ nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
>  			return -EBADF;
>  	}
>  
> +	status = nfs4_set_lock_state(state, request);
> +	if (status != 0)
> +		return status;
> +
>  	do {
>  		status = nfs4_proc_setlk(state, cmd, request);
>  		if ((status != -EAGAIN) || IS_SETLK(cmd))
>