| Message ID | 1473174760-29859-7-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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)) > > >
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))
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(-)