linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yihao Wu <wuyihao@linux.alibaba.com>
To: linux-nfs@vger.kernel.org, Jeff Layton <jlayton@kernel.org>,
	"J. Bruce Fields" <bfields@fieldses.org>
Cc: stable@vger.kernel.org, Joseph Qi <joseph.qi@linux.alibaba.com>,
	caspar@linux.alibaba.com
Subject: [PATCH 1/2] NFSv4.1: Again fix a race where CB_NOTIFY_LOCK fails to wake a waiter
Date: Wed, 8 May 2019 17:13:25 +0800	[thread overview]
Message-ID: <2a1cebca-1efb-1686-475b-a581e50e61b4@linux.alibaba.com> (raw)
In-Reply-To: <d0b6fc01-0a73-e4f7-b393-3ecc9aacffb0@linux.alibaba.com>

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 can
be fixed by adding memory barrier pair.

Specifically, if any CB_NOTIFY_LOCK should be handled between unlocking
the wait queue and freezable_schedule_timeout, only two cases are
possible. So CB_NOTIFY_LOCK will not be dropped unexpectly.

1. The callback thread marks the NFS client as waked. Then NFS client
noticed that itself is waked, so it don't goes to sleep. And it cleans
its wake mark.

2. The NFS client noticed that itself is not waked yet, so it goes to
sleep. No modification will ever happen to the wake mark in between.

Fixes: a1d617d ("nfs: allow blocking locks to be awoken by lock callbacks")
Signed-off-by: Yihao Wu <wuyihao@linux.alibaba.com>
---
 fs/nfs/nfs4proc.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 741ff8c..f13ea09 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6867,7 +6867,6 @@ struct nfs4_lock_waiter {
 	struct task_struct	*task;
 	struct inode		*inode;
 	struct nfs_lowner	*owner;
-	bool			notified;
 };
 
 static int
@@ -6889,13 +6888,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;
 }
@@ -6914,8 +6913,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 */
@@ -6928,21 +6926,12 @@ 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);
+		wait_woken(&wait, TASK_INTERRUPTIBLE, NFS4_LOCK_MAXTIMEOUT);
 	}
 
 	finish_wait(q, &wait);
-- 
1.8.3.1


  reply	other threads:[~2019-05-08  9:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-08  9:11 [PATCH 0/2] Fix two bugs CB_NOTIFY_LOCK failing to wake a water Yihao Wu
2019-05-08  9:13 ` Yihao Wu [this message]
2019-05-08  9:15   ` [PATCH 2/2] NFSv4.1: Fix bug only the first CB_NOTIFY_LOCK is handled Yihao Wu
2019-05-08  9:23     ` Greg KH
2019-05-08  9:19   ` [PATCH 1/2] NFSv4.1: Again fix a race where CB_NOTIFY_LOCK fails to wake a waiter Greg KH
2019-05-08  9:39     ` Yihao Wu
2019-05-08 12:24   ` Jeff Layton
2019-05-08 18:18     ` Yihao Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a1cebca-1efb-1686-475b-a581e50e61b4@linux.alibaba.com \
    --to=wuyihao@linux.alibaba.com \
    --cc=bfields@fieldses.org \
    --cc=caspar@linux.alibaba.com \
    --cc=jlayton@kernel.org \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).