On Fri, 2016-09-16 at 21:20 +0000, Trond Myklebust wrote: > > > > On Sep 16, 2016, at 16:27, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > This also consolidates the waiting logic into a single function, > > instead of having it spread across two like it is now. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/nfs/nfs4proc.c | 51 ++++++++++++++++++++++++------------------ > > --------- > > 1 file changed, 24 insertions(+), 27 deletions(-) > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > > index c807850ac476..a7517abaf3c7 100644 > > --- a/fs/nfs/nfs4proc.c > > +++ b/fs/nfs/nfs4proc.c > > @@ -5530,22 +5530,6 @@ int nfs4_proc_delegreturn(struct inode > > *inode, struct rpc_cred *cred, const nfs4 > > return err; > > } > > > > -#define NFS4_LOCK_MINTIMEOUT (1 * HZ) > > -#define NFS4_LOCK_MAXTIMEOUT (30 * HZ) > > - > > -/* > > - * sleep, with exponential backoff, and retry the LOCK operation. > > - */ > > -static unsigned long > > -nfs4_set_lock_task_retry(unsigned long timeout) > > -{ > > - freezable_schedule_timeout_interruptible(timeout); > > - timeout <<= 1; > > - if (timeout > NFS4_LOCK_MAXTIMEOUT) > > - return NFS4_LOCK_MAXTIMEOUT; > > - return timeout; > > -} > > - > > static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, > > struct file_lock *request) > > { > > struct inode *inode = state->inode; > > @@ -6178,12 +6162,34 @@ static int nfs4_proc_setlk(struct > > nfs4_state *state, int cmd, struct file_lock * > > return err; > > } > > > > +#define NFS4_LOCK_MINTIMEOUT (1 * HZ) > > +#define NFS4_LOCK_MAXTIMEOUT (30 * HZ) > > + > > +static int > > +nfs4_retry_setlk(struct nfs4_state *state, int cmd, struct > > file_lock *request) > > +{ > > + int status; > > + unsigned long timeout = NFS4_LOCK_MINTIMEOUT; > > + > > + do { > > + status = nfs4_proc_setlk(state, cmd, request); > > + if ((status != -EAGAIN) || IS_SETLK(cmd)) > > + break; > > + freezable_schedule_timeout_interruptible(timeout); > > + timeout *= 2; > > + timeout = min_t(unsigned long, > > NFS4_LOCK_MAXTIMEOUT, timeout); > > + status = -ERESTARTSYS; > > + if (signalled()) > > + break; > > + } while(status < 0); > > Can it ever be >= 0 here? Why not just use 'while (!signalled())'? > Good point. I'll make that change. Thanks, > > > > + return status; > > +} > > + > > static int > > nfs4_proc_lock(struct file *filp, int cmd, struct file_lock > > *request) > > { > > struct nfs_open_context *ctx; > > struct nfs4_state *state; > > - unsigned long timeout = NFS4_LOCK_MINTIMEOUT; > > int status; > > > > /* verify open state */ > > @@ -6232,16 +6238,7 @@ nfs4_proc_lock(struct file *filp, int cmd, > > struct file_lock *request) > > if (status != 0) > > return status; > > > > - do { > > - status = nfs4_proc_setlk(state, cmd, request); > > - if ((status != -EAGAIN) || IS_SETLK(cmd)) > > - break; > > - timeout = nfs4_set_lock_task_retry(timeout); > > - status = -ERESTARTSYS; > > - if (signalled()) > > - break; > > - } while(status < 0); > > - return status; > > + return nfs4_retry_setlk(state, cmd, request); > > } > > > > int nfs4_lock_delegation_recall(struct file_lock *fl, struct > > nfs4_state *state, const nfs4_stateid *stateid) > > -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html