linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/8] NFSv4: Fix a credential refcount leak in nfs41_check_delegation_stateid
@ 2019-08-03 14:58 Trond Myklebust
  2019-08-03 14:58 ` [PATCH 2/8] NFSv4: Fix delegation state recovery Trond Myklebust
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

It is unsafe to dereference delegation outside the rcu lock, and in
any case, the refcount is guaranteed held if cred is non-zero.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4proc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 39896afc6edf..a6d73609b163 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2778,8 +2778,7 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
 		nfs_finish_clear_delegation_stateid(state, &stateid);
 
-	if (delegation->cred)
-		put_cred(cred);
+	put_cred(cred);
 }
 
 /**
-- 
2.21.0


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

* [PATCH 2/8] NFSv4: Fix delegation state recovery
  2019-08-03 14:58 [PATCH 1/8] NFSv4: Fix a credential refcount leak in nfs41_check_delegation_stateid Trond Myklebust
@ 2019-08-03 14:58 ` Trond Myklebust
  2019-08-03 14:58   ` [PATCH 3/8] NFSv4: Print an error in the syslog when state is marked as irrecoverable Trond Myklebust
  2019-08-03 17:09   ` [PATCH 2/8] NFSv4: Fix delegation state recovery Sasha Levin
  0 siblings, 2 replies; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

Once we clear the NFS_DELEGATED_STATE flag, we're telling
nfs_delegation_claim_opens() that we're done recovering all open state
for that stateid, so we really need to ensure that we test for all
open modes that are currently cached and recover them before exiting
nfs4_open_delegation_recall().

Fixes: 24311f884189d ("NFSv4: Recovery of recalled read delegations...")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: stable@vger.kernel.org # v4.3+
---
 fs/nfs/delegation.c |  2 +-
 fs/nfs/delegation.h |  2 +-
 fs/nfs/nfs4proc.c   | 25 ++++++++++++-------------
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 0ff3facf81da..0af854cce8ff 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -153,7 +153,7 @@ static int nfs_delegation_claim_opens(struct inode *inode,
 		/* Block nfs4_proc_unlck */
 		mutex_lock(&sp->so_delegreturn_mutex);
 		seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
-		err = nfs4_open_delegation_recall(ctx, state, stateid, type);
+		err = nfs4_open_delegation_recall(ctx, state, stateid);
 		if (!err)
 			err = nfs_delegation_claim_locks(state, stateid);
 		if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 5799777df5ec..9eb87ae4c982 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -63,7 +63,7 @@ void nfs_reap_expired_delegations(struct nfs_client *clp);
 
 /* NFSv4 delegation-related procedures */
 int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, const nfs4_stateid *stateid, int issync);
-int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
+int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid);
 int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
 bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_stateid *dst, const struct cred **cred);
 bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a6d73609b163..21e3c159bc69 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2177,12 +2177,10 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 		case -NFS4ERR_BAD_HIGH_SLOT:
 		case -NFS4ERR_CONN_NOT_BOUND_TO_SESSION:
 		case -NFS4ERR_DEADSESSION:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			nfs4_schedule_session_recovery(server->nfs_client->cl_session, err);
 			return -EAGAIN;
 		case -NFS4ERR_STALE_CLIENTID:
 		case -NFS4ERR_STALE_STATEID:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			/* Don't recall a delegation if it was lost */
 			nfs4_schedule_lease_recovery(server->nfs_client);
 			return -EAGAIN;
@@ -2203,7 +2201,6 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 			return -EAGAIN;
 		case -NFS4ERR_DELAY:
 		case -NFS4ERR_GRACE:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			ssleep(1);
 			return -EAGAIN;
 		case -ENOMEM:
@@ -2219,8 +2216,7 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 }
 
 int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
-		struct nfs4_state *state, const nfs4_stateid *stateid,
-		fmode_t type)
+		struct nfs4_state *state, const nfs4_stateid *stateid)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
 	struct nfs4_opendata *opendata;
@@ -2231,20 +2227,23 @@ int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
 	if (IS_ERR(opendata))
 		return PTR_ERR(opendata);
 	nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
-	nfs_state_clear_delegation(state);
-	switch (type & (FMODE_READ|FMODE_WRITE)) {
-	case FMODE_READ|FMODE_WRITE:
-	case FMODE_WRITE:
+	if (!test_bit(NFS_O_RDWR_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
 		if (err)
-			break;
+			goto out;
+	}
+	if (!test_bit(NFS_O_WRONLY_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_WRITE);
 		if (err)
-			break;
-		/* Fall through */
-	case FMODE_READ:
+			goto out;
+	}
+	if (!test_bit(NFS_O_RDONLY_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_READ);
+		if (err)
+			goto out;
 	}
+	nfs_state_clear_delegation(state);
+out:
 	nfs4_opendata_put(opendata);
 	return nfs4_handle_delegation_recall_error(server, state, stateid, NULL, err);
 }
-- 
2.21.0


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

* [PATCH 3/8] NFSv4: Print an error in the syslog when state is marked as irrecoverable
  2019-08-03 14:58 ` [PATCH 2/8] NFSv4: Fix delegation state recovery Trond Myklebust
@ 2019-08-03 14:58   ` Trond Myklebust
  2019-08-03 14:58     ` [PATCH 4/8] NFSv4: When recovering state fails with EAGAIN, retry the same recovery Trond Myklebust
  2019-08-03 17:09   ` [PATCH 2/8] NFSv4: Fix delegation state recovery Sasha Levin
  1 sibling, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

When error recovery fails due to a fatal error on the server, ensure
we log it in the syslog.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4state.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9afd051a4876..a71a61e5fe2c 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1463,7 +1463,7 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
 		nfs4_schedule_state_manager(clp);
 }
 
-static void nfs4_state_mark_open_context_bad(struct nfs4_state *state)
+static void nfs4_state_mark_open_context_bad(struct nfs4_state *state, int err)
 {
 	struct inode *inode = state->inode;
 	struct nfs_inode *nfsi = NFS_I(inode);
@@ -1474,6 +1474,8 @@ static void nfs4_state_mark_open_context_bad(struct nfs4_state *state)
 		if (ctx->state != state)
 			continue;
 		set_bit(NFS_CONTEXT_BAD, &ctx->flags);
+		pr_warn("NFSv4: state recovery failed for open file %pd2, "
+				"error = %d\n", ctx->dentry, err);
 	}
 	rcu_read_unlock();
 }
@@ -1481,7 +1483,7 @@ static void nfs4_state_mark_open_context_bad(struct nfs4_state *state)
 static void nfs4_state_mark_recovery_failed(struct nfs4_state *state, int error)
 {
 	set_bit(NFS_STATE_RECOVERY_FAILED, &state->flags);
-	nfs4_state_mark_open_context_bad(state);
+	nfs4_state_mark_open_context_bad(state, error);
 }
 
 
-- 
2.21.0


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

* [PATCH 4/8] NFSv4: When recovering state fails with EAGAIN, retry the same recovery
  2019-08-03 14:58   ` [PATCH 3/8] NFSv4: Print an error in the syslog when state is marked as irrecoverable Trond Myklebust
@ 2019-08-03 14:58     ` Trond Myklebust
  2019-08-03 14:58       ` [PATCH 5/8] NFSv4: Report the error from nfs4_select_rw_stateid() Trond Myklebust
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

If the server returns with EAGAIN when we're trying to recover from
a server reboot, we currently delay for 1 second, but then mark the
stateid as needing recovery after the grace period has expired.

Instead, we should just retry the same recovery process immediately
after the 1 second delay. Break out of the loop after 10 retries.

Fixes: 35a61606a612 ("NFS: Reduce indentation of the switch statement...")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4state.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index a71a61e5fe2c..d03b9cf42bd0 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1607,6 +1607,7 @@ static int __nfs4_reclaim_open_state(struct nfs4_state_owner *sp, struct nfs4_st
 static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs4_state_recovery_ops *ops)
 {
 	struct nfs4_state *state;
+	unsigned int loop = 0;
 	int status = 0;
 
 	/* Note: we rely on the sp->so_states list being ordered 
@@ -1633,8 +1634,10 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs
 
 		switch (status) {
 		default:
-			if (status >= 0)
+			if (status >= 0) {
+				loop = 0;
 				break;
+			}
 			printk(KERN_ERR "NFS: %s: unhandled error %d\n", __func__, status);
 			/* Fall through */
 		case -ENOENT:
@@ -1648,6 +1651,10 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs
 			break;
 		case -EAGAIN:
 			ssleep(1);
+			if (loop++ < 10) {
+				set_bit(ops->state_flag_bit, &state->flags);
+				break;
+			}
 			/* Fall through */
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_STALE_STATEID:
-- 
2.21.0


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

* [PATCH 5/8] NFSv4: Report the error from nfs4_select_rw_stateid()
  2019-08-03 14:58     ` [PATCH 4/8] NFSv4: When recovering state fails with EAGAIN, retry the same recovery Trond Myklebust
@ 2019-08-03 14:58       ` Trond Myklebust
  2019-08-03 14:58         ` [PATCH 6/8] NFSv4.1: Fix open stateid recovery Trond Myklebust
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

In pnfs_update_layout() ensure that we do report any fatal errors from
nfs4_select_rw_stateid().

Fixes: d9aba2b40de6 ("NFSv4: Don't use the zero stateid with layoutget")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/pnfs.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 75bd5b552ba4..4525d5acae38 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1903,12 +1903,6 @@ pnfs_update_layout(struct inode *ino,
 		goto out_unlock;
 	}
 
-	if (!nfs4_valid_open_stateid(ctx->state)) {
-		trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
-				PNFS_UPDATE_LAYOUT_INVALID_OPEN);
-		goto out_unlock;
-	}
-
 	/*
 	 * Choose a stateid for the LAYOUTGET. If we don't have a layout
 	 * stateid, or it has been invalidated, then we must use the open
@@ -1939,6 +1933,7 @@ pnfs_update_layout(struct inode *ino,
 					iomode == IOMODE_RW ? FMODE_WRITE : FMODE_READ,
 					NULL, &stateid, NULL);
 		if (status != 0) {
+			lseg = ERR_PTR(status);
 			trace_pnfs_update_layout(ino, pos, count,
 					iomode, lo, lseg,
 					PNFS_UPDATE_LAYOUT_INVALID_OPEN);
-- 
2.21.0


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

* [PATCH 6/8] NFSv4.1: Fix open stateid recovery
  2019-08-03 14:58       ` [PATCH 5/8] NFSv4: Report the error from nfs4_select_rw_stateid() Trond Myklebust
@ 2019-08-03 14:58         ` Trond Myklebust
  2019-08-03 14:58           ` [PATCH 7/8] NFSv4.1: Only reap expired delegations Trond Myklebust
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

The logic for checking in nfs41_check_open_stateid() whether the state
is supported by a delegation is inverted. In addition, it makes more
sense to perform that check before we check for expired locks.

Fixes: 8a64c4ef106d1 ("NFSv4.1: Even if the stateid is OK,...")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4proc.c | 65 +++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 21e3c159bc69..c9e14ce0b7b2 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1683,6 +1683,14 @@ static void nfs_state_set_open_stateid(struct nfs4_state *state,
 	write_sequnlock(&state->seqlock);
 }
 
+static void nfs_state_clear_open_state_flags(struct nfs4_state *state)
+{
+	clear_bit(NFS_O_RDWR_STATE, &state->flags);
+	clear_bit(NFS_O_WRONLY_STATE, &state->flags);
+	clear_bit(NFS_O_RDONLY_STATE, &state->flags);
+	clear_bit(NFS_OPEN_STATE, &state->flags);
+}
+
 static void nfs_state_set_delegation(struct nfs4_state *state,
 		const nfs4_stateid *deleg_stateid,
 		fmode_t fmode)
@@ -2074,13 +2082,7 @@ static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *
 {
 	int ret;
 
-	/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
-	clear_bit(NFS_O_RDWR_STATE, &state->flags);
-	clear_bit(NFS_O_WRONLY_STATE, &state->flags);
-	clear_bit(NFS_O_RDONLY_STATE, &state->flags);
 	/* memory barrier prior to reading state->n_* */
-	clear_bit(NFS_DELEGATED_STATE, &state->flags);
-	clear_bit(NFS_OPEN_STATE, &state->flags);
 	smp_rmb();
 	ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
 	if (ret != 0)
@@ -2156,6 +2158,8 @@ static int nfs4_open_reclaim(struct nfs4_state_owner *sp, struct nfs4_state *sta
 	ctx = nfs4_state_find_open_context(state);
 	if (IS_ERR(ctx))
 		return -EAGAIN;
+	clear_bit(NFS_DELEGATED_STATE, &state->flags);
+	nfs_state_clear_open_state_flags(state);
 	ret = nfs4_do_open_reclaim(ctx, state);
 	put_nfs_open_context(ctx);
 	return ret;
@@ -2697,6 +2701,7 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
 {
 	/* NFSv4.0 doesn't allow for delegation recovery on open expire */
 	nfs40_clear_delegation_stateid(state);
+	nfs_state_clear_open_state_flags(state);
 	return nfs4_open_expired(sp, state);
 }
 
@@ -2739,13 +2744,13 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
 	return -NFS4ERR_EXPIRED;
 }
 
-static void nfs41_check_delegation_stateid(struct nfs4_state *state)
+static int nfs41_check_delegation_stateid(struct nfs4_state *state)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
 	nfs4_stateid stateid;
 	struct nfs_delegation *delegation;
 	const struct cred *cred = NULL;
-	int status;
+	int status, ret = NFS_OK;
 
 	/* Get the delegation credential for use by test/free_stateid */
 	rcu_read_lock();
@@ -2753,20 +2758,15 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	if (delegation == NULL) {
 		rcu_read_unlock();
 		nfs_state_clear_delegation(state);
-		return;
+		return NFS_OK;
 	}
 
 	nfs4_stateid_copy(&stateid, &delegation->stateid);
-	if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
-		rcu_read_unlock();
-		nfs_state_clear_delegation(state);
-		return;
-	}
 
 	if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED,
 				&delegation->flags)) {
 		rcu_read_unlock();
-		return;
+		return NFS_OK;
 	}
 
 	if (delegation->cred)
@@ -2776,8 +2776,24 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	trace_nfs4_test_delegation_stateid(state, NULL, status);
 	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
 		nfs_finish_clear_delegation_stateid(state, &stateid);
+	else
+		ret = status;
 
 	put_cred(cred);
+	return ret;
+}
+
+static void nfs41_delegation_recover_stateid(struct nfs4_state *state)
+{
+	nfs4_stateid tmp;
+
+	if (test_bit(NFS_DELEGATED_STATE, &state->flags) &&
+	    nfs4_copy_delegation_stateid(state->inode, state->state,
+				&tmp, NULL) &&
+	    nfs4_stateid_match_other(&state->stateid, &tmp))
+		nfs_state_set_delegation(state, &tmp, state->state);
+	else
+		nfs_state_clear_delegation(state);
 }
 
 /**
@@ -2847,21 +2863,12 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
 	const struct cred *cred = state->owner->so_cred;
 	int status;
 
-	if (test_bit(NFS_OPEN_STATE, &state->flags) == 0) {
-		if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)  {
-			if (nfs4_have_delegation(state->inode, state->state))
-				return NFS_OK;
-			return -NFS4ERR_OPENMODE;
-		}
+	if (test_bit(NFS_OPEN_STATE, &state->flags) == 0)
 		return -NFS4ERR_BAD_STATEID;
-	}
 	status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
 	trace_nfs4_test_open_stateid(state, NULL, status);
 	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) {
-		clear_bit(NFS_O_RDONLY_STATE, &state->flags);
-		clear_bit(NFS_O_WRONLY_STATE, &state->flags);
-		clear_bit(NFS_O_RDWR_STATE, &state->flags);
-		clear_bit(NFS_OPEN_STATE, &state->flags);
+		nfs_state_clear_open_state_flags(state);
 		stateid->type = NFS4_INVALID_STATEID_TYPE;
 		return status;
 	}
@@ -2874,7 +2881,11 @@ static int nfs41_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
 {
 	int status;
 
-	nfs41_check_delegation_stateid(state);
+	status = nfs41_check_delegation_stateid(state);
+	if (status != NFS_OK)
+		return status;
+	nfs41_delegation_recover_stateid(state);
+
 	status = nfs41_check_expired_locks(state);
 	if (status != NFS_OK)
 		return status;
-- 
2.21.0


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

* [PATCH 7/8] NFSv4.1: Only reap expired delegations
  2019-08-03 14:58         ` [PATCH 6/8] NFSv4.1: Fix open stateid recovery Trond Myklebust
@ 2019-08-03 14:58           ` Trond Myklebust
  2019-08-03 14:58             ` [PATCH 8/8] NFSv4: Check the return value of update_open_stateid() Trond Myklebust
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

Fix nfs_reap_expired_delegations() to ensure that we only reap delegations
that are actually expired, rather than triggering on random errors.

Fixes: 45870d6909d5a ("NFSv4.1: Test delegation stateids when server...")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/delegation.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 0af854cce8ff..071b90a45933 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -1046,6 +1046,22 @@ void nfs_test_expired_all_delegations(struct nfs_client *clp)
 	nfs4_schedule_state_manager(clp);
 }
 
+static void
+nfs_delegation_test_free_expired(struct inode *inode,
+		nfs4_stateid *stateid,
+		const struct cred *cred)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	const struct nfs4_minor_version_ops *ops = server->nfs_client->cl_mvops;
+	int status;
+
+	if (!cred)
+		return;
+	status = ops->test_and_free_expired(server, stateid, cred);
+	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
+		nfs_remove_bad_delegation(inode, stateid);
+}
+
 /**
  * nfs_reap_expired_delegations - reap expired delegations
  * @clp: nfs_client to process
@@ -1057,7 +1073,6 @@ void nfs_test_expired_all_delegations(struct nfs_client *clp)
  */
 void nfs_reap_expired_delegations(struct nfs_client *clp)
 {
-	const struct nfs4_minor_version_ops *ops = clp->cl_mvops;
 	struct nfs_delegation *delegation;
 	struct nfs_server *server;
 	struct inode *inode;
@@ -1088,11 +1103,7 @@ void nfs_reap_expired_delegations(struct nfs_client *clp)
 			nfs4_stateid_copy(&stateid, &delegation->stateid);
 			clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags);
 			rcu_read_unlock();
-			if (cred != NULL &&
-			    ops->test_and_free_expired(server, &stateid, cred) < 0) {
-				nfs_revoke_delegation(inode, &stateid);
-				nfs_inode_find_state_and_recover(inode, &stateid);
-			}
+			nfs_delegation_test_free_expired(inode, &stateid, cred);
 			put_cred(cred);
 			if (nfs4_server_rebooted(clp)) {
 				nfs_inode_mark_test_expired_delegation(server,inode);
-- 
2.21.0


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

* [PATCH 8/8] NFSv4: Check the return value of update_open_stateid()
  2019-08-03 14:58           ` [PATCH 7/8] NFSv4.1: Only reap expired delegations Trond Myklebust
@ 2019-08-03 14:58             ` Trond Myklebust
  2019-08-03 17:09               ` Sasha Levin
  0 siblings, 1 reply; 10+ messages in thread
From: Trond Myklebust @ 2019-08-03 14:58 UTC (permalink / raw)
  To: linux-nfs

Ensure that we always check the return value of update_open_stateid()
so that we can retry if the update of local state failed.

Fixes: e23008ec81ef3 ("NFSv4 reduce attribute requests for open reclaim")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: stable@vger.kernel.org # v3.7+
---
 fs/nfs/nfs4proc.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c9e14ce0b7b2..3e0b93f2b61a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1915,8 +1915,9 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data)
 	if (data->o_res.delegation_type != 0)
 		nfs4_opendata_check_deleg(data, state);
 update:
-	update_open_stateid(state, &data->o_res.stateid, NULL,
-			    data->o_arg.fmode);
+	if (!update_open_stateid(state, &data->o_res.stateid,
+				NULL, data->o_arg.fmode))
+		return ERR_PTR(-EAGAIN);
 	refcount_inc(&state->count);
 
 	return state;
@@ -1981,8 +1982,11 @@ _nfs4_opendata_to_nfs4_state(struct nfs4_opendata *data)
 
 	if (data->o_res.delegation_type != 0)
 		nfs4_opendata_check_deleg(data, state);
-	update_open_stateid(state, &data->o_res.stateid, NULL,
-			data->o_arg.fmode);
+	if (!update_open_stateid(state, &data->o_res.stateid,
+				NULL, data->o_arg.fmode)) {
+		nfs4_put_open_state(state);
+		state = ERR_PTR(-EAGAIN);
+	}
 out:
 	nfs_release_seqid(data->o_arg.seqid);
 	return state;
-- 
2.21.0


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

* Re: [PATCH 2/8] NFSv4: Fix delegation state recovery
  2019-08-03 14:58 ` [PATCH 2/8] NFSv4: Fix delegation state recovery Trond Myklebust
  2019-08-03 14:58   ` [PATCH 3/8] NFSv4: Print an error in the syslog when state is marked as irrecoverable Trond Myklebust
@ 2019-08-03 17:09   ` Sasha Levin
  1 sibling, 0 replies; 10+ messages in thread
From: Sasha Levin @ 2019-08-03 17:09 UTC (permalink / raw)
  To: Sasha Levin, Trond Myklebust, linux-nfs; +Cc: stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 24311f884189 NFSv4: Recovery of recalled read delegations is broken.

The bot has tested the following trees: v5.2.5, v4.19.63, v4.14.135, v4.9.186, v4.4.186.

v5.2.5: Build OK!
v4.19.63: Failed to apply! Possible dependencies:
    07d02a67b7fa ("SUNRPC: Simplify lookup code")
    79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
    8276c902bbe9 ("SUNRPC: remove uid and gid from struct auth_cred")
    95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
    97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.14.135: Failed to apply! Possible dependencies:
    07d02a67b7fa ("SUNRPC: Simplify lookup code")
    12f275cdd163 ("NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.")
    1eb5d98f16f6 ("nfs: convert to new i_version API")
    35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
    79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
    95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
    97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.9.186: Failed to apply! Possible dependencies:
    1eb5d98f16f6 ("nfs: convert to new i_version API")
    35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
    39bc88e5e38e ("arm64: Disable TTBR0_EL1 during normal kernel execution")
    7c0f6ba682b9 ("Replace <asm/uaccess.h> with <linux/uaccess.h> globally")
    9cf09d68b89a ("arm64: xen: Enable user access before a privcmd hvc call")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    bd38967d406f ("arm64: Factor out PAN enabling/disabling into separate uaccess_* macros")

v4.4.186: Failed to apply! Possible dependencies:
    0654cc726fc6 ("NFSv4.1/pNFS: Add a helper to mark the layout as returned")
    10335556c9e6 ("NFSv4.1/pNFS: pnfs_error_mark_layout_for_return() must always return layout")
    13c13a6ad71f ("pNFS: Fix missing layoutreturn calls")
    2454dfea0aef ("NFSv4.x/pnfs: Fix a race between layoutget and pnfs_destroy_layout")
    3982a6a2d0e6 ("pnfs: keep track of the return sequence number in pnfs_layout_hdr")
    4b0934baf931 ("NFSv4.1/pNFS: Fix a race in initiate_file_draining()")
    506c0d68269e ("NFSv4.1/pNFS: Cleanup constify struct pnfs_layout_range arguments")
    50f563ef5d41 ("NFSv4.1/pNFS: Use nfs4_stateid_copy for copying stateids")
    5c97f5de2c7c ("NFSv4.1/pNFS: pnfs_mark_matching_lsegs_return() should set the iomode")
    68d264cf02b0 ("NFS42: handle layoutstats stateid error")
    6d597e175012 ("pnfs: only tear down lsegs that precede seqid in LAYOUTRETURN args")
    71b39854a500 ("NFSv4.1/pNFS: Cleanup pnfs_mark_matching_lsegs_invalid()")
    9fd4b9fc7695 ("NFSv4.x/pnfs: Fix a race between layoutget and bulk recalls")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b20135d0b243 ("NFSv4.1/pNFS: Don't queue up a new commit if the layout segment is invalid")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    e036f46453f2 ("NFS: pnfs_mark_matching_lsegs_return() should match the layout sequence id")
    e0d9243048fd ("NFSv4.1/pNFS: Don't return NFS4ERR_DELAY unnecessarily in CB_LAYOUTRECALL")
    ed429d6b934d ("NFSv4.1/pNFS: Don't pass stateids by value to pnfs_send_layoutreturn()")
    fc7ff36747b9 ("pNFS: If we have to delay the layout callback, mark the layout for return")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks,
Sasha

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

* Re: [PATCH 8/8] NFSv4: Check the return value of update_open_stateid()
  2019-08-03 14:58             ` [PATCH 8/8] NFSv4: Check the return value of update_open_stateid() Trond Myklebust
@ 2019-08-03 17:09               ` Sasha Levin
  0 siblings, 0 replies; 10+ messages in thread
From: Sasha Levin @ 2019-08-03 17:09 UTC (permalink / raw)
  To: Sasha Levin, Trond Myklebust, linux-nfs; +Cc: stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: e23008ec81ef NFSv4 reduce attribute requests for open reclaim.

The bot has tested the following trees: v5.2.5, v4.19.63, v4.14.135, v4.9.186, v4.4.186.

v5.2.5: Build OK!
v4.19.63: Failed to apply! Possible dependencies:
    ace9fad43aa6 ("NFSv4: Convert struct nfs4_state to use refcount_t")

v4.14.135: Failed to apply! Possible dependencies:
    ace9fad43aa6 ("NFSv4: Convert struct nfs4_state to use refcount_t")
    c9399f21c215 ("NFSv4: Fix OPEN / CLOSE race")

v4.9.186: Failed to apply! Possible dependencies:
    4e2fcac77390 ("NFSv4: Use correct inode in _nfs4_opendata_to_nfs4_state()")
    75e8c48b9ef3 ("NFSv4: Use the nfs4_state being recovered in _nfs4_opendata_to_nfs4_state()")
    ace9fad43aa6 ("NFSv4: Convert struct nfs4_state to use refcount_t")
    c9399f21c215 ("NFSv4: Fix OPEN / CLOSE race")

v4.4.186: Failed to apply! Possible dependencies:
    1393d9612ba0 ("NFSv4: Fix a race when updating an open_stateid")
    4586f6e28327 ("NFSv4.1: Add a helper function to deal with expired stateids")
    4e2fcac77390 ("NFSv4: Use correct inode in _nfs4_opendata_to_nfs4_state()")
    75e8c48b9ef3 ("NFSv4: Use the nfs4_state being recovered in _nfs4_opendata_to_nfs4_state()")
    8a64c4ef106d ("NFSv4.1: Even if the stateid is OK, we may need to recover the open modes")
    a8ce377a5db8 ("nfs: track whether server sets MAY_NOTIFY_LOCK flag")
    ace9fad43aa6 ("NFSv4: Convert struct nfs4_state to use refcount_t")
    b134fc4a5333 ("NFSv4: Don't test open_stateid unless it is set")
    c9399f21c215 ("NFSv4: Fix OPEN / CLOSE race")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks,
Sasha

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

end of thread, other threads:[~2019-08-03 17:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-03 14:58 [PATCH 1/8] NFSv4: Fix a credential refcount leak in nfs41_check_delegation_stateid Trond Myklebust
2019-08-03 14:58 ` [PATCH 2/8] NFSv4: Fix delegation state recovery Trond Myklebust
2019-08-03 14:58   ` [PATCH 3/8] NFSv4: Print an error in the syslog when state is marked as irrecoverable Trond Myklebust
2019-08-03 14:58     ` [PATCH 4/8] NFSv4: When recovering state fails with EAGAIN, retry the same recovery Trond Myklebust
2019-08-03 14:58       ` [PATCH 5/8] NFSv4: Report the error from nfs4_select_rw_stateid() Trond Myklebust
2019-08-03 14:58         ` [PATCH 6/8] NFSv4.1: Fix open stateid recovery Trond Myklebust
2019-08-03 14:58           ` [PATCH 7/8] NFSv4.1: Only reap expired delegations Trond Myklebust
2019-08-03 14:58             ` [PATCH 8/8] NFSv4: Check the return value of update_open_stateid() Trond Myklebust
2019-08-03 17:09               ` Sasha Levin
2019-08-03 17:09   ` [PATCH 2/8] NFSv4: Fix delegation state recovery Sasha Levin

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