Hi Yihao,
On 19/5/13 14:57, Yihao Wu wrote:
> Commit b7dbcc0e433f "NFSv4.1: Fix a race where CB_NOTIFY_LOCK fails to wake a waiter"
> found this bug. However it didn't fix it.
>
> This commit replaces schedule_timeout() with wait_woken() and
> default_wake_function() with woken_wake_function() in function
> nfs4_retry_setlk() and nfs4_wake_lock_waiter(). wait_woken() uses
> memory barriers in its implementation to avoid potential race condition
> when putting a process into sleeping state and then waking it up.
>
> Fixes: a1d617d8f134 ("nfs: allow blocking locks to be awoken by lock callbacks")
> Cc: stable@xxxxxxxxxxxxxxx #4.9+
> Signed-off-by: Yihao Wu <wuyihao@xxxxxxxxxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 23 +++++++----------------
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index c29cbef..f9ed6b5 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6932,7 +6932,6 @@ struct nfs4_lock_waiter {
> struct task_struct *task;
> struct inode *inode;
> struct nfs_lowner *owner;
> - bool notified;
> };
>
> static int
> @@ -6954,13 +6953,13 @@ struct nfs4_lock_waiter {
> /* Make sure it's for the right inode */
> if (nfs_compare_fh(NFS_FH(waiter->inode), &cbnl->cbnl_fh))
> 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);
> + ret = woken_wake_function(wait, mode, flags, key);
> + if (ret)
> + list_del_init(&wait->entry);
> wait->private = waiter;
> return ret;
> }
> @@ -6979,8 +6978,7 @@ struct nfs4_lock_waiter {
> .s_dev = server->s_dev };
> struct nfs4_lock_waiter waiter = { .task = current,
> .inode = state->inode,
> - .owner = &owner,
> - .notified = false };
> + .owner = &owner};
> wait_queue_entry_t wait;
>
> /* Don't bother with waitqueue if we don't expect a callback */
> @@ -6993,21 +6991,14 @@ struct nfs4_lock_waiter {
> add_wait_queue(q, &wait);
>
> while(!signalled()) {
> - waiter.notified = false;
> status = nfs4_proc_setlk(state, cmd, request);
> if ((status != -EAGAIN) || IS_SETLK(cmd))
> break;
>
> status = -ERESTARTSYS;
> - spin_lock_irqsave(&q->lock, flags);
> - if (waiter.notified) {
> - spin_unlock_irqrestore(&q->lock, flags);
> - continue;
> - }
> - set_current_state(TASK_INTERRUPTIBLE);
> - spin_unlock_irqrestore(&q->lock, flags);
> -
> - freezable_schedule_timeout(NFS4_LOCK_MAXTIMEOUT);
> + freezer_do_not_count();
> + wait_woken(&wait, TASK_INTERRUPTIBLE, NFS4_LOCK_MAXTIMEOUT);
> + freezer_count();
Since now variable 'flags' is not used anymore, we have to delete it as well.
Otherwise there is a compile warning “unused variable ‘flags’”.
Thanks,
Joseph
> }
>
> finish_wait(q, &wait);
>
