[PATCH v3 00/12] Fix delegation behaviour when server revokes some state

[PATCH v3 00/12] Fix delegation behaviour when server revokes some state

[Trond Myklebust]

According to RFC5661, if any of the SEQUENCE status bits
SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED, SEQ4_STATUS_ADMIN_STATE_REVOKED,
or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to use
TEST_STATEID to figure out which stateids have been revoked, so we
can acknowledge the loss of state using FREE_STATEID.

While we already do this for open and lock state, we have not been doing
so for all the delegations.

v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
v3: Now with added lock revoke fixes and close/delegreturn/locku fixes

Trond Myklebust (12):
  NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
  NFSv4.1: Don't check delegations that are already marked as revoked
  NFSv4.1: Allow test_stateid to handle session errors without waiting
  NFSv4.1: Add a helper function to deal with expired stateids
  NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a
    stateid
  NFSv4.1: Test delegation stateids when server declares "some state
    revoked"
  NFSv4.1: Deal with server reboots during delegation expiration
    recovery
  NFSv4.1: Don't recheck delegations that have already been checked
  NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
  NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
  NFSv4.1: FREE_STATEID can be asynchronous
  NFSv4.1: Ensure we call FREE_STATEID if needed on
    close/delegreturn/locku

 fs/nfs/delegation.c                    | 153 ++++++++++++++++++-
 fs/nfs/delegation.h                    |   6 +-
 fs/nfs/flexfilelayout/flexfilelayout.c |   2 +-
 fs/nfs/nfs4_fs.h                       |   5 +-
 fs/nfs/nfs4proc.c                      | 258 ++++++++++++++++++++++-----------
 fs/nfs/nfs4session.h                   |   1 +
 fs/nfs/nfs4state.c                     |  29 ++--
 include/linux/nfs4.h                   |   1 +
 8 files changed, 348 insertions(+), 107 deletions(-)

-- 
2.7.4

[PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags

[Trond Myklebust]

As described in RFC5661, section 18.46, some of the status flags exist
in order to tell the client when it needs to acknowledge the existence of
revoked state on the server and/or to recover state.
Those flags will then remain set until the recovery procedure is done.

In order to avoid looping, the client therefore needs to ignore
those particular flags while recovering.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4_fs.h     |  2 +-
 fs/nfs/nfs4proc.c    |  5 ++++-
 fs/nfs/nfs4session.h |  1 +
 fs/nfs/nfs4state.c   | 12 +++++++++++-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index f230aa62ca59..4390d73a92e5 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -439,7 +439,7 @@ extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
 extern int nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *);
 extern int nfs4_schedule_migration_recovery(const struct nfs_server *);
 extern void nfs4_schedule_lease_moved_recovery(struct nfs_client *);
-extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags);
+extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags, bool);
 extern void nfs41_handle_server_scope(struct nfs_client *,
 				      struct nfs41_server_scope **);
 extern void nfs4_put_lock_state(struct nfs4_lock_state *lsp);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 25a2af707233..bd33777f03c4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -616,6 +616,7 @@ int nfs40_setup_sequence(struct nfs4_slot_table *tbl,
 	}
 	spin_unlock(&tbl->slot_tbl_lock);
 
+	slot->privileged = args->sa_privileged ? 1 : 0;
 	args->sa_slot = slot;
 	res->sr_slot = slot;
 
@@ -728,7 +729,8 @@ static int nfs41_sequence_process(struct rpc_task *task,
 		clp = session->clp;
 		do_renew_lease(clp, res->sr_timestamp);
 		/* Check sequence flags */
-		nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags);
+		nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags,
+				!!slot->privileged);
 		nfs41_update_target_slotid(slot->table, slot, res);
 		break;
 	case 1:
@@ -875,6 +877,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
 	}
 	spin_unlock(&tbl->slot_tbl_lock);
 
+	slot->privileged = args->sa_privileged ? 1 : 0;
 	args->sa_slot = slot;
 
 	dprintk("<-- %s slotid=%u seqid=%u\n", __func__,
diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
index 3bb6af70973c..dae385500005 100644
--- a/fs/nfs/nfs4session.h
+++ b/fs/nfs/nfs4session.h
@@ -23,6 +23,7 @@ struct nfs4_slot {
 	u32			slot_nr;
 	u32		 	seq_nr;
 	unsigned int		interrupted : 1,
+				privileged : 1,
 				seq_done : 1;
 };
 
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index cada00aa5096..9801b5bb5fac 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2227,13 +2227,22 @@ static void nfs41_handle_cb_path_down(struct nfs_client *clp)
 		nfs4_schedule_state_manager(clp);
 }
 
-void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
+void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags,
+		bool recovery)
 {
 	if (!flags)
 		return;
 
 	dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
 		__func__, clp->cl_hostname, clp->cl_clientid, flags);
+	/*
+	 * If we're called from the state manager thread, then assume we're
+	 * already handling the RECLAIM_NEEDED and/or STATE_REVOKED.
+	 * Those flags are expected to remain set until we're done
+	 * recovering (see RFC5661, section 18.46.3).
+	 */
+	if (recovery)
+		goto out_recovery;
 
 	if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
 		nfs41_handle_server_reboot(clp);
@@ -2246,6 +2255,7 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
 		nfs4_schedule_lease_moved_recovery(clp);
 	if (flags & SEQ4_STATUS_RECALLABLE_STATE_REVOKED)
 		nfs41_handle_recallable_state_revoked(clp);
+out_recovery:
 	if (flags & SEQ4_STATUS_BACKCHANNEL_FAULT)
 		nfs41_handle_backchannel_fault(clp);
 	else if (flags & (SEQ4_STATUS_CB_PATH_DOWN |
-- 
2.7.4

[PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked

[Trond Myklebust]

If the delegation has been marked as revoked, we don't have to test
it, because we should already have called FREE_STATEID on it.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index bd33777f03c4..254ab545c391 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2423,6 +2423,11 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 		rcu_read_unlock();
 		return;
 	}
+	if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
+		rcu_read_unlock();
+		nfs_finish_clear_delegation_stateid(state);
+		return;
+	}
 
 	nfs4_stateid_copy(&stateid, &delegation->stateid);
 	cred = get_rpccred(delegation->cred);
-- 
2.7.4

[PATCH v3 03/12] NFSv4.1: Allow test_stateid to handle session errors without waiting

[Trond Myklebust]

If the server crashes while we're testing stateids for validity, then
we want to initiate session recovery. Usually, we will be calling from
a state manager thread, though, so we don't really want to wait.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 254ab545c391..9b7ff26b63ff 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8590,6 +8590,23 @@ static int _nfs41_test_stateid(struct nfs_server *server,
 	return -res.status;
 }
 
+static void nfs4_handle_delay_or_session_error(struct nfs_server *server,
+		int err, struct nfs4_exception *exception)
+{
+	exception->retry = 0;
+	switch(err) {
+	case -NFS4ERR_DELAY:
+		nfs4_handle_exception(server, err, exception);
+		break;
+	case -NFS4ERR_BADSESSION:
+	case -NFS4ERR_BADSLOT:
+	case -NFS4ERR_BAD_HIGH_SLOT:
+	case -NFS4ERR_CONN_NOT_BOUND_TO_SESSION:
+	case -NFS4ERR_DEADSESSION:
+		nfs4_do_handle_exception(server, err, exception);
+	}
+}
+
 /**
  * nfs41_test_stateid - perform a TEST_STATEID operation
  *
@@ -8609,9 +8626,7 @@ static int nfs41_test_stateid(struct nfs_server *server,
 	int err;
 	do {
 		err = _nfs41_test_stateid(server, stateid, cred);
-		if (err != -NFS4ERR_DELAY)
-			break;
-		nfs4_handle_exception(server, err, &exception);
+		nfs4_handle_delay_or_session_error(server, err, &exception);
 	} while (exception.retry);
 	return err;
 }
-- 
2.7.4

[PATCH v3 04/12] NFSv4.1: Add a helper function to deal with expired stateids

[Trond Myklebust]

In NFSv4.1 and newer, if the server decides to revoke some or all of
the protocol state, the client is required to iterate through all the
stateids that it holds and call TEST_STATEID to determine which stateids
still correspond to valid state, and then call FREE_STATEID on the
others.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 9b7ff26b63ff..2fe3445cb7b4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2408,6 +2408,26 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
 }
 
 #if defined(CONFIG_NFS_V4_1)
+static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
+		nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	int status;
+
+	status = nfs41_test_stateid(server, stateid, cred);
+
+	switch (status) {
+	case -NFS4ERR_EXPIRED:
+	case -NFS4ERR_ADMIN_REVOKED:
+	case -NFS4ERR_DELEG_REVOKED:
+		/* Ack the revoked state to the server */
+		nfs41_free_stateid(server, stateid, cred);
+	case -NFS4ERR_BAD_STATEID:
+		return status;
+	}
+	return NFS_OK;
+}
+
 static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
@@ -2432,16 +2452,10 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	nfs4_stateid_copy(&stateid, &delegation->stateid);
 	cred = get_rpccred(delegation->cred);
 	rcu_read_unlock();
-	status = nfs41_test_stateid(server, &stateid, cred);
+	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
 	trace_nfs4_test_delegation_stateid(state, NULL, status);
-
-	if (status != NFS_OK) {
-		/* Free the stateid unless the server explicitly
-		 * informs us the stateid is unrecognized. */
-		if (status != -NFS4ERR_BAD_STATEID)
-			nfs41_free_stateid(server, &stateid, cred);
+	if (status != NFS_OK)
 		nfs_finish_clear_delegation_stateid(state);
-	}
 
 	put_rpccred(cred);
 }
@@ -2467,14 +2481,9 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
 	    (test_bit(NFS_O_RDWR_STATE, &state->flags) == 0))
 		return -NFS4ERR_BAD_STATEID;
 
-	status = nfs41_test_stateid(server, stateid, cred);
+	status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
 	trace_nfs4_test_open_stateid(state, NULL, status);
 	if (status != NFS_OK) {
-		/* Free the stateid unless the server explicitly
-		 * informs us the stateid is unrecognized. */
-		if (status != -NFS4ERR_BAD_STATEID)
-			nfs41_free_stateid(server, stateid, cred);
-
 		clear_bit(NFS_O_RDONLY_STATE, &state->flags);
 		clear_bit(NFS_O_WRONLY_STATE, &state->flags);
 		clear_bit(NFS_O_RDWR_STATE, &state->flags);
@@ -6109,17 +6118,11 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
 		if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) {
 			struct rpc_cred *cred = lsp->ls_state->owner->so_cred;
 
-			status = nfs41_test_stateid(server,
+			status = nfs41_test_and_free_expired_stateid(server,
 					&lsp->ls_stateid,
 					cred);
 			trace_nfs4_test_lock_stateid(state, lsp, status);
 			if (status != NFS_OK) {
-				/* Free the stateid unless the server
-				 * informs us the stateid is unrecognized. */
-				if (status != -NFS4ERR_BAD_STATEID)
-					nfs41_free_stateid(server,
-							&lsp->ls_stateid,
-							cred);
 				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
 				ret = status;
 			}
-- 
2.7.4

[PATCH v3 05/12] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid

[Trond Myklebust]

Allow the callers of nfs_remove_bad_delegation() to specify the stateid
that needs to be marked as bad.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/delegation.c                    | 30 +++++++++++++++++++++++-------
 fs/nfs/delegation.h                    |  2 +-
 fs/nfs/flexfilelayout/flexfilelayout.c |  2 +-
 fs/nfs/nfs4proc.c                      | 14 ++++++++------
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 322c2585bc34..76dc11ecdedd 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -642,23 +642,39 @@ static void nfs_client_mark_return_unused_delegation_types(struct nfs_client *cl
 	rcu_read_unlock();
 }
 
-static void nfs_revoke_delegation(struct inode *inode)
+static void nfs_mark_delegation_revoked(struct nfs_server *server,
+		struct nfs_delegation *delegation)
+{
+	set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
+	nfs_mark_return_delegation(server, delegation);
+}
+
+static bool nfs_revoke_delegation(struct inode *inode,
+		const nfs4_stateid *stateid)
 {
 	struct nfs_delegation *delegation;
+	bool ret = false;
+
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
-	if (delegation != NULL) {
-		set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
-		nfs_mark_return_delegation(NFS_SERVER(inode), delegation);
-	}
+	if (delegation == NULL)
+		goto out;
+	if (stateid && !nfs4_stateid_match(stateid, &delegation->stateid))
+		goto out;
+	nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
+	ret = true;
+out:
 	rcu_read_unlock();
+	return ret;
 }
 
-void nfs_remove_bad_delegation(struct inode *inode)
+void nfs_remove_bad_delegation(struct inode *inode,
+		const nfs4_stateid *stateid)
 {
 	struct nfs_delegation *delegation;
 
-	nfs_revoke_delegation(inode);
+	if (!nfs_revoke_delegation(inode, stateid))
+		return;
 	delegation = nfs_inode_detach_delegation(inode);
 	if (delegation) {
 		nfs_inode_find_state_and_recover(inode, &delegation->stateid);
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 64724d252a79..d40827af5913 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -47,7 +47,7 @@ void nfs_expire_unused_delegation_types(struct nfs_client *clp, fmode_t flags);
 void nfs_expire_unreferenced_delegations(struct nfs_client *clp);
 int nfs_client_return_marked_delegations(struct nfs_client *clp);
 int nfs_delegations_present(struct nfs_client *clp);
-void nfs_remove_bad_delegation(struct inode *inode);
+void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid);
 
 void nfs_delegation_mark_reclaim(struct nfs_client *clp);
 void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 51b51369704c..98ace127bf86 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1080,7 +1080,7 @@ static int ff_layout_async_handle_error_v4(struct rpc_task *task,
 	case -NFS4ERR_BAD_STATEID:
 		if (state == NULL)
 			break;
-		nfs_remove_bad_delegation(state->inode);
+		nfs_remove_bad_delegation(state->inode, NULL);
 	case -NFS4ERR_OPENMODE:
 		if (state == NULL)
 			break;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 2fe3445cb7b4..0ae39e3b13e1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2385,9 +2385,10 @@ static int nfs4_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *sta
 	return ret;
 }
 
-static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state)
+static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state,
+		const nfs4_stateid *stateid)
 {
-	nfs_remove_bad_delegation(state->inode);
+	nfs_remove_bad_delegation(state->inode, stateid);
 	write_seqlock(&state->seqlock);
 	nfs4_stateid_copy(&state->stateid, &state->open_stateid);
 	write_sequnlock(&state->seqlock);
@@ -2397,7 +2398,7 @@ static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state)
 static void nfs40_clear_delegation_stateid(struct nfs4_state *state)
 {
 	if (rcu_access_pointer(NFS_I(state->inode)->delegation) != NULL)
-		nfs_finish_clear_delegation_stateid(state);
+		nfs_finish_clear_delegation_stateid(state, NULL);
 }
 
 static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *state)
@@ -2443,19 +2444,20 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 		rcu_read_unlock();
 		return;
 	}
+
+	nfs4_stateid_copy(&stateid, &delegation->stateid);
 	if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
 		rcu_read_unlock();
-		nfs_finish_clear_delegation_stateid(state);
+		nfs_finish_clear_delegation_stateid(state, &stateid);
 		return;
 	}
 
-	nfs4_stateid_copy(&stateid, &delegation->stateid);
 	cred = get_rpccred(delegation->cred);
 	rcu_read_unlock();
 	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
 	trace_nfs4_test_delegation_stateid(state, NULL, status);
 	if (status != NFS_OK)
-		nfs_finish_clear_delegation_stateid(state);
+		nfs_finish_clear_delegation_stateid(state, &stateid);
 
 	put_rpccred(cred);
 }
-- 
2.7.4

[PATCH v3 06/12] NFSv4.1: Test delegation stateids when server declares "some state revoked"

[Trond Myklebust]

According to RFC5661, if any of the SEQUENCE status bits
SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED, SEQ4_STATUS_ADMIN_STATE_REVOKED,
or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to use
TEST_STATEID to figure out which stateids have been revoked, so we
can acknowledge the loss of state using FREE_STATEID.

While we already do this for open and lock state, we have not been doing
so for all the delegations.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/delegation.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/nfs/delegation.h |  4 +++
 fs/nfs/nfs4_fs.h    |  3 ++
 fs/nfs/nfs4proc.c   | 10 ++++++
 fs/nfs/nfs4state.c  | 17 +++++-----
 5 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 76dc11ecdedd..ff0c1a0feb9d 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -802,8 +802,15 @@ static void nfs_delegation_mark_reclaim_server(struct nfs_server *server)
 {
 	struct nfs_delegation *delegation;
 
-	list_for_each_entry_rcu(delegation, &server->delegations, super_list)
+	list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
+		/*
+		 * If the delegation may have been admin revoked, then we
+		 * cannot reclaim it.
+		 */
+		if (test_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags))
+			continue;
 		set_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
+	}
 }
 
 /**
@@ -867,6 +874,94 @@ restart:
 	rcu_read_unlock();
 }
 
+static void nfs_mark_test_expired_delegation(struct nfs_server *server,
+	    struct nfs_delegation *delegation)
+{
+	clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
+	set_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags);
+	set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state);
+}
+
+static void nfs_delegation_mark_test_expired_server(struct nfs_server *server)
+{
+	struct nfs_delegation *delegation;
+
+	list_for_each_entry_rcu(delegation, &server->delegations, super_list)
+		nfs_mark_test_expired_delegation(server, delegation);
+}
+
+/**
+ * nfs_mark_test_expired_all_delegations - mark all delegations for testing
+ * @clp: nfs_client to process
+ *
+ * Iterates through all the delegations associated with this server and
+ * marks them as needing to be checked for validity.
+ */
+void nfs_mark_test_expired_all_delegations(struct nfs_client *clp)
+{
+	struct nfs_server *server;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link)
+		nfs_delegation_mark_test_expired_server(server);
+	rcu_read_unlock();
+}
+
+/**
+ * nfs_reap_expired_delegations - reap expired delegations
+ * @clp: nfs_client to process
+ *
+ * Iterates through all the delegations associated with this server and
+ * checks if they have may have been revoked. This function is usually
+ * expected to be called in cases where the server may have lost its
+ * lease.
+ */
+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;
+	struct rpc_cred *cred;
+	nfs4_stateid stateid;
+
+restart:
+	rcu_read_lock();
+	list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
+		list_for_each_entry_rcu(delegation, &server->delegations,
+								super_list) {
+			if (test_bit(NFS_DELEGATION_RETURNING,
+						&delegation->flags))
+				continue;
+			if (test_bit(NFS_DELEGATION_TEST_EXPIRED,
+						&delegation->flags) == 0)
+				continue;
+			if (!nfs_sb_active(server->super))
+				continue;
+			inode = nfs_delegation_grab_inode(delegation);
+			if (inode == NULL) {
+				rcu_read_unlock();
+				nfs_sb_deactive(server->super);
+				goto restart;
+			}
+			cred = get_rpccred_rcu(delegation->cred);
+			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);
+			}
+			put_rpccred(cred);
+			iput(inode);
+			nfs_sb_deactive(server->super);
+			goto restart;
+		}
+	}
+	rcu_read_unlock();
+}
+
 /**
  * nfs_delegations_present - check for existence of delegations
  * @clp: client state handle
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index d40827af5913..1442e3b1521d 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -32,6 +32,7 @@ enum {
 	NFS_DELEGATION_REFERENCED,
 	NFS_DELEGATION_RETURNING,
 	NFS_DELEGATION_REVOKED,
+	NFS_DELEGATION_TEST_EXPIRED,
 };
 
 int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res);
@@ -52,6 +53,9 @@ void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid)
 void nfs_delegation_mark_reclaim(struct nfs_client *clp);
 void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
 
+void nfs_mark_test_expired_all_delegations(struct nfs_client *clp);
+void nfs_reap_expired_delegations(struct nfs_client *clp);
+
 /* NFSv4 delegation-related procedures */
 int nfs4_proc_delegreturn(struct inode *inode, struct rpc_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);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4390d73a92e5..b9083a6cefd6 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -39,6 +39,7 @@ enum nfs4_client_state {
 	NFS4CLNT_BIND_CONN_TO_SESSION,
 	NFS4CLNT_MOVED,
 	NFS4CLNT_LEASE_MOVED,
+	NFS4CLNT_DELEGATION_EXPIRED,
 };
 
 #define NFS4_RENEW_TIMEOUT		0x01
@@ -57,6 +58,8 @@ struct nfs4_minor_version_ops {
 			struct nfs_fsinfo *);
 	void	(*free_lock_state)(struct nfs_server *,
 			struct nfs4_lock_state *);
+	int	(*test_and_free_expired)(struct nfs_server *,
+			nfs4_stateid *, struct rpc_cred *);
 	struct nfs_seqid *
 		(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
 	const struct rpc_call_ops *call_sync_ops;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0ae39e3b13e1..74dd9157539a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2408,6 +2408,13 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
 	return nfs4_open_expired(sp, state);
 }
 
+static int nfs40_test_and_free_expired_stateid(struct nfs_server *server,
+		nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	return -NFS4ERR_BAD_STATEID;
+}
+
 #if defined(CONFIG_NFS_V4_1)
 static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
 		nfs4_stateid *stateid,
@@ -8854,6 +8861,7 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
 	.match_stateid = nfs4_match_stateid,
 	.find_root_sec = nfs4_find_root_sec,
 	.free_lock_state = nfs4_release_lockowner,
+	.test_and_free_expired = nfs40_test_and_free_expired_stateid,
 	.alloc_seqid = nfs_alloc_seqid,
 	.call_sync_ops = &nfs40_call_sync_ops,
 	.reboot_recovery_ops = &nfs40_reboot_recovery_ops,
@@ -8881,6 +8889,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
 	.match_stateid = nfs41_match_stateid,
 	.find_root_sec = nfs41_find_root_sec,
 	.free_lock_state = nfs41_free_lock_state,
+	.test_and_free_expired = nfs41_test_and_free_expired_stateid,
 	.alloc_seqid = nfs_alloc_no_seqid,
 	.call_sync_ops = &nfs41_call_sync_ops,
 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
@@ -8910,6 +8919,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
 	.find_root_sec = nfs41_find_root_sec,
 	.free_lock_state = nfs41_free_lock_state,
 	.call_sync_ops = &nfs41_call_sync_ops,
+	.test_and_free_expired = nfs41_test_and_free_expired_stateid,
 	.alloc_seqid = nfs_alloc_no_seqid,
 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9801b5bb5fac..63da0411e2af 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1656,15 +1656,9 @@ static void nfs4_state_end_reclaim_reboot(struct nfs_client *clp)
 	put_rpccred(cred);
 }
 
-static void nfs_delegation_clear_all(struct nfs_client *clp)
-{
-	nfs_delegation_mark_reclaim(clp);
-	nfs_delegation_reap_unclaimed(clp);
-}
-
 static void nfs4_state_start_reclaim_nograce(struct nfs_client *clp)
 {
-	nfs_delegation_clear_all(clp);
+	nfs_mark_test_expired_all_delegations(clp);
 	nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce);
 }
 
@@ -2195,7 +2189,7 @@ static void nfs41_handle_all_state_revoked(struct nfs_client *clp)
 
 static void nfs41_handle_some_state_revoked(struct nfs_client *clp)
 {
-	nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce);
+	nfs4_state_start_reclaim_nograce(clp);
 	nfs4_schedule_state_manager(clp);
 
 	dprintk("%s: state revoked on server %s\n", __func__, clp->cl_hostname);
@@ -2420,6 +2414,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
 			nfs4_state_end_reclaim_reboot(clp);
 		}
 
+		/* Detect expired delegations... */
+		if (test_and_clear_bit(NFS4CLNT_DELEGATION_EXPIRED, &clp->cl_state)) {
+			section = "detect expired delegations";
+			nfs_reap_expired_delegations(clp);
+			continue;
+		}
+
 		/* Now recover expired state... */
 		if (test_and_clear_bit(NFS4CLNT_RECLAIM_NOGRACE, &clp->cl_state)) {
 			section = "reclaim nograce";
-- 
2.7.4

[PATCH v3 07/12] NFSv4.1: Deal with server reboots during delegation expiration recovery

[Trond Myklebust]

Ensure that if the server reboots while we're testing and recovering
from revoked delegations, we exit to allow the state manager to
handle matters.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/delegation.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index ff0c1a0feb9d..b873271c613a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -874,6 +874,13 @@ restart:
 	rcu_read_unlock();
 }
 
+static inline bool nfs4_server_rebooted(const struct nfs_client *clp)
+{
+	return (clp->cl_state & (BIT(NFS4CLNT_CHECK_LEASE) |
+				BIT(NFS4CLNT_LEASE_EXPIRED) |
+				BIT(NFS4CLNT_SESSION_RESET))) != 0;
+}
+
 static void nfs_mark_test_expired_delegation(struct nfs_server *server,
 	    struct nfs_delegation *delegation)
 {
@@ -882,6 +889,19 @@ static void nfs_mark_test_expired_delegation(struct nfs_server *server,
 	set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state);
 }
 
+static void nfs_inode_mark_test_expired_delegation(struct nfs_server *server,
+		struct inode *inode)
+{
+	struct nfs_delegation *delegation;
+
+	rcu_read_lock();
+	delegation = rcu_dereference(NFS_I(inode)->delegation);
+	if (delegation)
+		nfs_mark_test_expired_delegation(server, delegation);
+	rcu_read_unlock();
+
+}
+
 static void nfs_delegation_mark_test_expired_server(struct nfs_server *server)
 {
 	struct nfs_delegation *delegation;
@@ -954,6 +974,12 @@ restart:
 				nfs_inode_find_state_and_recover(inode, &stateid);
 			}
 			put_rpccred(cred);
+			if (nfs4_server_rebooted(clp)) {
+				nfs_inode_mark_test_expired_delegation(server,inode);
+				iput(inode);
+				nfs_sb_deactive(server->super);
+				return;
+			}
 			iput(inode);
 			nfs_sb_deactive(server->super);
 			goto restart;
-- 
2.7.4

[PATCH v3 08/12] NFSv4.1: Don't recheck delegations that have already been checked

[Trond Myklebust]

Ensure we don't spam the server with test_stateid() calls for
delegations that have already been checked.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 74dd9157539a..6cae73a08ba0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2459,6 +2459,11 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 		return;
 	}
 
+	if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags)) {
+		rcu_read_unlock();
+		return;
+	}
+
 	cred = get_rpccred(delegation->cred);
 	rcu_read_unlock();
 	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
-- 
2.7.4

[PATCH v3 09/12] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID

[Trond Myklebust]

In some cases (e.g. when the SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED sequence
flag is set) we may already know that the stateid was revoked and that the
only valid operation we can call is FREE_STATEID. In those cases, allow
the stateid to carry the information in the type field, so that we skip
the redundant call to TEST_STATEID.
---
 fs/nfs/nfs4proc.c    | 32 +++++++++++++++++++++++---------
 include/linux/nfs4.h |  1 +
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6cae73a08ba0..6484148f1c3d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2422,18 +2422,29 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
 {
 	int status;
 
-	status = nfs41_test_stateid(server, stateid, cred);
+	switch (stateid->type) {
+	default:
+		break;
+	case NFS4_INVALID_STATEID_TYPE:
+	case NFS4_SPECIAL_STATEID_TYPE:
+		return -NFS4ERR_BAD_STATEID;
+	case NFS4_REVOKED_STATEID_TYPE:
+		goto out_free;
+	}
 
+	status = nfs41_test_stateid(server, stateid, cred);
 	switch (status) {
 	case -NFS4ERR_EXPIRED:
 	case -NFS4ERR_ADMIN_REVOKED:
 	case -NFS4ERR_DELEG_REVOKED:
-		/* Ack the revoked state to the server */
-		nfs41_free_stateid(server, stateid, cred);
-	case -NFS4ERR_BAD_STATEID:
+		break;
+	default:
 		return status;
 	}
-	return NFS_OK;
+out_free:
+	/* Ack the revoked state to the server */
+	nfs41_free_stateid(server, stateid, cred);
+	return -NFS4ERR_EXPIRED;
 }
 
 static void nfs41_check_delegation_stateid(struct nfs4_state *state)
@@ -2468,7 +2479,7 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	rcu_read_unlock();
 	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
 	trace_nfs4_test_delegation_stateid(state, NULL, status);
-	if (status != NFS_OK)
+	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
 		nfs_finish_clear_delegation_stateid(state, &stateid);
 
 	put_rpccred(cred);
@@ -2497,7 +2508,7 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
 
 	status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
 	trace_nfs4_test_open_stateid(state, NULL, status);
-	if (status != NFS_OK) {
+	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);
@@ -6124,7 +6135,7 @@ out:
  */
 static int nfs41_check_expired_locks(struct nfs4_state *state)
 {
-	int status, ret = -NFS4ERR_BAD_STATEID;
+	int status, ret = NFS_OK;
 	struct nfs4_lock_state *lsp;
 	struct nfs_server *server = NFS_SERVER(state->inode);
 
@@ -6136,9 +6147,12 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
 					&lsp->ls_stateid,
 					cred);
 			trace_nfs4_test_lock_stateid(state, lsp, status);
-			if (status != NFS_OK) {
+			if (status == -NFS4ERR_EXPIRED ||
+			    status == -NFS4ERR_BAD_STATEID)
 				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
+			else if (status != NFS_OK) {
 				ret = status;
+				break;
 			}
 		}
 	};
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index c6564ada9beb..9094faf0699d 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -67,6 +67,7 @@ struct nfs4_stateid_struct {
 		NFS4_DELEGATION_STATEID_TYPE,
 		NFS4_LAYOUT_STATEID_TYPE,
 		NFS4_PNFS_DS_STATEID_TYPE,
+		NFS4_REVOKED_STATEID_TYPE,
 	} type;
 };
 
-- 
2.7.4

[PATCH v3 10/12] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks

[Trond Myklebust]

Right now, we're only running TEST/FREE_STATEID on the locks if
the open stateid recovery succeeds. The protocol requires us to
always do so.
The fix would be to move the call to TEST/FREE_STATEID and do it
before we attempt open recovery.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 92 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6484148f1c3d..c7e9f91c5cfb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2486,6 +2486,45 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 }
 
 /**
+ * nfs41_check_expired_locks - possibly free a lock stateid
+ *
+ * @state: NFSv4 state for an inode
+ *
+ * Returns NFS_OK if recovery for this stateid is now finished.
+ * Otherwise a negative NFS4ERR value is returned.
+ */
+static int nfs41_check_expired_locks(struct nfs4_state *state)
+{
+	int status, ret = NFS_OK;
+	struct nfs4_lock_state *lsp;
+	struct nfs_server *server = NFS_SERVER(state->inode);
+
+	if (!test_bit(LK_STATE_IN_USE, &state->flags))
+		goto out;
+	list_for_each_entry(lsp, &state->lock_states, ls_locks) {
+		if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) {
+			struct rpc_cred *cred = lsp->ls_state->owner->so_cred;
+
+			status = nfs41_test_and_free_expired_stateid(server,
+					&lsp->ls_stateid,
+					cred);
+			trace_nfs4_test_lock_stateid(state, lsp, status);
+			if (status == -NFS4ERR_EXPIRED ||
+			    status == -NFS4ERR_BAD_STATEID) {
+				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
+				if (!recover_lost_locks)
+					set_bit(NFS_LOCK_LOST, &lsp->ls_flags);
+			} else if (status != NFS_OK) {
+				ret = status;
+				break;
+			}
+		}
+	};
+out:
+	return ret;
+}
+
+/**
  * nfs41_check_open_stateid - possibly free an open stateid
  *
  * @state: NFSv4 state for an inode
@@ -2522,6 +2561,9 @@ static int nfs41_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
 	int status;
 
 	nfs41_check_delegation_stateid(state);
+	status = nfs41_check_expired_locks(state);
+	if (status != NFS_OK)
+		return status;
 	status = nfs41_check_open_stateid(state);
 	if (status != NFS_OK)
 		status = nfs4_open_expired(sp, state);
@@ -6125,49 +6167,19 @@ out:
 }
 
 #if defined(CONFIG_NFS_V4_1)
-/**
- * nfs41_check_expired_locks - possibly free a lock stateid
- *
- * @state: NFSv4 state for an inode
- *
- * Returns NFS_OK if recovery for this stateid is now finished.
- * Otherwise a negative NFS4ERR value is returned.
- */
-static int nfs41_check_expired_locks(struct nfs4_state *state)
-{
-	int status, ret = NFS_OK;
-	struct nfs4_lock_state *lsp;
-	struct nfs_server *server = NFS_SERVER(state->inode);
-
-	list_for_each_entry(lsp, &state->lock_states, ls_locks) {
-		if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) {
-			struct rpc_cred *cred = lsp->ls_state->owner->so_cred;
-
-			status = nfs41_test_and_free_expired_stateid(server,
-					&lsp->ls_stateid,
-					cred);
-			trace_nfs4_test_lock_stateid(state, lsp, status);
-			if (status == -NFS4ERR_EXPIRED ||
-			    status == -NFS4ERR_BAD_STATEID)
-				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
-			else if (status != NFS_OK) {
-				ret = status;
-				break;
-			}
-		}
-	};
-
-	return ret;
-}
-
 static int nfs41_lock_expired(struct nfs4_state *state, struct file_lock *request)
 {
-	int status = NFS_OK;
+	struct nfs4_lock_state *lsp;
+	int status;
 
-	if (test_bit(LK_STATE_IN_USE, &state->flags))
-		status = nfs41_check_expired_locks(state);
-	if (status != NFS_OK)
-		status = nfs4_lock_expired(state, request);
+	status = nfs4_set_lock_state(state, request);
+	if (status != 0)
+		return status;
+	lsp = request->fl_u.nfs4_fl.owner;
+	if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) ||
+	    test_bit(NFS_LOCK_LOST, &lsp->ls_flags))
+		return 0;
+	status = nfs4_lock_expired(state, request);
 	return status;
 }
 #endif
-- 
2.7.4

[PATCH v3 11/12] NFSv4.1: FREE_STATEID can be asynchronous

[Trond Myklebust]

Nothing should need to be serialised with FREE_STATEID on the client,
so let's make the RPC call always asynchronous. Also constify the
stateid argument.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c7e9f91c5cfb..5442e0e1de14 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -99,8 +99,8 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 #ifdef CONFIG_NFS_V4_1
 static int nfs41_test_stateid(struct nfs_server *, nfs4_stateid *,
 		struct rpc_cred *);
-static int nfs41_free_stateid(struct nfs_server *, nfs4_stateid *,
-		struct rpc_cred *);
+static int nfs41_free_stateid(struct nfs_server *, const nfs4_stateid *,
+		struct rpc_cred *, bool);
 #endif
 
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
@@ -2443,7 +2443,7 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
 	}
 out_free:
 	/* Ack the revoked state to the server */
-	nfs41_free_stateid(server, stateid, cred);
+	nfs41_free_stateid(server, stateid, cred, true);
 	return -NFS4ERR_EXPIRED;
 }
 
@@ -8714,7 +8714,7 @@ static const struct rpc_call_ops nfs41_free_stateid_ops = {
 };
 
 static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server,
-		nfs4_stateid *stateid,
+		const nfs4_stateid *stateid,
 		struct rpc_cred *cred,
 		bool privileged)
 {
@@ -8757,38 +8757,31 @@ static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server,
  * @server: server / transport on which to perform the operation
  * @stateid: state ID to release
  * @cred: credential
+ * @is_recovery: set to true if this call needs to be privileged
  *
- * Returns NFS_OK if the server freed "stateid".  Otherwise a
- * negative NFS4ERR value is returned.
+ * Note: this function is always asynchronous.
  */
 static int nfs41_free_stateid(struct nfs_server *server,
-		nfs4_stateid *stateid,
-		struct rpc_cred *cred)
+		const nfs4_stateid *stateid,
+		struct rpc_cred *cred,
+		bool is_recovery)
 {
 	struct rpc_task *task;
-	int ret;
 
-	task = _nfs41_free_stateid(server, stateid, cred, true);
+	task = _nfs41_free_stateid(server, stateid, cred, is_recovery);
 	if (IS_ERR(task))
 		return PTR_ERR(task);
-	ret = rpc_wait_for_completion_task(task);
-	if (!ret)
-		ret = task->tk_status;
 	rpc_put_task(task);
-	return ret;
+	return 0;
 }
 
 static void
 nfs41_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp)
 {
-	struct rpc_task *task;
 	struct rpc_cred *cred = lsp->ls_state->owner->so_cred;
 
-	task = _nfs41_free_stateid(server, &lsp->ls_stateid, cred, false);
+	nfs41_free_stateid(server, &lsp->ls_stateid, cred, false);
 	nfs4_free_lock_state(server, lsp);
-	if (IS_ERR(task))
-		return;
-	rpc_put_task(task);
 }
 
 static bool nfs41_match_stateid(const nfs4_stateid *s1,
-- 
2.7.4

[PATCH v3 12/12] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku

[Trond Myklebust]

If a server returns NFS4ERR_ADMIN_REVOKED, NFS4ERR_DELEG_REVOKED
or NFS4ERR_EXPIRED on a call to close, open_downgrade, delegreturn, or
locku, we should call FREE_STATEID before attempting to recover.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5442e0e1de14..ffb3218a4a00 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -328,6 +328,18 @@ static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dent
 	kunmap_atomic(start);
 }
 
+static void nfs4_free_revoked_stateid(struct nfs_server *server,
+		const nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	const struct nfs4_minor_version_ops *ops = server->nfs_client->cl_mvops;
+	nfs4_stateid tmp;
+
+	nfs4_stateid_copy(&tmp, stateid);
+	tmp.type = NFS4_REVOKED_STATEID_TYPE;
+	ops->test_and_free_expired(server, &tmp, cred);
+}
+
 static long nfs4_update_delay(long *timeout)
 {
 	long ret;
@@ -2983,9 +2995,12 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 			break;
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_STALE_STATEID:
+		case -NFS4ERR_EXPIRED:
+			nfs4_free_revoked_stateid(server,
+					&calldata->arg.stateid,
+					task->tk_msg.rpc_cred);
 		case -NFS4ERR_OLD_STATEID:
 		case -NFS4ERR_BAD_STATEID:
-		case -NFS4ERR_EXPIRED:
 			if (!nfs4_stateid_match(&calldata->arg.stateid,
 						&state->open_stateid)) {
 				rpc_restart_call_prepare(task);
@@ -5477,10 +5492,13 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 		renew_lease(data->res.server, data->timestamp);
 	case -NFS4ERR_ADMIN_REVOKED:
 	case -NFS4ERR_DELEG_REVOKED:
+	case -NFS4ERR_EXPIRED:
+		nfs4_free_revoked_stateid(data->res.server,
+				data->args.stateid,
+				task->tk_msg.rpc_cred);
 	case -NFS4ERR_BAD_STATEID:
 	case -NFS4ERR_OLD_STATEID:
 	case -NFS4ERR_STALE_STATEID:
-	case -NFS4ERR_EXPIRED:
 		task->tk_status = 0;
 		if (data->roc)
 			pnfs_roc_set_barrier(data->inode, data->roc_barrier);
@@ -5745,10 +5763,14 @@ static void nfs4_locku_done(struct rpc_task *task, void *data)
 			if (nfs4_update_lock_stateid(calldata->lsp,
 					&calldata->res.stateid))
 				break;
+		case -NFS4ERR_ADMIN_REVOKED:
+		case -NFS4ERR_EXPIRED:
+			nfs4_free_revoked_stateid(calldata->server,
+					&calldata->arg.stateid,
+					task->tk_msg.rpc_cred);
 		case -NFS4ERR_BAD_STATEID:
 		case -NFS4ERR_OLD_STATEID:
 		case -NFS4ERR_STALE_STATEID:
-		case -NFS4ERR_EXPIRED:
 			if (!nfs4_stateid_match(&calldata->arg.stateid,
 						&calldata->lsp->ls_stateid))
 				rpc_restart_call_prepare(task);
-- 
2.7.4

Re: [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags

[Olga Kornievskaia]

On Fri, Sep 9, 2016 at 2:04 PM, Trond Myklebust
<trond.myklebust@primarydata.com> wrote:
> As described in RFC5661, section 18.46, some of the status flags exist
> in order to tell the client when it needs to acknowledge the existence of
> revoked state on the server and/or to recover state.
> Those flags will then remain set until the recovery procedure is done.
>
> In order to avoid looping, the client therefore needs to ignore
> those particular flags while recovering.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  fs/nfs/nfs4_fs.h     |  2 +-
>  fs/nfs/nfs4proc.c    |  5 ++++-
>  fs/nfs/nfs4session.h |  1 +
>  fs/nfs/nfs4state.c   | 12 +++++++++++-
>  4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index f230aa62ca59..4390d73a92e5 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -439,7 +439,7 @@ extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
>  extern int nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *);
>  extern int nfs4_schedule_migration_recovery(const struct nfs_server *);
>  extern void nfs4_schedule_lease_moved_recovery(struct nfs_client *);
> -extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags);
> +extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags, bool);
>  extern void nfs41_handle_server_scope(struct nfs_client *,
>                                       struct nfs41_server_scope **);
>  extern void nfs4_put_lock_state(struct nfs4_lock_state *lsp);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 25a2af707233..bd33777f03c4 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -616,6 +616,7 @@ int nfs40_setup_sequence(struct nfs4_slot_table *tbl,
>         }
>         spin_unlock(&tbl->slot_tbl_lock);
>
> +       slot->privileged = args->sa_privileged ? 1 : 0;
>         args->sa_slot = slot;
>         res->sr_slot = slot;
>
> @@ -728,7 +729,8 @@ static int nfs41_sequence_process(struct rpc_task *task,
>                 clp = session->clp;
>                 do_renew_lease(clp, res->sr_timestamp);
>                 /* Check sequence flags */
> -               nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags);
> +               nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags,
> +                               !!slot->privileged);

Is this suppose to be "!!"?

>                 nfs41_update_target_slotid(slot->table, slot, res);
>                 break;
>         case 1:
> @@ -875,6 +877,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
>         }
>         spin_unlock(&tbl->slot_tbl_lock);
>
> +       slot->privileged = args->sa_privileged ? 1 : 0;
>         args->sa_slot = slot;
>
>         dprintk("<-- %s slotid=%u seqid=%u\n", __func__,
> diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
> index 3bb6af70973c..dae385500005 100644
> --- a/fs/nfs/nfs4session.h
> +++ b/fs/nfs/nfs4session.h
> @@ -23,6 +23,7 @@ struct nfs4_slot {
>         u32                     slot_nr;
>         u32                     seq_nr;
>         unsigned int            interrupted : 1,
> +                               privileged : 1,
>                                 seq_done : 1;
>  };
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index cada00aa5096..9801b5bb5fac 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -2227,13 +2227,22 @@ static void nfs41_handle_cb_path_down(struct nfs_client *clp)
>                 nfs4_schedule_state_manager(clp);
>  }
>
> -void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
> +void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags,
> +               bool recovery)
>  {
>         if (!flags)
>                 return;
>
>         dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
>                 __func__, clp->cl_hostname, clp->cl_clientid, flags);
> +       /*
> +        * If we're called from the state manager thread, then assume we're
> +        * already handling the RECLAIM_NEEDED and/or STATE_REVOKED.
> +        * Those flags are expected to remain set until we're done
> +        * recovering (see RFC5661, section 18.46.3).
> +        */
> +       if (recovery)
> +               goto out_recovery;
>
>         if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
>                 nfs41_handle_server_reboot(clp);
> @@ -2246,6 +2255,7 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
>                 nfs4_schedule_lease_moved_recovery(clp);
>         if (flags & SEQ4_STATUS_RECALLABLE_STATE_REVOKED)
>                 nfs41_handle_recallable_state_revoked(clp);
> +out_recovery:
>         if (flags & SEQ4_STATUS_BACKCHANNEL_FAULT)
>                 nfs41_handle_backchannel_fault(clp);
>         else if (flags & (SEQ4_STATUS_CB_PATH_DOWN |
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags

[Trond Myklebust]

DQo+IE9uIFNlcCA5LCAyMDE2LCBhdCAxNDoyNiwgT2xnYSBLb3JuaWV2c2thaWEgPGFnbG9AdW1p
Y2guZWR1PiB3cm90ZToNCj4gDQo+IE9uIEZyaSwgU2VwIDksIDIwMTYgYXQgMjowNCBQTSwgVHJv
bmQgTXlrbGVidXN0DQo+IDx0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tPiB3cm90ZToN
Cj4+IEFzIGRlc2NyaWJlZCBpbiBSRkM1NjYxLCBzZWN0aW9uIDE4LjQ2LCBzb21lIG9mIHRoZSBz
dGF0dXMgZmxhZ3MgZXhpc3QNCj4+IGluIG9yZGVyIHRvIHRlbGwgdGhlIGNsaWVudCB3aGVuIGl0
IG5lZWRzIHRvIGFja25vd2xlZGdlIHRoZSBleGlzdGVuY2Ugb2YNCj4+IHJldm9rZWQgc3RhdGUg
b24gdGhlIHNlcnZlciBhbmQvb3IgdG8gcmVjb3ZlciBzdGF0ZS4NCj4+IFRob3NlIGZsYWdzIHdp
bGwgdGhlbiByZW1haW4gc2V0IHVudGlsIHRoZSByZWNvdmVyeSBwcm9jZWR1cmUgaXMgZG9uZS4N
Cj4+IA0KPj4gSW4gb3JkZXIgdG8gYXZvaWQgbG9vcGluZywgdGhlIGNsaWVudCB0aGVyZWZvcmUg
bmVlZHMgdG8gaWdub3JlDQo+PiB0aG9zZSBwYXJ0aWN1bGFyIGZsYWdzIHdoaWxlIHJlY292ZXJp
bmcuDQo+PiANCj4+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVi
dXN0QHByaW1hcnlkYXRhLmNvbT4NCj4+IC0tLQ0KPj4gZnMvbmZzL25mczRfZnMuaCAgICAgfCAg
MiArLQ0KPj4gZnMvbmZzL25mczRwcm9jLmMgICAgfCAgNSArKysrLQ0KPj4gZnMvbmZzL25mczRz
ZXNzaW9uLmggfCAgMSArDQo+PiBmcy9uZnMvbmZzNHN0YXRlLmMgICB8IDEyICsrKysrKysrKysr
LQ0KPj4gNCBmaWxlcyBjaGFuZ2VkLCAxNyBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQ0K
Pj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRfZnMuaCBiL2ZzL25mcy9uZnM0X2ZzLmgN
Cj4+IGluZGV4IGYyMzBhYTYyY2E1OS4uNDM5MGQ3M2E5MmU1IDEwMDY0NA0KPj4gLS0tIGEvZnMv
bmZzL25mczRfZnMuaA0KPj4gKysrIGIvZnMvbmZzL25mczRfZnMuaA0KPj4gQEAgLTQzOSw3ICs0
MzksNyBAQCBleHRlcm4gdm9pZCBuZnM0X3NjaGVkdWxlX3BhdGhfZG93bl9yZWNvdmVyeShzdHJ1
Y3QgbmZzX2NsaWVudCAqY2xwKTsNCj4+IGV4dGVybiBpbnQgbmZzNF9zY2hlZHVsZV9zdGF0ZWlk
X3JlY292ZXJ5KGNvbnN0IHN0cnVjdCBuZnNfc2VydmVyICosIHN0cnVjdCBuZnM0X3N0YXRlICop
Ow0KPj4gZXh0ZXJuIGludCBuZnM0X3NjaGVkdWxlX21pZ3JhdGlvbl9yZWNvdmVyeShjb25zdCBz
dHJ1Y3QgbmZzX3NlcnZlciAqKTsNCj4+IGV4dGVybiB2b2lkIG5mczRfc2NoZWR1bGVfbGVhc2Vf
bW92ZWRfcmVjb3Zlcnkoc3RydWN0IG5mc19jbGllbnQgKik7DQo+PiAtZXh0ZXJuIHZvaWQgbmZz
NDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHUz
MiBmbGFncyk7DQo+PiArZXh0ZXJuIHZvaWQgbmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJy
b3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHUzMiBmbGFncywgYm9vbCk7DQo+PiBleHRlcm4g
dm9pZCBuZnM0MV9oYW5kbGVfc2VydmVyX3Njb3BlKHN0cnVjdCBuZnNfY2xpZW50ICosDQo+PiAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IG5mczQxX3NlcnZlcl9z
Y29wZSAqKik7DQo+PiBleHRlcm4gdm9pZCBuZnM0X3B1dF9sb2NrX3N0YXRlKHN0cnVjdCBuZnM0
X2xvY2tfc3RhdGUgKmxzcCk7DQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRwcm9jLmMgYi9m
cy9uZnMvbmZzNHByb2MuYw0KPj4gaW5kZXggMjVhMmFmNzA3MjMzLi5iZDMzNzc3ZjAzYzQgMTAw
NjQ0DQo+PiAtLS0gYS9mcy9uZnMvbmZzNHByb2MuYw0KPj4gKysrIGIvZnMvbmZzL25mczRwcm9j
LmMNCj4+IEBAIC02MTYsNiArNjE2LDcgQEAgaW50IG5mczQwX3NldHVwX3NlcXVlbmNlKHN0cnVj
dCBuZnM0X3Nsb3RfdGFibGUgKnRibCwNCj4+ICAgICAgICB9DQo+PiAgICAgICAgc3Bpbl91bmxv
Y2soJnRibC0+c2xvdF90YmxfbG9jayk7DQo+PiANCj4+ICsgICAgICAgc2xvdC0+cHJpdmlsZWdl
ZCA9IGFyZ3MtPnNhX3ByaXZpbGVnZWQgPyAxIDogMDsNCj4+ICAgICAgICBhcmdzLT5zYV9zbG90
ID0gc2xvdDsNCj4+ICAgICAgICByZXMtPnNyX3Nsb3QgPSBzbG90Ow0KPj4gDQo+PiBAQCAtNzI4
LDcgKzcyOSw4IEBAIHN0YXRpYyBpbnQgbmZzNDFfc2VxdWVuY2VfcHJvY2VzcyhzdHJ1Y3QgcnBj
X3Rhc2sgKnRhc2ssDQo+PiAgICAgICAgICAgICAgICBjbHAgPSBzZXNzaW9uLT5jbHA7DQo+PiAg
ICAgICAgICAgICAgICBkb19yZW5ld19sZWFzZShjbHAsIHJlcy0+c3JfdGltZXN0YW1wKTsNCj4+
ICAgICAgICAgICAgICAgIC8qIENoZWNrIHNlcXVlbmNlIGZsYWdzICovDQo+PiAtICAgICAgICAg
ICAgICAgbmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKGNscCwgcmVzLT5zcl9zdGF0
dXNfZmxhZ3MpOw0KPj4gKyAgICAgICAgICAgICAgIG5mczQxX2hhbmRsZV9zZXF1ZW5jZV9mbGFn
X2Vycm9ycyhjbHAsIHJlcy0+c3Jfc3RhdHVzX2ZsYWdzLA0KPj4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAhIXNsb3QtPnByaXZpbGVnZWQpOw0KPiANCj4gSXMgdGhpcyBzdXBwb3Nl
IHRvIGJlICIhIeKAnT8NCg0KWWVzLiBJdCBjb252ZXJ0cyB0byBhIGJvb2xlYW4uDQoNCj4gDQo+
PiAgICAgICAgICAgICAgICBuZnM0MV91cGRhdGVfdGFyZ2V0X3Nsb3RpZChzbG90LT50YWJsZSwg
c2xvdCwgcmVzKTsNCj4+ICAgICAgICAgICAgICAgIGJyZWFrOw0KPj4gICAgICAgIGNhc2UgMToN
Cj4+IEBAIC04NzUsNiArODc3LDcgQEAgaW50IG5mczQxX3NldHVwX3NlcXVlbmNlKHN0cnVjdCBu
ZnM0X3Nlc3Npb24gKnNlc3Npb24sDQo+PiAgICAgICAgfQ0KPj4gICAgICAgIHNwaW5fdW5sb2Nr
KCZ0YmwtPnNsb3RfdGJsX2xvY2spOw0KPj4gDQo+PiArICAgICAgIHNsb3QtPnByaXZpbGVnZWQg
PSBhcmdzLT5zYV9wcml2aWxlZ2VkID8gMSA6IDA7DQo+PiAgICAgICAgYXJncy0+c2Ffc2xvdCA9
IHNsb3Q7DQo+PiANCj4+ICAgICAgICBkcHJpbnRrKCI8LS0gJXMgc2xvdGlkPSV1IHNlcWlkPSV1
XG4iLCBfX2Z1bmNfXywNCj4+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNHNlc3Npb24uaCBiL2Zz
L25mcy9uZnM0c2Vzc2lvbi5oDQo+PiBpbmRleCAzYmI2YWY3MDk3M2MuLmRhZTM4NTUwMDAwNSAx
MDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9uZnM0c2Vzc2lvbi5oDQo+PiArKysgYi9mcy9uZnMvbmZz
NHNlc3Npb24uaA0KPj4gQEAgLTIzLDYgKzIzLDcgQEAgc3RydWN0IG5mczRfc2xvdCB7DQo+PiAg
ICAgICAgdTMyICAgICAgICAgICAgICAgICAgICAgc2xvdF9ucjsNCj4+ICAgICAgICB1MzIgICAg
ICAgICAgICAgICAgICAgICBzZXFfbnI7DQo+PiAgICAgICAgdW5zaWduZWQgaW50ICAgICAgICAg
ICAgaW50ZXJydXB0ZWQgOiAxLA0KPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBw
cml2aWxlZ2VkIDogMSwNCj4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzZXFfZG9u
ZSA6IDE7DQo+PiB9Ow0KPj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRzdGF0ZS5jIGIv
ZnMvbmZzL25mczRzdGF0ZS5jDQo+PiBpbmRleCBjYWRhMDBhYTUwOTYuLjk4MDFiNWJiNWZhYyAx
MDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9uZnM0c3RhdGUuYw0KPj4gKysrIGIvZnMvbmZzL25mczRz
dGF0ZS5jDQo+PiBAQCAtMjIyNywxMyArMjIyNywyMiBAQCBzdGF0aWMgdm9pZCBuZnM0MV9oYW5k
bGVfY2JfcGF0aF9kb3duKHN0cnVjdCBuZnNfY2xpZW50ICpjbHApDQo+PiAgICAgICAgICAgICAg
ICBuZnM0X3NjaGVkdWxlX3N0YXRlX21hbmFnZXIoY2xwKTsNCj4+IH0NCj4+IA0KPj4gLXZvaWQg
bmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAs
IHUzMiBmbGFncykNCj4+ICt2b2lkIG5mczQxX2hhbmRsZV9zZXF1ZW5jZV9mbGFnX2Vycm9ycyhz
dHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCB1MzIgZmxhZ3MsDQo+PiArICAgICAgICAgICAgICAgYm9v
bCByZWNvdmVyeSkNCj4+IHsNCj4+ICAgICAgICBpZiAoIWZsYWdzKQ0KPj4gICAgICAgICAgICAg
ICAgcmV0dXJuOw0KPj4gDQo+PiAgICAgICAgZHByaW50aygiJXM6IFwiJXNcIiAoY2xpZW50IElE
ICVsbHgpIGZsYWdzPTB4JTA4eFxuIiwNCj4+ICAgICAgICAgICAgICAgIF9fZnVuY19fLCBjbHAt
PmNsX2hvc3RuYW1lLCBjbHAtPmNsX2NsaWVudGlkLCBmbGFncyk7DQo+PiArICAgICAgIC8qDQo+
PiArICAgICAgICAqIElmIHdlJ3JlIGNhbGxlZCBmcm9tIHRoZSBzdGF0ZSBtYW5hZ2VyIHRocmVh
ZCwgdGhlbiBhc3N1bWUgd2UncmUNCj4+ICsgICAgICAgICogYWxyZWFkeSBoYW5kbGluZyB0aGUg
UkVDTEFJTV9ORUVERUQgYW5kL29yIFNUQVRFX1JFVk9LRUQuDQo+PiArICAgICAgICAqIFRob3Nl
IGZsYWdzIGFyZSBleHBlY3RlZCB0byByZW1haW4gc2V0IHVudGlsIHdlJ3JlIGRvbmUNCj4+ICsg
ICAgICAgICogcmVjb3ZlcmluZyAoc2VlIFJGQzU2NjEsIHNlY3Rpb24gMTguNDYuMykuDQo+PiAr
ICAgICAgICAqLw0KPj4gKyAgICAgICBpZiAocmVjb3ZlcnkpDQo+PiArICAgICAgICAgICAgICAg
Z290byBvdXRfcmVjb3Zlcnk7DQo+PiANCj4+ICAgICAgICBpZiAoZmxhZ3MgJiBTRVE0X1NUQVRV
U19SRVNUQVJUX1JFQ0xBSU1fTkVFREVEKQ0KPj4gICAgICAgICAgICAgICAgbmZzNDFfaGFuZGxl
X3NlcnZlcl9yZWJvb3QoY2xwKTsNCj4+IEBAIC0yMjQ2LDYgKzIyNTUsNyBAQCB2b2lkIG5mczQx
X2hhbmRsZV9zZXF1ZW5jZV9mbGFnX2Vycm9ycyhzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCB1MzIg
ZmxhZ3MpDQo+PiAgICAgICAgICAgICAgICBuZnM0X3NjaGVkdWxlX2xlYXNlX21vdmVkX3JlY292
ZXJ5KGNscCk7DQo+PiAgICAgICAgaWYgKGZsYWdzICYgU0VRNF9TVEFUVVNfUkVDQUxMQUJMRV9T
VEFURV9SRVZPS0VEKQ0KPj4gICAgICAgICAgICAgICAgbmZzNDFfaGFuZGxlX3JlY2FsbGFibGVf
c3RhdGVfcmV2b2tlZChjbHApOw0KPj4gK291dF9yZWNvdmVyeToNCj4+ICAgICAgICBpZiAoZmxh
Z3MgJiBTRVE0X1NUQVRVU19CQUNLQ0hBTk5FTF9GQVVMVCkNCj4+ICAgICAgICAgICAgICAgIG5m
czQxX2hhbmRsZV9iYWNrY2hhbm5lbF9mYXVsdChjbHApOw0KPj4gICAgICAgIGVsc2UgaWYgKGZs
YWdzICYgKFNFUTRfU1RBVFVTX0NCX1BBVEhfRE9XTiB8DQo+PiAtLQ0KPj4gMi43LjQNCj4+IA0K
Pj4gLS0NCj4+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1
bnN1YnNjcmliZSBsaW51eC1uZnMiIGluDQo+PiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFq
b3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDov
L3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoNCg==

Re: [PATCH v3 00/12] Fix delegation behaviour when server revokes some state

[Oleg Drokin]

On Sep 9, 2016, at 2:04 PM, Trond Myklebust wrote:

> According to RFC5661, if any of the SEQUENCE status bits
> SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
> SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED, SEQ4_STATUS_ADMIN_STATE_REVOKED,
> or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to use
> TEST_STATEID to figure out which stateids have been revoked, so we
> can acknowledge the loss of state using FREE_STATEID.
> 
> While we already do this for open and lock state, we have not been doing
> so for all the delegations.
> 
> v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
> v3: Now with added lock revoke fixes and close/delegreturn/locku fixes

With this set there are changes in behavior of my reproducer.

Change #1, server leaks dentries now, so once it's stopped, I cannot
unmount the ext4 that was exported.
I see your changes are just to the client, so I guess there's another nfsd bug
that needs to be tracked here.

Change #2, instead of client wedged in the old way, it's now wedges in another,
but similar way:
nfs client mountpoint is busy, but instead of server-ip-named thread spinning,
it's one of actual userspace threads that's now wedged.
-1 debug logs shows:
[21122.814992] NFS call  test_stateid ffff880069fb1f24
[21122.814997] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000
[21122.814997] --> nfs41_setup_sequence
[21122.814999] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21122.815000] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21122.815001] <-- nfs41_setup_sequence slotid=0 seqid=3502621
[21122.815010] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502621 slotid=0 max_slotid=0 cache_this=0
[21122.815191] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21122.815192] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21122.815193] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21122.815194] nfs41_sequence_process: Error 0 free the slot 
[21122.815195] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21122.815226] NFS reply test_stateid: succeeded, 0
[21122.815235] --> nfs_put_client({3})
[21122.815253] --> nfs_put_client({2})
[21122.815260] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000
[21122.815260] --> nfs41_setup_sequence
[21122.815262] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21122.815263] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21122.815264] <-- nfs41_setup_sequence slotid=0 seqid=3502622
[21122.815273] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502622 slotid=0 max_slotid=0 cache_this=1
[21122.815437] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21122.815438] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21122.815439] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21122.815440] nfs41_sequence_process: Error 0 free the slot 
[21122.815442] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21122.815462] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
[21122.815578] NFS call  test_stateid ffff880069fb1f24
[21122.815583] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000
[21122.815583] --> nfs41_setup_sequence
[21122.815585] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21122.815586] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21122.815587] <-- nfs41_setup_sequence slotid=0 seqid=3502623
[21122.815596] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502623 slotid=0 max_slotid=0 cache_this=0
[21122.819581] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21122.819583] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21122.819585] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21122.819586] nfs41_sequence_process: Error 0 free the slot 
[21122.819588] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21122.819613] NFS reply test_stateid: succeeded, 0
[21122.819629] --> nfs_put_client({3})
[21122.819658] --> nfs_put_client({2})
[21122.819669] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000
[21122.819669] --> nfs41_setup_sequence
[21122.819671] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21122.819672] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21122.819673] <-- nfs41_setup_sequence slotid=0 seqid=3502624
[21122.819688] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502624 slotid=0 max_slotid=0 cache_this=1
[21122.819924] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21122.819926] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21122.819927] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21122.819928] nfs41_sequence_process: Error 0 free the slot 
[21122.819929] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21122.819952] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
[21122.823396] NFS call  test_stateid ffff880069fb1f24

This is happening not as frequently as a previous problem, or possibly it's
just masked by the dentry leakage server-side that I saw more frequently than this one.

> Trond Myklebust (12):
>  NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
>  NFSv4.1: Don't check delegations that are already marked as revoked
>  NFSv4.1: Allow test_stateid to handle session errors without waiting
>  NFSv4.1: Add a helper function to deal with expired stateids
>  NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a
>    stateid
>  NFSv4.1: Test delegation stateids when server declares "some state
>    revoked"
>  NFSv4.1: Deal with server reboots during delegation expiration
>    recovery
>  NFSv4.1: Don't recheck delegations that have already been checked
>  NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
>  NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
>  NFSv4.1: FREE_STATEID can be asynchronous
>  NFSv4.1: Ensure we call FREE_STATEID if needed on
>    close/delegreturn/locku
> 
> fs/nfs/delegation.c                    | 153 ++++++++++++++++++-
> fs/nfs/delegation.h                    |   6 +-
> fs/nfs/flexfilelayout/flexfilelayout.c |   2 +-
> fs/nfs/nfs4_fs.h                       |   5 +-
> fs/nfs/nfs4proc.c                      | 258 ++++++++++++++++++++++-----------
> fs/nfs/nfs4session.h                   |   1 +
> fs/nfs/nfs4state.c                     |  29 ++--
> include/linux/nfs4.h                   |   1 +
> 8 files changed, 348 insertions(+), 107 deletions(-)
> 
> -- 
> 2.7.4