linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] nfs4: lock notification fixes
@ 2018-03-18 12:37 Jeff Layton
  2018-03-18 12:37 ` [PATCH 1/3] nfs4: always reset notified flag to false before repolling for lock Jeff Layton
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jeff Layton @ 2018-03-18 12:37 UTC (permalink / raw)
  To: trondmy, anna.schumaker; +Cc: linux-nfs

From: Jeff Layton <jlayton@redhat.com>

Just a few patches for some minor problems that I spotted while testing
how lock notifications get handled with a network partition.

None of these are particularly urgent so should be fine for v4.17.

Jeff Layton (3):
  nfs4: always reset notified flag to false before repolling for lock
  nfs4: don't compare clientid in nfs4_wake_lock_waiter
  nfs4: wake any lock waiters on successful RECLAIM_COMPLETE

 fs/nfs/nfs4proc.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

-- 
2.14.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] nfs4: always reset notified flag to false before repolling for lock
  2018-03-18 12:37 [PATCH 0/3] nfs4: lock notification fixes Jeff Layton
@ 2018-03-18 12:37 ` Jeff Layton
  2018-03-18 12:37 ` [PATCH 2/3] nfs4: don't compare clientid in nfs4_wake_lock_waiter Jeff Layton
  2018-03-18 12:37 ` [PATCH 3/3] nfs4: wake any lock waiters on successful RECLAIM_COMPLETE Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2018-03-18 12:37 UTC (permalink / raw)
  To: trondmy, anna.schumaker; +Cc: linux-nfs

From: Jeff Layton <jlayton@redhat.com>

We may get a notification and lose the race to another client. Ensure
that we wait again for a notification in that case.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/nfs4proc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 47f3c273245e..5ab28454f117 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6673,6 +6673,7 @@ nfs4_retry_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
 	add_wait_queue(q, &wait);
 
 	while(!signalled()) {
+		waiter.notified = false;
 		status = nfs4_proc_setlk(state, cmd, request);
 		if ((status != -EAGAIN) || IS_SETLK(cmd))
 			break;
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] nfs4: don't compare clientid in nfs4_wake_lock_waiter
  2018-03-18 12:37 [PATCH 0/3] nfs4: lock notification fixes Jeff Layton
  2018-03-18 12:37 ` [PATCH 1/3] nfs4: always reset notified flag to false before repolling for lock Jeff Layton
@ 2018-03-18 12:37 ` Jeff Layton
  2018-03-18 12:37 ` [PATCH 3/3] nfs4: wake any lock waiters on successful RECLAIM_COMPLETE Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2018-03-18 12:37 UTC (permalink / raw)
  To: trondmy, anna.schumaker; +Cc: linux-nfs

From: Jeff Layton <jlayton@redhat.com>

The task is expected to sleep for a while here, and it's possible that
a new EXCHANGE_ID has occurred in the interim, and we were assigned a
new clientid. Since this is a per-client list, there isn't a lot of
value in vetting the clientid on the incoming request.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/nfs4proc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5ab28454f117..669ba9211177 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6626,10 +6626,8 @@ nfs4_wake_lock_waiter(wait_queue_entry_t *wait, unsigned int mode, int flags, vo
 	struct nfs_lowner	*lowner = &cbnl->cbnl_owner,
 				*wowner = waiter->owner;
 
-	/* 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)
+	/* Only wake if the callback was for the same owner. */
+	if (lowner->id != wowner->id || lowner->s_dev != wowner->s_dev)
 		return 0;
 
 	/* Make sure it's for the right inode */
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] nfs4: wake any lock waiters on successful RECLAIM_COMPLETE
  2018-03-18 12:37 [PATCH 0/3] nfs4: lock notification fixes Jeff Layton
  2018-03-18 12:37 ` [PATCH 1/3] nfs4: always reset notified flag to false before repolling for lock Jeff Layton
  2018-03-18 12:37 ` [PATCH 2/3] nfs4: don't compare clientid in nfs4_wake_lock_waiter Jeff Layton
@ 2018-03-18 12:37 ` Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2018-03-18 12:37 UTC (permalink / raw)
  To: trondmy, anna.schumaker; +Cc: linux-nfs

From: Jeff Layton <jlayton@redhat.com>

If we have a RECLAIM_COMPLETE with a populated cl_lock_waitq, then
that implies that a reconnect has occurred. Since we can't expect a
CB_NOTIFY_LOCK callback at that point, just wake up the entire queue
so that all the tasks can re-poll for their locks.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/nfs4proc.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 669ba9211177..ec68edce6cc3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6621,20 +6621,24 @@ static int
 nfs4_wake_lock_waiter(wait_queue_entry_t *wait, unsigned int mode, int flags, void *key)
 {
 	int ret;
-	struct cb_notify_lock_args *cbnl = key;
 	struct nfs4_lock_waiter	*waiter	= wait->private;
-	struct nfs_lowner	*lowner = &cbnl->cbnl_owner,
-				*wowner = waiter->owner;
 
-	/* Only wake if the callback was for the same owner. */
-	if (lowner->id != wowner->id || lowner->s_dev != wowner->s_dev)
-		return 0;
+	/* NULL key means to wake up everyone */
+	if (key) {
+		struct cb_notify_lock_args	*cbnl = key;
+		struct nfs_lowner		*lowner = &cbnl->cbnl_owner,
+						*wowner = waiter->owner;
 
-	/* Make sure it's for the right inode */
-	if (nfs_compare_fh(NFS_FH(waiter->inode), &cbnl->cbnl_fh))
-		return 0;
+		/* Only wake if the callback was for the same owner. */
+		if (lowner->id != wowner->id || lowner->s_dev != wowner->s_dev)
+			return 0;
 
-	waiter->notified = true;
+		/* 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;
@@ -8413,6 +8417,8 @@ static int nfs41_reclaim_complete_handle_errors(struct rpc_task *task, struct nf
 {
 	switch(task->tk_status) {
 	case 0:
+		wake_up_all(&clp->cl_lock_waitq);
+		/* Fallthrough */
 	case -NFS4ERR_COMPLETE_ALREADY:
 	case -NFS4ERR_WRONG_CRED: /* What to do here? */
 		break;
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-18 12:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-18 12:37 [PATCH 0/3] nfs4: lock notification fixes Jeff Layton
2018-03-18 12:37 ` [PATCH 1/3] nfs4: always reset notified flag to false before repolling for lock Jeff Layton
2018-03-18 12:37 ` [PATCH 2/3] nfs4: don't compare clientid in nfs4_wake_lock_waiter Jeff Layton
2018-03-18 12:37 ` [PATCH 3/3] nfs4: wake any lock waiters on successful RECLAIM_COMPLETE Jeff Layton

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).