All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
@ 2016-09-15 16:45 Trond Myklebust
  2016-09-15 16:45 ` [PATCH v4 01/20] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
  2016-09-16  4:38 ` [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Oleg Drokin
  0 siblings, 2 replies; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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
v4: Close a bunch of corner cases

Trond Myklebust (20):
  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
  NFSv4: nfs_inode_find_delegation_state_and_recover() should check all
    stateids
  NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single
    stateid
  NFSv4: nfs4_handle_delegation_recall_error() handle expiration as
    revoke case
  NFSv4: nfs4_handle_setlk_error() handle expiration as revoke case
  NFSv4.1: nfs4_layoutget_handle_exception handle revoked state
  NFSv4: Pass the stateid to the exception handler in
    nfs4_read/write_done_cb
  NFSv4: Fix a race in nfs_inode_reclaim_delegation()
  NFSv4: Fix a race when updating an open_stateid

 fs/nfs/delegation.c                    | 183 +++++++++++++++--
 fs/nfs/delegation.h                    |   8 +-
 fs/nfs/flexfilelayout/flexfilelayout.c |   2 +-
 fs/nfs/nfs4_fs.h                       |   5 +-
 fs/nfs/nfs4proc.c                      | 364 ++++++++++++++++++++++-----------
 fs/nfs/nfs4session.h                   |   1 +
 fs/nfs/nfs4state.c                     |  73 +++++--
 include/linux/nfs4.h                   |   1 +
 8 files changed, 490 insertions(+), 147 deletions(-)

-- 
2.7.4


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

* [PATCH v4 01/20] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
  2016-09-15 16:45 [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Trond Myklebust
@ 2016-09-15 16:45 ` Trond Myklebust
  2016-09-15 16:45   ` [PATCH v4 02/20] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
  2016-09-16  4:38 ` [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Oleg Drokin
  1 sibling, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 251e48e7ba16..6b700c59eede 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


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

* [PATCH v4 02/20] NFSv4.1: Don't check delegations that are already marked as revoked
  2016-09-15 16:45 ` [PATCH v4 01/20] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
@ 2016-09-15 16:45   ` Trond Myklebust
  2016-09-15 16:45     ` [PATCH v4 03/20] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 6b700c59eede..a026156434ae 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


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

* [PATCH v4 03/20] NFSv4.1: Allow test_stateid to handle session errors without waiting
  2016-09-15 16:45   ` [PATCH v4 02/20] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
@ 2016-09-15 16:45     ` Trond Myklebust
  2016-09-15 16:45       ` [PATCH v4 04/20] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 a026156434ae..2b569d5fb3e2 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8598,6 +8598,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
  *
@@ -8617,9 +8634,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


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

* [PATCH v4 04/20] NFSv4.1: Add a helper function to deal with expired stateids
  2016-09-15 16:45     ` [PATCH v4 03/20] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
@ 2016-09-15 16:45       ` Trond Myklebust
  2016-09-15 16:45         ` [PATCH v4 05/20] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 2b569d5fb3e2..0ade81441ac2 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


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

* [PATCH v4 05/20] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid
  2016-09-15 16:45       ` [PATCH v4 04/20] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
@ 2016-09-15 16:45         ` Trond Myklebust
  2016-09-15 16:45           ` [PATCH v4 06/20] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 0ade81441ac2..9c203c221876 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


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

* [PATCH v4 06/20] NFSv4.1: Test delegation stateids when server declares "some state revoked"
  2016-09-15 16:45         ` [PATCH v4 05/20] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
@ 2016-09-15 16:45           ` Trond Myklebust
  2016-09-15 16:45             ` [PATCH v4 07/20] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 9c203c221876..3552f3a4ceb0 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,
@@ -8862,6 +8869,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,
@@ -8889,6 +8897,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,
@@ -8918,6 +8927,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


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

* [PATCH v4 07/20] NFSv4.1: Deal with server reboots during delegation expiration recovery
  2016-09-15 16:45           ` [PATCH v4 06/20] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
@ 2016-09-15 16:45             ` Trond Myklebust
  2016-09-15 16:45               ` [PATCH v4 08/20] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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


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

* [PATCH v4 08/20] NFSv4.1: Don't recheck delegations that have already been checked
  2016-09-15 16:45             ` [PATCH v4 07/20] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
@ 2016-09-15 16:45               ` Trond Myklebust
  2016-09-15 16:45                 ` [PATCH v4 09/20] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 3552f3a4ceb0..97a614370604 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


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

* [PATCH v4 09/20] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
  2016-09-15 16:45               ` [PATCH v4 08/20] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
@ 2016-09-15 16:45                 ` Trond Myklebust
  2016-09-15 16:45                   ` [PATCH v4 10/20] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 97a614370604..3c1b8cb7dd95 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


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

* [PATCH v4 10/20] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
  2016-09-15 16:45                 ` [PATCH v4 09/20] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
@ 2016-09-15 16:45                   ` Trond Myklebust
  2016-09-15 16:45                     ` [PATCH v4 11/20] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 3c1b8cb7dd95..33ca6d768bd2 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


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

* [PATCH v4 11/20] NFSv4.1: FREE_STATEID can be asynchronous
  2016-09-15 16:45                   ` [PATCH v4 10/20] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
@ 2016-09-15 16:45                     ` Trond Myklebust
  2016-09-15 16:45                       ` [PATCH v4 12/20] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 33ca6d768bd2..a53caeb73f75 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;
 }
 
@@ -8722,7 +8722,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)
 {
@@ -8765,38 +8765,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


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

* [PATCH v4 12/20] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku
  2016-09-15 16:45                     ` [PATCH v4 11/20] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
@ 2016-09-15 16:45                       ` Trond Myklebust
  2016-09-15 16:45                         ` [PATCH v4 13/20] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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 | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a53caeb73f75..f79d4f55d83f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -328,6 +328,33 @@ static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dent
 	kunmap_atomic(start);
 }
 
+static void nfs4_test_and_free_stateid(struct nfs_server *server,
+		nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	const struct nfs4_minor_version_ops *ops = server->nfs_client->cl_mvops;
+
+	ops->test_and_free_expired(server, stateid, cred);
+}
+
+static void __nfs4_free_revoked_stateid(struct nfs_server *server,
+		nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	stateid->type = NFS4_REVOKED_STATEID_TYPE;
+	nfs4_test_and_free_stateid(server, stateid, cred);
+}
+
+static void nfs4_free_revoked_stateid(struct nfs_server *server,
+		const nfs4_stateid *stateid,
+		struct rpc_cred *cred)
+{
+	nfs4_stateid tmp;
+
+	nfs4_stateid_copy(&tmp, stateid);
+	__nfs4_free_revoked_stateid(server, &tmp, cred);
+}
+
 static long nfs4_update_delay(long *timeout)
 {
 	long ret;
@@ -2983,9 +3010,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 +5507,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 +5778,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


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

* [PATCH v4 13/20] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids
  2016-09-15 16:45                       ` [PATCH v4 12/20] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
@ 2016-09-15 16:45                         ` Trond Myklebust
  2016-09-15 16:45                           ` [PATCH v4 14/20] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

Modify the helper nfs_inode_find_delegation_state_and_recover() so that it
can check all stateids for whether or not they need recovery.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/delegation.c | 18 ++++++++++++++++++
 fs/nfs/delegation.h |  2 ++
 fs/nfs/nfs4state.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index b873271c613a..efd50159ab4a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -988,6 +988,24 @@ restart:
 	rcu_read_unlock();
 }
 
+void nfs_inode_find_delegation_state_and_recover(struct inode *inode,
+		const nfs4_stateid *stateid)
+{
+	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+	struct nfs_delegation *delegation;
+	bool found = false;
+
+	rcu_read_lock();
+	delegation = rcu_dereference(NFS_I(inode)->delegation);
+	if (delegation && nfs4_stateid_match(&delegation->stateid, stateid)) {
+		nfs_mark_test_expired_delegation(NFS_SERVER(inode), delegation);
+		found = true;
+	}
+	rcu_read_unlock();
+	if (found)
+		nfs4_schedule_state_manager(clp);
+}
+
 /**
  * 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 1442e3b1521d..e9d555796873 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -66,6 +66,8 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
 int nfs4_have_delegation(struct inode *inode, fmode_t flags);
 int nfs4_check_delegation(struct inode *inode, fmode_t flags);
 bool nfs4_delegation_flush_on_close(const struct inode *inode);
+void nfs_inode_find_delegation_state_and_recover(struct inode *inode,
+		const nfs4_stateid *stateid);
 
 #endif
 
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 63da0411e2af..609643f31e7a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1337,6 +1337,35 @@ int nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4_
 }
 EXPORT_SYMBOL_GPL(nfs4_schedule_stateid_recovery);
 
+static struct nfs4_lock_state *
+nfs_state_find_lock_state_by_stateid(struct nfs4_state *state,
+		const nfs4_stateid *stateid)
+{
+	struct nfs4_lock_state *pos;
+
+	list_for_each_entry(pos, &state->lock_states, ls_locks) {
+		if (!test_bit(NFS_LOCK_INITIALIZED, &pos->ls_flags))
+			continue;
+		if (nfs4_stateid_match(&pos->ls_stateid, stateid))
+			return pos;
+	}
+	return NULL;
+}
+
+static bool nfs_state_lock_state_matches_stateid(struct nfs4_state *state,
+		const nfs4_stateid *stateid)
+{
+	bool found = false;
+
+	if (test_bit(LK_STATE_IN_USE, &state->flags)) {
+		spin_lock(&state->state_lock);
+		if (nfs_state_find_lock_state_by_stateid(state, stateid))
+			found = true;
+		spin_unlock(&state->state_lock);
+	}
+	return found;
+}
+
 void nfs_inode_find_state_and_recover(struct inode *inode,
 		const nfs4_stateid *stateid)
 {
@@ -1351,14 +1380,19 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
 		state = ctx->state;
 		if (state == NULL)
 			continue;
-		if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
-			continue;
-		if (!nfs4_stateid_match(&state->stateid, stateid))
+		if (nfs4_stateid_match(&state->stateid, stateid)) {
+			nfs4_state_mark_reclaim_nograce(clp, state);
+			found = true;
 			continue;
-		nfs4_state_mark_reclaim_nograce(clp, state);
-		found = true;
+		}
+		if (nfs_state_lock_state_matches_stateid(state, stateid)) {
+			nfs4_state_mark_reclaim_nograce(clp, state);
+			found = true;
+		}
 	}
 	spin_unlock(&inode->i_lock);
+
+	nfs_inode_find_delegation_state_and_recover(inode, stateid);
 	if (found)
 		nfs4_schedule_state_manager(clp);
 }
-- 
2.7.4


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

* [PATCH v4 14/20] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid
  2016-09-15 16:45                         ` [PATCH v4 13/20] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
@ 2016-09-15 16:45                           ` Trond Myklebust
  2016-09-15 16:45                             ` [PATCH v4 15/20] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If we're not yet sure that all state has expired or been revoked, we
should try to do a minimal recovery on just the one stateid.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f79d4f55d83f..84ee836a976c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -400,10 +400,16 @@ static int nfs4_do_handle_exception(struct nfs_server *server,
 	switch(errorcode) {
 		case 0:
 			return 0;
-		case -NFS4ERR_OPENMODE:
 		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_ADMIN_REVOKED:
+		case -NFS4ERR_EXPIRED:
 		case -NFS4ERR_BAD_STATEID:
+			if (inode != NULL && stateid != NULL) {
+				nfs_inode_find_delegation_state_and_recover(inode,
+						stateid);
+				goto wait_on_recovery;
+			}
+		case -NFS4ERR_OPENMODE:
 			if (inode) {
 				int err;
 
@@ -422,12 +428,6 @@ static int nfs4_do_handle_exception(struct nfs_server *server,
 			if (ret < 0)
 				break;
 			goto wait_on_recovery;
-		case -NFS4ERR_EXPIRED:
-			if (state != NULL) {
-				ret = nfs4_schedule_stateid_recovery(server, state);
-				if (ret < 0)
-					break;
-			}
 		case -NFS4ERR_STALE_STATEID:
 		case -NFS4ERR_STALE_CLIENTID:
 			nfs4_schedule_lease_recovery(clp);
-- 
2.7.4


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

* [PATCH v4 15/20] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case
  2016-09-15 16:45                           ` [PATCH v4 14/20] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
@ 2016-09-15 16:45                             ` Trond Myklebust
  2016-09-15 16:45                               ` [PATCH v4 16/20] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If the server tells us our stateid has expired, then handle that as if
it was revoked.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 84ee836a976c..75edf90455b4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1919,7 +1919,6 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 		case -NFS4ERR_STALE_CLIENTID:
 		case -NFS4ERR_STALE_STATEID:
 			set_bit(NFS_DELEGATED_STATE, &state->flags);
-		case -NFS4ERR_EXPIRED:
 			/* Don't recall a delegation if it was lost */
 			nfs4_schedule_lease_recovery(server->nfs_client);
 			return -EAGAIN;
@@ -1931,6 +1930,7 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 			return -EAGAIN;
 		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_ADMIN_REVOKED:
+		case -NFS4ERR_EXPIRED:
 		case -NFS4ERR_BAD_STATEID:
 		case -NFS4ERR_OPENMODE:
 			nfs_inode_find_state_and_recover(state->inode,
-- 
2.7.4


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

* [PATCH v4 16/20] NFSv4: nfs4_handle_setlk_error() handle expiration as revoke case
  2016-09-15 16:45                             ` [PATCH v4 15/20] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
@ 2016-09-15 16:45                               ` Trond Myklebust
  2016-09-15 16:45                                 ` [PATCH v4 17/20] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If the server tells us our stateid has expired, then handle that as if
it was revoked.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 75edf90455b4..0c050e932c7e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6086,6 +6086,7 @@ static void nfs4_handle_setlk_error(struct nfs_server *server, struct nfs4_lock_
 {
 	switch (error) {
 	case -NFS4ERR_ADMIN_REVOKED:
+	case -NFS4ERR_EXPIRED:
 	case -NFS4ERR_BAD_STATEID:
 		lsp->ls_seqid.flags &= ~NFS_SEQID_CONFIRMED;
 		if (new_lock_owner != 0 ||
@@ -6094,7 +6095,6 @@ static void nfs4_handle_setlk_error(struct nfs_server *server, struct nfs4_lock_
 		break;
 	case -NFS4ERR_STALE_STATEID:
 		lsp->ls_seqid.flags &= ~NFS_SEQID_CONFIRMED;
-	case -NFS4ERR_EXPIRED:
 		nfs4_schedule_lease_recovery(server->nfs_client);
 	};
 }
-- 
2.7.4


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

* [PATCH v4 17/20] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state
  2016-09-15 16:45                               ` [PATCH v4 16/20] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
@ 2016-09-15 16:45                                 ` Trond Myklebust
  2016-09-15 16:45                                   ` [PATCH v4 18/20] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

Handle revoked open/lock/delegation stateids when LAYOUTGET tells us
the state was revoked.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0c050e932c7e..e2ab6f665dba 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8069,6 +8069,8 @@ nfs4_layoutget_handle_exception(struct rpc_task *task,
 	case -NFS4ERR_RECALLCONFLICT:
 		status = -ERECALLCONFLICT;
 		break;
+	case -NFS4ERR_DELEG_REVOKED:
+	case -NFS4ERR_ADMIN_REVOKED:
 	case -NFS4ERR_EXPIRED:
 	case -NFS4ERR_BAD_STATEID:
 		exception->timeout = 0;
@@ -8080,6 +8082,7 @@ nfs4_layoutget_handle_exception(struct rpc_task *task,
 					&lgp->args.ctx->state->stateid)) {
 			spin_unlock(&inode->i_lock);
 			exception->state = lgp->args.ctx->state;
+			exception->stateid = &lgp->args.stateid;
 			break;
 		}
 
-- 
2.7.4


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

* [PATCH v4 18/20] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb
  2016-09-15 16:45                                 ` [PATCH v4 17/20] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
@ 2016-09-15 16:45                                   ` Trond Myklebust
  2016-09-15 16:45                                     ` [PATCH v4 19/20] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

The actual stateid used in the READ or WRITE can represent a delegation,
a lock or a stateid, so it is useful to pass it as an argument to the
exception handler when an expired/revoked response is received from the
server. It also ensures that we don't re-label the state as needing
recovery if that has already occurred.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e2ab6f665dba..82449e578608 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4523,11 +4523,18 @@ static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_pgio_header *hdr)
 	struct nfs_server *server = NFS_SERVER(hdr->inode);
 
 	trace_nfs4_read(hdr, task->tk_status);
-	if (nfs4_async_handle_error(task, server,
-				    hdr->args.context->state,
-				    NULL) == -EAGAIN) {
-		rpc_restart_call_prepare(task);
-		return -EAGAIN;
+	if (task->tk_status < 0) {
+		struct nfs4_exception exception = {
+			.inode = hdr->inode,
+			.state = hdr->args.context->state,
+			.stateid = &hdr->args.stateid,
+		};
+		task->tk_status = nfs4_async_handle_exception(task,
+				server, task->tk_status, &exception);
+		if (exception.retry) {
+			rpc_restart_call_prepare(task);
+			return -EAGAIN;
+		}
 	}
 
 	__nfs4_read_done_cb(hdr);
@@ -4596,11 +4603,19 @@ static int nfs4_write_done_cb(struct rpc_task *task,
 	struct inode *inode = hdr->inode;
 
 	trace_nfs4_write(hdr, task->tk_status);
-	if (nfs4_async_handle_error(task, NFS_SERVER(inode),
-				    hdr->args.context->state,
-				    NULL) == -EAGAIN) {
-		rpc_restart_call_prepare(task);
-		return -EAGAIN;
+	if (task->tk_status < 0) {
+		struct nfs4_exception exception = {
+			.inode = hdr->inode,
+			.state = hdr->args.context->state,
+			.stateid = &hdr->args.stateid,
+		};
+		task->tk_status = nfs4_async_handle_exception(task,
+				NFS_SERVER(inode), task->tk_status,
+				&exception);
+		if (exception.retry) {
+			rpc_restart_call_prepare(task);
+			return -EAGAIN;
+		}
 	}
 	if (task->tk_status >= 0) {
 		renew_lease(NFS_SERVER(inode), hdr->timestamp);
-- 
2.7.4


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

* [PATCH v4 19/20] NFSv4: Fix a race in nfs_inode_reclaim_delegation()
  2016-09-15 16:45                                   ` [PATCH v4 18/20] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
@ 2016-09-15 16:45                                     ` Trond Myklebust
  2016-09-15 16:46                                       ` [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:45 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If we race with a delegreturn before taking the spin lock, we
currently end up dropping the delegation stateid.

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

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index efd50159ab4a..a17d9a15eb3b 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -185,15 +185,13 @@ void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred,
 			rcu_read_unlock();
 			put_rpccred(oldcred);
 			trace_nfs4_reclaim_delegation(inode, res->delegation_type);
-		} else {
-			/* We appear to have raced with a delegation return. */
-			spin_unlock(&delegation->lock);
-			rcu_read_unlock();
-			nfs_inode_set_delegation(inode, cred, res);
+			return;
 		}
-	} else {
-		rcu_read_unlock();
+		/* We appear to have raced with a delegation return. */
+		spin_unlock(&delegation->lock);
 	}
+	rcu_read_unlock();
+	nfs_inode_set_delegation(inode, cred, res);
 }
 
 static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
-- 
2.7.4


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

* [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid
  2016-09-15 16:45                                     ` [PATCH v4 19/20] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
@ 2016-09-15 16:46                                       ` Trond Myklebust
  2016-09-15 22:17                                         ` kbuild test robot
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-15 16:46 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If we're replacing an old stateid which has a different 'other' field,
then we probably need to free the old stateid.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 82449e578608..08c6e46c5b95 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1399,11 +1399,12 @@ static void nfs_test_and_clear_all_open_stateid(struct nfs4_state *state)
 }
 
 static bool nfs_need_update_open_stateid(struct nfs4_state *state,
-		nfs4_stateid *stateid)
+		const nfs4_stateid *stateid, nfs4_stateid *freeme)
 {
 	if (test_and_set_bit(NFS_OPEN_STATE, &state->flags) == 0)
 		return true;
 	if (!nfs4_stateid_match_other(stateid, &state->open_stateid)) {
+		nfs4_stateid_copy(freeme, &state->open_stateid);
 		nfs_test_and_clear_all_open_stateid(state);
 		return true;
 	}
@@ -1467,7 +1468,9 @@ static void nfs_clear_open_stateid(struct nfs4_state *state,
 		nfs4_schedule_state_manager(state->owner->so_server->nfs_client);
 }
 
-static void nfs_set_open_stateid_locked(struct nfs4_state *state, nfs4_stateid *stateid, fmode_t fmode)
+static void nfs_set_open_stateid_locked(struct nfs4_state *state,
+		const nfs4_stateid *stateid, fmode_t fmode,
+		nfs4_stateid *freeme)
 {
 	switch (fmode) {
 		case FMODE_READ:
@@ -1479,14 +1482,18 @@ static void nfs_set_open_stateid_locked(struct nfs4_state *state, nfs4_stateid *
 		case FMODE_READ|FMODE_WRITE:
 			set_bit(NFS_O_RDWR_STATE, &state->flags);
 	}
-	if (!nfs_need_update_open_stateid(state, stateid))
+	if (!nfs_need_update_open_stateid(state, stateid, freeme))
 		return;
 	if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
 		nfs4_stateid_copy(&state->stateid, stateid);
 	nfs4_stateid_copy(&state->open_stateid, stateid);
 }
 
-static void __update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, const nfs4_stateid *deleg_stateid, fmode_t fmode)
+static void __update_open_stateid(struct nfs4_state *state,
+		const nfs4_stateid *open_stateid,
+		const nfs4_stateid *deleg_stateid,
+		fmode_t fmode,
+		nfs4_stateid *freeme)
 {
 	/*
 	 * Protect the call to nfs4_state_set_mode_locked and
@@ -1499,16 +1506,22 @@ static void __update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_s
 		set_bit(NFS_DELEGATED_STATE, &state->flags);
 	}
 	if (open_stateid != NULL)
-		nfs_set_open_stateid_locked(state, open_stateid, fmode);
+		nfs_set_open_stateid_locked(state, open_stateid, fmode, freeme);
 	write_sequnlock(&state->seqlock);
 	update_open_stateflags(state, fmode);
 	spin_unlock(&state->owner->so_lock);
 }
 
-static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, nfs4_stateid *delegation, fmode_t fmode)
+static int update_open_stateid(struct nfs4_state *state,
+		const nfs4_stateid *open_stateid,
+		const nfs4_stateid *delegation,
+		fmode_t fmode)
 {
+	struct nfs_server *server = NFS_SERVER(state->inode);
+	struct nfs_client *clp = server->nfs_client;
 	struct nfs_inode *nfsi = NFS_I(state->inode);
 	struct nfs_delegation *deleg_cur;
+	nfs4_stateid freeme = {0};
 	int ret = 0;
 
 	fmode &= (FMODE_READ|FMODE_WRITE);
@@ -1530,7 +1543,8 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
 		goto no_delegation_unlock;
 
 	nfs_mark_delegation_referenced(deleg_cur);
-	__update_open_stateid(state, open_stateid, &deleg_cur->stateid, fmode);
+	__update_open_stateid(state, open_stateid, &deleg_cur->stateid,
+			fmode, &freeme);
 	ret = 1;
 no_delegation_unlock:
 	spin_unlock(&deleg_cur->lock);
@@ -1538,11 +1552,14 @@ no_delegation:
 	rcu_read_unlock();
 
 	if (!ret && open_stateid != NULL) {
-		__update_open_stateid(state, open_stateid, NULL, fmode);
+		__update_open_stateid(state, open_stateid, NULL, fmode, &freeme);
 		ret = 1;
 	}
 	if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags))
-		nfs4_schedule_state_manager(state->owner->so_server->nfs_client);
+		nfs4_schedule_state_manager(clp);
+	if (freeme.type != 0)
+		nfs4_test_and_free_stateid(server, &freeme,
+				state->owner->so_cred);
 
 	return ret;
 }
-- 
2.7.4


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

* Re: [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid
  2016-09-15 16:46                                       ` [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
@ 2016-09-15 22:17                                         ` kbuild test robot
  0 siblings, 0 replies; 27+ messages in thread
From: kbuild test robot @ 2016-09-15 22:17 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: kbuild-all, anna.schumaker, linux-nfs, Oleg Drokin

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

Hi Trond,

[auto build test WARNING on nfs/linux-next]
[also build test WARNING on v4.8-rc6 next-20160915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Trond-Myklebust/Fix-delegation-behaviour-when-server-revokes-some-state/20160916-050650
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All warnings (new ones prefixed by >>):

   fs/nfs/nfs4proc.c: In function 'update_open_stateid':
>> fs/nfs/nfs4proc.c:1524:2: warning: missing braces around initializer [-Wmissing-braces]
     nfs4_stateid freeme = {0};
     ^
   fs/nfs/nfs4proc.c:1524:2: warning: (near initialization for 'freeme.<anonymous>') [-Wmissing-braces]

vim +1524 fs/nfs/nfs4proc.c

  1508		if (open_stateid != NULL)
  1509			nfs_set_open_stateid_locked(state, open_stateid, fmode, freeme);
  1510		write_sequnlock(&state->seqlock);
  1511		update_open_stateflags(state, fmode);
  1512		spin_unlock(&state->owner->so_lock);
  1513	}
  1514	
  1515	static int update_open_stateid(struct nfs4_state *state,
  1516			const nfs4_stateid *open_stateid,
  1517			const nfs4_stateid *delegation,
  1518			fmode_t fmode)
  1519	{
  1520		struct nfs_server *server = NFS_SERVER(state->inode);
  1521		struct nfs_client *clp = server->nfs_client;
  1522		struct nfs_inode *nfsi = NFS_I(state->inode);
  1523		struct nfs_delegation *deleg_cur;
> 1524		nfs4_stateid freeme = {0};
  1525		int ret = 0;
  1526	
  1527		fmode &= (FMODE_READ|FMODE_WRITE);
  1528	
  1529		rcu_read_lock();
  1530		deleg_cur = rcu_dereference(nfsi->delegation);
  1531		if (deleg_cur == NULL)
  1532			goto no_delegation;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 11444 bytes --]

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

* Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
  2016-09-15 16:45 [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Trond Myklebust
  2016-09-15 16:45 ` [PATCH v4 01/20] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
@ 2016-09-16  4:38 ` Oleg Drokin
  2016-09-16 15:36   ` Trond Myklebust
  1 sibling, 1 reply; 27+ messages in thread
From: Oleg Drokin @ 2016-09-16  4:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs

On Sep 15, 2016, at 12:45 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
> v4: Close a bunch of corner cases

This one seems to be looping on the client in a way very similar
to the first failure in that it's the serverip-named process that's
using the cpu, but the debug log is very similar to the second failure
in test stateid, except this time it's not "success 0", but "success -10025":

[21114.650534] NFS call  test_stateid ffff8800ca2f3da4
[21114.650537] --> nfs41_call_sync_prepare data->seq_server ffff880099003000
[21114.650538] --> nfs41_setup_sequence
[21114.650538] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21114.650539] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21114.650539] <-- nfs41_setup_sequence slotid=0 seqid=160421364
[21114.650544] encode_sequence: sessionid=1473979571:1:2:0 seqid=160421364 slotid=0 max_slotid=0 cache_this=0
[21114.650646] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21114.650646] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21114.650647] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21114.650648] nfs41_sequence_process: Error 0 free the slot 
[21114.650648] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21114.650660] NFS reply test_stateid: succeeded, -10025
[21114.650663] NFS call  test_stateid ffff8800ca2f3da4
[21114.650666] --> nfs41_call_sync_prepare data->seq_server ffff880099003000
[21114.650666] --> nfs41_setup_sequence
[21114.650667] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21114.650667] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21114.650667] <-- nfs41_setup_sequence slotid=0 seqid=160421365
[21114.650673] encode_sequence: sessionid=1473979571:1:2:0 seqid=160421365 slotid=0 max_slotid=0 cache_this=0
[21114.650792] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21114.650792] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21114.650793] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21114.650793] nfs41_sequence_process: Error 0 free the slot 
[21114.650794] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21114.650806] NFS reply test_stateid: succeeded, -10025
[21114.650809] NFS call  test_stateid ffff8800ca2f3da4
[21114.650812] --> nfs41_call_sync_prepare data->seq_server ffff880099003000
[21114.650812] --> nfs41_setup_sequence
[21114.650812] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[21114.650813] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[21114.650813] <-- nfs41_setup_sequence slotid=0 seqid=160421366
[21114.650819] encode_sequence: sessionid=1473979571:1:2:0 seqid=160421366 slotid=0 max_slotid=0 cache_this=0
[21114.650922] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[21114.650922] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[21114.650923] nfs4_free_slot: slotid 1 highest_used_slotid 0
[21114.650923] nfs41_sequence_process: Error 0 free the slot 
[21114.650924] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[21114.650936] NFS reply test_stateid: succeeded, -10025



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

* Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
  2016-09-16  4:38 ` [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Oleg Drokin
@ 2016-09-16 15:36   ` Trond Myklebust
  2016-09-16 21:15     ` Oleg Drokin
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2016-09-16 15:36 UTC (permalink / raw)
  To: Trond Myklebust, green; +Cc: anna.schumaker, linux-nfs

T24gRnJpLCAyMDE2LTA5LTE2IGF0IDAwOjM4IC0wNDAwLCBPbGVnIERyb2tpbiB3cm90ZToNCj4g
T24gU2VwIDE1LCAyMDE2LCBhdCAxMjo0NSBQTSwgVHJvbmQgTXlrbGVidXN0IHdyb3RlOg0KPiAN
Cj4gPiANCj4gPiBBY2NvcmRpbmcgdG8gUkZDNTY2MSwgaWYgYW55IG9mIHRoZSBTRVFVRU5DRSBz
dGF0dXMgYml0cw0KPiA+IFNFUTRfU1RBVFVTX0VYUElSRURfQUxMX1NUQVRFX1JFVk9LRUQsDQo+
ID4gU0VRNF9TVEFUVVNfRVhQSVJFRF9TT01FX1NUQVRFX1JFVk9LRUQsDQo+ID4gU0VRNF9TVEFU
VVNfQURNSU5fU1RBVEVfUkVWT0tFRCwNCj4gPiBvciBTRVE0X1NUQVRVU19SRUNBTExBQkxFX1NU
QVRFX1JFVk9LRUQgYXJlIHNldCwgdGhlbiB3ZSBuZWVkIHRvDQo+ID4gdXNlDQo+ID4gVEVTVF9T
VEFURUlEIHRvIGZpZ3VyZSBvdXQgd2hpY2ggc3RhdGVpZHMgaGF2ZSBiZWVuIHJldm9rZWQsIHNv
IHdlDQo+ID4gY2FuIGFja25vd2xlZGdlIHRoZSBsb3NzIG9mIHN0YXRlIHVzaW5nIEZSRUVfU1RB
VEVJRC4NCj4gPiANCj4gPiBXaGlsZSB3ZSBhbHJlYWR5IGRvIHRoaXMgZm9yIG9wZW4gYW5kIGxv
Y2sgc3RhdGUsIHdlIGhhdmUgbm90IGJlZW4NCj4gPiBkb2luZw0KPiA+IHNvIGZvciBhbGwgdGhl
IGRlbGVnYXRpb25zLg0KPiA+IA0KPiA+IHYyOiBuZnNfdjRfMl9taW5vcl9vcHMgbmVlZHMgdG8g
c2V0IC50ZXN0X2FuZF9mcmVlX2V4cGlyZWQgdG9vDQo+ID4gdjM6IE5vdyB3aXRoIGFkZGVkIGxv
Y2sgcmV2b2tlIGZpeGVzIGFuZCBjbG9zZS9kZWxlZ3JldHVybi9sb2NrdQ0KPiA+IGZpeGVzDQo+
ID4gdjQ6IENsb3NlIGEgYnVuY2ggb2YgY29ybmVyIGNhc2VzDQo+IA0KPiBUaGlzIG9uZSBzZWVt
cyB0byBiZSBsb29waW5nIG9uIHRoZSBjbGllbnQgaW4gYSB3YXkgdmVyeSBzaW1pbGFyDQo+IHRv
IHRoZSBmaXJzdCBmYWlsdXJlIGluIHRoYXQgaXQncyB0aGUgc2VydmVyaXAtbmFtZWQgcHJvY2Vz
cyB0aGF0J3MNCj4gdXNpbmcgdGhlIGNwdSwgYnV0IHRoZSBkZWJ1ZyBsb2cgaXMgdmVyeSBzaW1p
bGFyIHRvIHRoZSBzZWNvbmQNCj4gZmFpbHVyZQ0KPiBpbiB0ZXN0IHN0YXRlaWQsIGV4Y2VwdCB0
aGlzIHRpbWUgaXQncyBub3QgInN1Y2Nlc3MgMCIsIGJ1dCAic3VjY2Vzcw0KPiAtMTAwMjUiOg0K
PsKgDQpBaC4uLiBJIHRoaW5rIEkgc2VlIHdoYXQgdGhlIGlzc3VlIGlzLi4uIERvZXMgdGhlIGZv
bGxvd2luZyBoZWxwPw0KDQpDaGVlcnMNCsKgIFRyb25kDQoNCjg8LS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQpGcm9tIGQ5ODg3
MzQ1ZGJlZmZjYzdiOGJkZTE3YjA2ZGY4ZjA3NjdmMjFhYmYgTW9uIFNlcCAxNyAwMDowMDowMCAy
MDAxDQpGcm9tOiBUcm9uZCBNeWtsZWJ1c3QgPHRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5j
b20+DQpEYXRlOiBGcmksIDE2IFNlcCAyMDE2IDExOjE3OjA0IC0wNDAwDQpTdWJqZWN0OiBbUEFU
Q0ggdjUgMTQvMjJdIE5GU3Y0OiBFbnN1cmUgd2UgZG9uJ3QgcmUtdGVzdCByZXZva2VkIGFuZCBm
cmVlZA0KIHN0YXRlaWRzDQoNClRoaXMgZml4ZXMgYSBwb3RlbnRpYWwgaW5maW5pdGUgbG9vcCBp
biBuZnNfcmVhcF9leHBpcmVkX2RlbGVnYXRpb25zLg0KDQpTaWduZWQtb2ZmLWJ5OiBUcm9uZCBN
eWtsZWJ1c3QgPHRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20+DQotLS0NCiBmcy9uZnMv
ZGVsZWdhdGlvbi5jIHwgMyArKysNCiAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25zKCspDQoN
CmRpZmYgLS1naXQgYS9mcy9uZnMvZGVsZWdhdGlvbi5jIGIvZnMvbmZzL2RlbGVnYXRpb24uYw0K
aW5kZXggZjQzMWZkZTEzZTkwLi4zNGFhNWRmYmU5OWUgMTAwNjQ0DQotLS0gYS9mcy9uZnMvZGVs
ZWdhdGlvbi5jDQorKysgYi9mcy9uZnMvZGVsZWdhdGlvbi5jDQpAQCAtNjQ3LDYgKzY0Nyw3IEBA
IHN0YXRpYyB2b2lkIG5mc19tYXJrX2RlbGVnYXRpb25fcmV2b2tlZChzdHJ1Y3QgbmZzX3NlcnZl
ciAqc2VydmVyLA0KIAkJc3RydWN0IG5mc19kZWxlZ2F0aW9uICpkZWxlZ2F0aW9uKQ0KIHsNCiAJ
c2V0X2JpdChORlNfREVMRUdBVElPTl9SRVZPS0VELCAmZGVsZWdhdGlvbi0+ZmxhZ3MpOw0KKwlk
ZWxlZ2F0aW9uLT5zdGF0ZWlkLnR5cGUgPSBORlM0X0lOVkFMSURfU1RBVEVJRF9UWVBFOw0KIAlu
ZnNfbWFya19yZXR1cm5fZGVsZWdhdGlvbihzZXJ2ZXIsIGRlbGVnYXRpb24pOw0KIH0NCiANCkBA
IC04ODUsNiArODg2LDggQEAgc3RhdGljIGlubGluZSBib29sIG5mczRfc2VydmVyX3JlYm9vdGVk
KGNvbnN0IHN0cnVjdCBuZnNfY2xpZW50ICpjbHApDQogc3RhdGljIHZvaWQgbmZzX21hcmtfdGVz
dF9leHBpcmVkX2RlbGVnYXRpb24oc3RydWN0IG5mc19zZXJ2ZXIgKnNlcnZlciwNCiAJICAgIHN0
cnVjdCBuZnNfZGVsZWdhdGlvbiAqZGVsZWdhdGlvbikNCiB7DQorCWlmIChkZWxlZ2F0aW9uLT5z
dGF0ZWlkLnR5cGUgPT0gTkZTNF9JTlZBTElEX1NUQVRFSURfVFlQRSkNCisJCXJldHVybjsNCiAJ
Y2xlYXJfYml0KE5GU19ERUxFR0FUSU9OX05FRURfUkVDTEFJTSwgJmRlbGVnYXRpb24tPmZsYWdz
KTsNCiAJc2V0X2JpdChORlNfREVMRUdBVElPTl9URVNUX0VYUElSRUQsICZkZWxlZ2F0aW9uLT5m
bGFncyk7DQogCXNldF9iaXQoTkZTNENMTlRfREVMRUdBVElPTl9FWFBJUkVELCAmc2VydmVyLT5u
ZnNfY2xpZW50LT5jbF9zdGF0ZSk7DQotLSANCjIuNy40


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

* Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
  2016-09-16 15:36   ` Trond Myklebust
@ 2016-09-16 21:15     ` Oleg Drokin
  2016-09-16 21:21       ` Oleg Drokin
  0 siblings, 1 reply; 27+ messages in thread
From: Oleg Drokin @ 2016-09-16 21:15 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs


On Sep 16, 2016, at 11:36 AM, Trond Myklebust wrote:

> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>> On Sep 15, 2016, at 12:45 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
>>> v4: Close a bunch of corner cases
>> 
>> This one seems to be looping on the client in a way very similar
>> to the first failure in that it's the serverip-named process that's
>> using the cpu, but the debug log is very similar to the second
>> failure
>> in test stateid, except this time it's not "success 0", but "success
>> -10025":
>>  
> Ah... I think I see what the issue is... Does the following help?

Well, it changed the output back to what I had with the first patch set, I think:
I.e. now it's nfsid test succeded, 0, and the process using the cpu is again
a userspace one:

[14231.187643] NFS call  test_stateid ffff88005e979f24
[14231.187648] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.187648] --> nfs41_setup_sequence
[14231.187650] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.187651] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.187651] <-- nfs41_setup_sequence slotid=0 seqid=3355909
[14231.187660] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355909 slotid=0 max_slotid=0 cache_this=0
[14231.187879] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.187880] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.187882] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.187882] nfs41_sequence_process: Error 0 free the slot 
[14231.187884] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.187903] NFS reply test_stateid: succeeded, 0
[14231.187911] --> nfs_put_client({3})
[14231.187969] --> nfs_put_client({2})
[14231.187976] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.187977] --> nfs41_setup_sequence
[14231.187978] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.187979] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.187980] <-- nfs41_setup_sequence slotid=0 seqid=3355910
[14231.187989] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355910 slotid=0 max_slotid=0 cache_this=1
[14231.188151] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.188152] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.188154] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.188155] nfs41_sequence_process: Error 0 free the slot 
[14231.188156] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.188178] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
[14231.188355] NFS call  test_stateid ffff88005e979f24
[14231.188360] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.188361] --> nfs41_setup_sequence
[14231.188362] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.188363] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.188364] <-- nfs41_setup_sequence slotid=0 seqid=3355911
[14231.188373] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355911 slotid=0 max_slotid=0 cache_this=0
[14231.188584] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.188586] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.188587] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.188588] nfs41_sequence_process: Error 0 free the slot 
[14231.188589] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.188608] NFS reply test_stateid: succeeded, 0
[14231.188617] --> nfs_put_client({3})
[14231.188672] --> nfs_put_client({2})
[14231.188679] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.188680] --> nfs41_setup_sequence
[14231.188681] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.188682] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.188683] <-- nfs41_setup_sequence slotid=0 seqid=3355912
[14231.188692] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355912 slotid=0 max_slotid=0 cache_this=1
[14231.188848] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.188849] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.188851] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.188855] nfs41_sequence_process: Error 0 free the slot 
[14231.188871] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.188891] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
[14231.189059] NFS call  test_stateid ffff88005e979f24
[14231.189064] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.189065] --> nfs41_setup_sequence
[14231.189066] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.189067] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.189068] <-- nfs41_setup_sequence slotid=0 seqid=3355913
[14231.189077] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355913 slotid=0 max_slotid=0 cache_this=0
[14231.189289] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.189290] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.189291] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.189292] nfs41_sequence_process: Error 0 free the slot 
[14231.189294] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.189311] NFS reply test_stateid: succeeded, 0
[14231.189320] --> nfs_put_client({3})
[14231.189373] --> nfs_put_client({2})
[14231.189380] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
[14231.189381] --> nfs41_setup_sequence
[14231.189382] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[14231.189383] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[14231.189384] <-- nfs41_setup_sequence slotid=0 seqid=3355914
[14231.189393] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355914 slotid=0 max_slotid=0 cache_this=1
[14231.193709] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[14231.193710] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[14231.193712] nfs4_free_slot: slotid 1 highest_used_slotid 0
[14231.193713] nfs41_sequence_process: Error 0 free the slot 
[14231.193715] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[14231.193742] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost



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

* Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
  2016-09-16 21:15     ` Oleg Drokin
@ 2016-09-16 21:21       ` Oleg Drokin
  2016-09-16 21:28         ` Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Oleg Drokin @ 2016-09-16 21:21 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: anna.schumaker@netapp.com Schumaker,
	linux-nfs@vger.kernel.org NFS Mailing


On Sep 16, 2016, at 5:15 PM, Oleg Drokin wrote:

> 
> On Sep 16, 2016, at 11:36 AM, Trond Myklebust wrote:
> 
>> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>>> On Sep 15, 2016, at 12:45 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
>>>> v4: Close a bunch of corner cases
>>> 
>>> This one seems to be looping on the client in a way very similar
>>> to the first failure in that it's the serverip-named process that's
>>> using the cpu, but the debug log is very similar to the second
>>> failure
>>> in test stateid, except this time it's not "success 0", but "success
>>> -10025":
>>> 
>> Ah... I think I see what the issue is... Does the following help?
> 
> Well, it changed the output back to what I had with the first patch set, I think:
> I.e. now it's nfsid test succeded, 0, and the process using the cpu is again
> a userspace one:

Ah, I did not realize you probably meant not to apply this on top of the previous
batch, but replace patch #14 with this one.
So I am trying it now.

> 
> [14231.187643] NFS call  test_stateid ffff88005e979f24
> [14231.187648] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.187648] --> nfs41_setup_sequence
> [14231.187650] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.187651] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.187651] <-- nfs41_setup_sequence slotid=0 seqid=3355909
> [14231.187660] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355909 slotid=0 max_slotid=0 cache_this=0
> [14231.187879] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.187880] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.187882] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.187882] nfs41_sequence_process: Error 0 free the slot 
> [14231.187884] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.187903] NFS reply test_stateid: succeeded, 0
> [14231.187911] --> nfs_put_client({3})
> [14231.187969] --> nfs_put_client({2})
> [14231.187976] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.187977] --> nfs41_setup_sequence
> [14231.187978] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.187979] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.187980] <-- nfs41_setup_sequence slotid=0 seqid=3355910
> [14231.187989] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355910 slotid=0 max_slotid=0 cache_this=1
> [14231.188151] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.188152] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.188154] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.188155] nfs41_sequence_process: Error 0 free the slot 
> [14231.188156] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.188178] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
> [14231.188355] NFS call  test_stateid ffff88005e979f24
> [14231.188360] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.188361] --> nfs41_setup_sequence
> [14231.188362] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.188363] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.188364] <-- nfs41_setup_sequence slotid=0 seqid=3355911
> [14231.188373] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355911 slotid=0 max_slotid=0 cache_this=0
> [14231.188584] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.188586] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.188587] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.188588] nfs41_sequence_process: Error 0 free the slot 
> [14231.188589] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.188608] NFS reply test_stateid: succeeded, 0
> [14231.188617] --> nfs_put_client({3})
> [14231.188672] --> nfs_put_client({2})
> [14231.188679] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.188680] --> nfs41_setup_sequence
> [14231.188681] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.188682] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.188683] <-- nfs41_setup_sequence slotid=0 seqid=3355912
> [14231.188692] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355912 slotid=0 max_slotid=0 cache_this=1
> [14231.188848] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.188849] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.188851] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.188855] nfs41_sequence_process: Error 0 free the slot 
> [14231.188871] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.188891] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
> [14231.189059] NFS call  test_stateid ffff88005e979f24
> [14231.189064] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.189065] --> nfs41_setup_sequence
> [14231.189066] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.189067] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.189068] <-- nfs41_setup_sequence slotid=0 seqid=3355913
> [14231.189077] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355913 slotid=0 max_slotid=0 cache_this=0
> [14231.189289] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.189290] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.189291] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.189292] nfs41_sequence_process: Error 0 free the slot 
> [14231.189294] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.189311] NFS reply test_stateid: succeeded, 0
> [14231.189320] --> nfs_put_client({3})
> [14231.189373] --> nfs_put_client({2})
> [14231.189380] --> nfs41_call_sync_prepare data->seq_server ffff8800aadcb000
> [14231.189381] --> nfs41_setup_sequence
> [14231.189382] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
> [14231.189383] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
> [14231.189384] <-- nfs41_setup_sequence slotid=0 seqid=3355914
> [14231.189393] encode_sequence: sessionid=1474050426:3:4:0 seqid=3355914 slotid=0 max_slotid=0 cache_this=1
> [14231.193709] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
> [14231.193710] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
> [14231.193712] nfs4_free_slot: slotid 1 highest_used_slotid 0
> [14231.193713] nfs41_sequence_process: Error 0 free the slot 
> [14231.193715] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
> [14231.193742] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost
> 
> 


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

* Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state
  2016-09-16 21:21       ` Oleg Drokin
@ 2016-09-16 21:28         ` Trond Myklebust
  0 siblings, 0 replies; 27+ messages in thread
From: Trond Myklebust @ 2016-09-16 21:28 UTC (permalink / raw)
  To: Oleg Drokin; +Cc: Schumaker Anna, List Linux NFS Mailing


> On Sep 16, 2016, at 17:21, Oleg Drokin <green@linuxhacker.ru> wrote:
>=20
>=20
> On Sep 16, 2016, at 5:15 PM, Oleg Drokin wrote:
>=20
>>=20
>> On Sep 16, 2016, at 11:36 AM, Trond Myklebust wrote:
>>=20
>>> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>>>> On Sep 15, 2016, at 12:45 PM, Trond Myklebust wrote:
>>>>=20
>>>>>=20
>>>>> 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.
>>>>>=20
>>>>> While we already do this for open and lock state, we have not been
>>>>> doing
>>>>> so for all the delegations.
>>>>>=20
>>>>> 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
>>>>> v4: Close a bunch of corner cases
>>>>=20
>>>> This one seems to be looping on the client in a way very similar
>>>> to the first failure in that it's the serverip-named process that's
>>>> using the cpu, but the debug log is very similar to the second
>>>> failure
>>>> in test stateid, except this time it's not "success 0", but "success
>>>> -10025":
>>>>=20
>>> Ah... I think I see what the issue is... Does the following help?
>>=20
>> Well, it changed the output back to what I had with the first patch set,=
 I think:
>> I.e. now it's nfsid test succeded, 0, and the process using the cpu is a=
gain
>> a userspace one:
>=20
> Ah, I did not realize you probably meant not to apply this on top of the =
previous
> batch, but replace patch #14 with this one.
> So I am trying it now.

No, not at all. What you did was entirely correct. I=92ve inserted it into =
position 14 in the =91v5=92 patch series because that is where it belongs i=
n order to prevent bisection issues, but it should work identically on top =
of the existing v4...

Thanks!


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

end of thread, other threads:[~2016-09-16 21:28 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 16:45 [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Trond Myklebust
2016-09-15 16:45 ` [PATCH v4 01/20] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
2016-09-15 16:45   ` [PATCH v4 02/20] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
2016-09-15 16:45     ` [PATCH v4 03/20] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
2016-09-15 16:45       ` [PATCH v4 04/20] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
2016-09-15 16:45         ` [PATCH v4 05/20] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
2016-09-15 16:45           ` [PATCH v4 06/20] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
2016-09-15 16:45             ` [PATCH v4 07/20] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
2016-09-15 16:45               ` [PATCH v4 08/20] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
2016-09-15 16:45                 ` [PATCH v4 09/20] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
2016-09-15 16:45                   ` [PATCH v4 10/20] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
2016-09-15 16:45                     ` [PATCH v4 11/20] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
2016-09-15 16:45                       ` [PATCH v4 12/20] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
2016-09-15 16:45                         ` [PATCH v4 13/20] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
2016-09-15 16:45                           ` [PATCH v4 14/20] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
2016-09-15 16:45                             ` [PATCH v4 15/20] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
2016-09-15 16:45                               ` [PATCH v4 16/20] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
2016-09-15 16:45                                 ` [PATCH v4 17/20] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
2016-09-15 16:45                                   ` [PATCH v4 18/20] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
2016-09-15 16:45                                     ` [PATCH v4 19/20] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
2016-09-15 16:46                                       ` [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
2016-09-15 22:17                                         ` kbuild test robot
2016-09-16  4:38 ` [PATCH v4 00/20] Fix delegation behaviour when server revokes some state Oleg Drokin
2016-09-16 15:36   ` Trond Myklebust
2016-09-16 21:15     ` Oleg Drokin
2016-09-16 21:21       ` Oleg Drokin
2016-09-16 21:28         ` Trond Myklebust

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.