[6/9] nfs: move nfs4_set_lock_state call into caller
diff mbox

Message ID 1473174760-29859-7-git-send-email-jlayton@redhat.com
State New
Headers show

Commit Message

Jeff Layton Sept. 6, 2016, 3:12 p.m. UTC
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.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/nfs4proc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Schumaker, Anna Sept. 8, 2016, 7:47 p.m. UTC | #1
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))
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Layton Sept. 8, 2016, 9:41 p.m. UTC | #2
On Thu, 2016-09-08 at 15:47 -0400, Anna Schumaker wrote:
> 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
> 

I'm not sure I understand. _nfs4_proc_setlk and nfs4_proc_setlk each
only have one caller so there are no cases where we'd call these
functions and not call nfs4_set_lock_state first.

The first thing that nfs4_set_lock_state does is this:

        if (fl->fl_ops != NULL)
                return 0;

So it's a no-op on every subsequent attempt.

> > 
> > 
> > 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))
> > 
>

Patch
diff mbox

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))