Skip to contentSkip to conversation

nfs4: lock notification fixes

PATCH New Jeff Layton 1 participant 4 messages Mar 18, 2018

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Jeff Layton

author maintainer
#0 lore raw

cover nfs4: lock notification fixes

From: Jeff Layton <[email protected]>

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(-)
Signature · 1 line
2.14.3
author maintainer
#1 lore raw

PATCH 1/3 nfs4: always reset notified flag to false before repolling for lock

From: Jeff Layton <[email protected]>

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 <[email protected]>
---
 fs/nfs/nfs4proc.c | 1 +
 1 file changed, 1 insertion(+)
fs/nfs/nfs4proc.c +1 -0
index 47f3c273245e..5ab28454f117 100644
@@ -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;
Signature · 1 line
2.14.3
author maintainer
#2 lore raw

PATCH 2/3 nfs4: don't compare clientid in nfs4_wake_lock_waiter

In reply to Jeff Layton

From: Jeff Layton <[email protected]>

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 <[email protected]>
---
 fs/nfs/nfs4proc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
fs/nfs/nfs4proc.c +2 -4
index 5ab28454f117..669ba9211177 100644
@@ -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 */
Signature · 1 line
2.14.3
author maintainer
#3 lore raw

PATCH 3/3 nfs4: wake any lock waiters on successful RECLAIM_COMPLETE

In reply to Jeff Layton

From: Jeff Layton <[email protected]>

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 <[email protected]>
---
 fs/nfs/nfs4proc.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
fs/nfs/nfs4proc.c +16 -10
index 669ba9211177..ec68edce6cc3 100644
@@ -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;
Signature · 1 line
2.14.3