Hi Jeff,
On 09/06/2016 11:12 AM, Jeff Layton wrote:
> Add a waitqueue head to the client structure. Have clients set a wait
> on that queue prior to requesting a lock from the server. If the lock
> is blocked, then we can use that to wait for wakeups.
>
> Note that we do need to do this "manually" since we need to set the
> wait on the waitqueue prior to requesting the lock, but requesting a
> lock can involve activities that can block.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfs/callback_proc.c | 1 +
> fs/nfs/nfs4client.c | 1 +
> fs/nfs/nfs4proc.c | 67 ++++++++++++++++++++++++++++++++++++++++++++---
> include/linux/nfs_fs_sb.h | 1 +
> 4 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index a211af339f32..4ba6a8763f91 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -645,6 +645,7 @@ __be32 nfs4_callback_notify_lock(struct cb_notify_lock_args *args, void *dummy,
> fc_tbl = &cps->clp->cl_session->fc_slot_table;
>
> status = htonl(NFS4_OK);
> + __wake_up(&cps->clp->cl_lock_waitq, TASK_NORMAL, 0, &args->cbnl_owner);
> return status;
> }
> #endif /* CONFIG_NFS_V4_1 */
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index cd3b7cfdde16..89b7a794ff10 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -199,6 +199,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
> clp->cl_minorversion = cl_init->minorversion;
> clp->cl_mvops = nfs_v4_minor_ops[cl_init->minorversion];
> clp->cl_mig_gen = 1;
> + init_waitqueue_head(&clp->cl_lock_waitq);
> return clp;
>
> error:
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 5573f19539a6..3a6669063c44 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5531,13 +5531,54 @@ int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4
> #define NFS4_LOCK_MINTIMEOUT (1 * HZ)
> #define NFS4_LOCK_MAXTIMEOUT (30 * HZ)
>
> +struct nfs4_lock_waiter {
> + struct task_struct *task;
> + struct nfs_lowner *owner;
> + bool notified;
> +};
> +
> +static int
> +nfs4_wake_lock_waiter(wait_queue_t *wait, unsigned int mode, int flags, void *key)
> +{
> + int ret;
> + struct nfs4_lock_waiter *waiter = wait->private;
> + struct nfs_lowner *lowner = key, *wowner = waiter->owner;
> +
> + /* Don't wake anybody if the string looked bogus */
> + if (!lowner->id && !lowner->s_dev)
> + return 0;
> +
> + /* Only wake if the callback was for the same owner */
> + if (lowner->clientid != wowner->clientid ||
> + lowner->id != wowner->id ||
> + lowner->s_dev != wowner->s_dev)
> + return 0;
> +
> + waiter->notified = true;
> +
> + /* override "private" so we can use default_wake_function */
> + wait->private = waiter->task;
> + ret = autoremove_wake_function(wait, mode, flags, key);
> + wait->private = waiter;
> + return ret;
> +}
> +
> /*
> * sleep, with exponential backoff, and retry the LOCK operation.
> */
> static unsigned long
> -nfs4_set_lock_task_retry(unsigned long timeout)
> +nfs4_wait_for_lock_retry(unsigned long timeout, wait_queue_head_t *q,
> + struct nfs4_lock_waiter *waiter)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&q->lock, flags);
> + if (waiter->notified) {
> + spin_unlock_irqrestore(&q->lock, flags);
> + return timeout;
> + }
> set_current_state(TASK_INTERRUPTIBLE);
> + spin_unlock_irqrestore(&q->lock, flags);
> freezable_schedule_timeout_unsafe(timeout);
> timeout <<= 1;
> if (timeout > NFS4_LOCK_MAXTIMEOUT)
> @@ -6232,10 +6273,30 @@ nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
> return status;
>
> do {
> + struct nfs4_lock_state *lsp = request->fl_u.nfs4_fl.owner;
> + struct nfs_server *server = NFS_SERVER(lsp->ls_state->inode);
> + struct nfs_client *clp = server->nfs_client;
> + struct nfs_lowner owner = { .clientid = clp->cl_clientid,
> + .id = lsp->ls_seqid.owner_id,
> + .s_dev = server->s_dev };
> + struct nfs4_lock_waiter waiter = { .task = current,
> + .owner = &owner,
> + .notified = false };
> + wait_queue_t wait;
> +
> + init_wait(&wait);
> + wait.private = &waiter;
> + wait.func = nfs4_wake_lock_waiter;
> +
> + add_wait_queue(&clp->cl_lock_waitq, &wait);
> status = nfs4_proc_setlk(state, cmd, request);
> - if ((status != -EAGAIN) || IS_SETLK(cmd))
> + if ((status != -EAGAIN) || IS_SETLK(cmd)) {
> + finish_wait(&clp->cl_lock_waitq, &wait);
> break;
> - timeout = nfs4_set_lock_task_retry(timeout);
> + }
> + timeout = nfs4_wait_for_lock_retry(timeout,
> + &clp->cl_lock_waitq, &waiter);
> + finish_wait(&clp->cl_lock_waitq, &wait);
I'm curious why you didn't turn everything in this do-while loop into a new function? This code might be cleaner that way :)
Thanks,
Anna
> status = -ERESTARTSYS;
> if (signalled())
> break;
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 14a762d2734d..fb2319a0ce65 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -103,6 +103,7 @@ struct nfs_client {
> #define NFS_SP4_MACH_CRED_WRITE 5 /* WRITE */
> #define NFS_SP4_MACH_CRED_COMMIT 6 /* COMMIT */
> #define NFS_SP4_MACH_CRED_PNFS_CLEANUP 7 /* LAYOUTRETURN */
> + wait_queue_head_t cl_lock_waitq;
> #endif /* CONFIG_NFS_V4 */
>
> /* Our own IP address, as a null-terminated string.
>
--
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
