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
