All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 00/29] Fix delegation behaviour when server revokes some state
@ 2016-09-20 16:55 Trond Myklebust
  2016-09-20 16:55 ` [PATCH v6 01/29] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
  2016-09-20 22:06 ` [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Oleg Drokin
  0 siblings, 2 replies; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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
v5: Report revoked delegations as invalid in nfs_have_delegation()
    Fix an infinite loop in nfs_reap_expired_delegations.
        Fixes for other looping behaviour
v6: Fix nfs4_do_handle_exception to handle all stateids, not just delegations
    Stable fix for nfs4_copy_delegation_stateid
    Marked fix "NFSv4: Don't report revoked delegations as valid in
        nfs_have_delegation" for stable.
    Stable fix for the inode mode/fileid corruption

Trond Myklebust (29):
  NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
  NFS: Fix inode corruption in nfs_prime_dcache()
  NFSv4: Don't report revoked delegations as valid in
    nfs_have_delegation()
  NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is
    invalid
  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: Ensure we don't re-test revoked and freed stateids
  NFSv4: nfs_inode_find_delegation_state_and_recover() should check all
    stateids
  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
  NFS: Always call nfs_inode_find_state_and_recover() when revoking a
    delegation
  NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single
    stateid
  NFSv4: Don't test open_stateid unless it is set
  NFSv4: Mark the lock and open stateids as invalid after freeing them
  NFSv4: Open state recovery must account for file permission changes
  NFSv4: Fix retry issues with nfs41_test/free_stateid

 fs/nfs/delegation.c                    | 212 ++++++++++++++++--
 fs/nfs/delegation.h                    |   8 +-
 fs/nfs/dir.c                           |  14 +-
 fs/nfs/flexfilelayout/flexfilelayout.c |   2 +-
 fs/nfs/nfs4_fs.h                       |   5 +-
 fs/nfs/nfs4proc.c                      | 378 +++++++++++++++++++++++----------
 fs/nfs/nfs4session.h                   |   1 +
 fs/nfs/nfs4state.c                     |  76 +++++--
 include/linux/nfs4.h                   |   1 +
 9 files changed, 538 insertions(+), 159 deletions(-)

-- 
2.7.4


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

* [PATCH v6 01/29] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
  2016-09-20 16:55 [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Trond Myklebust
@ 2016-09-20 16:55 ` Trond Myklebust
  2016-09-20 16:55   ` [PATCH v6 02/29] NFS: Fix inode corruption in nfs_prime_dcache() Trond Myklebust
  2016-09-20 22:06 ` [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Oleg Drokin
  1 sibling, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 02/29] NFS: Fix inode corruption in nfs_prime_dcache()
  2016-09-20 16:55 ` [PATCH v6 01/29] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
@ 2016-09-20 16:55   ` Trond Myklebust
  2016-09-20 16:55     ` [PATCH v6 03/29] NFSv4: Don't report revoked delegations as valid in nfs_have_delegation() Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

Due to inode number reuse in filesystems, we can end up corrupting the
inode on our client if we apply the file attributes without ensuring that
the filehandle matches.
Typical symptoms include spurious "mode changed" reports in the syslog.

We still do want to ensure that we don't invalidate the dentry if the
inode number matches, but we don't have a filehandle.

Fixes: fa9233699cc1 ("NFS: Don't require a filehandle to refresh...")
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: stable@vger.kernel.org # v4.0+
---
 fs/nfs/dir.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 177fefb26c18..578c8ce1aca1 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -435,11 +435,11 @@ int nfs_same_file(struct dentry *dentry, struct nfs_entry *entry)
 		return 0;
 
 	nfsi = NFS_I(inode);
-	if (entry->fattr->fileid == nfsi->fileid)
-		return 1;
-	if (nfs_compare_fh(entry->fh, &nfsi->fh) == 0)
-		return 1;
-	return 0;
+	if (entry->fattr->fileid != nfsi->fileid)
+		return 0;
+	if (entry->fh->size && nfs_compare_fh(entry->fh, &nfsi->fh) != 0)
+		return 0;
+	return 1;
 }
 
 static
@@ -517,6 +517,8 @@ again:
 					&entry->fattr->fsid))
 			goto out;
 		if (nfs_same_file(dentry, entry)) {
+			if (!entry->fh->size)
+				goto out;
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 			status = nfs_refresh_inode(d_inode(dentry), entry->fattr);
 			if (!status)
@@ -529,6 +531,8 @@ again:
 			goto again;
 		}
 	}
+	if (!entry->fh->size)
+		goto out;
 
 	inode = nfs_fhget(dentry->d_sb, entry->fh, entry->fattr, entry->label);
 	alias = d_splice_alias(inode, dentry);
-- 
2.7.4


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

* [PATCH v6 03/29] NFSv4: Don't report revoked delegations as valid in nfs_have_delegation()
  2016-09-20 16:55   ` [PATCH v6 02/29] NFS: Fix inode corruption in nfs_prime_dcache() Trond Myklebust
@ 2016-09-20 16:55     ` Trond Myklebust
  2016-09-20 16:55       ` [PATCH v6 04/29] NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is invalid Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If the delegation is revoked, then it can't be used for caching.

Fixes: 869f9dfa4d6d ("NFSv4: Fix races between nfs_remove_bad_delegation()...")
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: stable@vger.kernel.org # v3.19+
---
 fs/nfs/delegation.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 322c2585bc34..86d2c748140b 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -51,6 +51,7 @@ nfs4_do_check_delegation(struct inode *inode, fmode_t flags, bool mark)
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
 	if (delegation != NULL && (delegation->type & flags) == flags &&
+	    !test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
 	    !test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
 		if (mark)
 			nfs_mark_delegation_referenced(delegation);
-- 
2.7.4


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

* [PATCH v6 04/29] NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is invalid
  2016-09-20 16:55     ` [PATCH v6 03/29] NFSv4: Don't report revoked delegations as valid in nfs_have_delegation() Trond Myklebust
@ 2016-09-20 16:55       ` Trond Myklebust
  2016-09-20 16:55         ` [PATCH v6 05/29] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

We must not allow the use of delegations that have been revoked or are
being returned.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Fixes: 869f9dfa4d6d ("NFSv4: Fix races between nfs_remove_bad_delegation()...")
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: stable@vger.kernel.org # v3.19+
---
 fs/nfs/delegation.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 86d2c748140b..b9c65421ed81 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -41,6 +41,17 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation)
 	set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags);
 }
 
+static bool
+nfs4_is_valid_delegation(const struct nfs_delegation *delegation,
+		fmode_t flags)
+{
+	if (delegation != NULL && (delegation->type & flags) == flags &&
+	    !test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
+	    !test_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
+		return true;
+	return false;
+}
+
 static int
 nfs4_do_check_delegation(struct inode *inode, fmode_t flags, bool mark)
 {
@@ -50,9 +61,7 @@ nfs4_do_check_delegation(struct inode *inode, fmode_t flags, bool mark)
 	flags &= FMODE_READ|FMODE_WRITE;
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
-	if (delegation != NULL && (delegation->type & flags) == flags &&
-	    !test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
-	    !test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
+	if (nfs4_is_valid_delegation(delegation, flags)) {
 		if (mark)
 			nfs_mark_delegation_referenced(delegation);
 		ret = 1;
@@ -894,7 +903,7 @@ bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags,
 	flags &= FMODE_READ|FMODE_WRITE;
 	rcu_read_lock();
 	delegation = rcu_dereference(nfsi->delegation);
-	ret = (delegation != NULL && (delegation->type & flags) == flags);
+	ret = nfs4_is_valid_delegation(delegation, flags);
 	if (ret) {
 		nfs4_stateid_copy(dst, &delegation->stateid);
 		nfs_mark_delegation_referenced(delegation);
-- 
2.7.4


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

* [PATCH v6 05/29] NFSv4.1: Don't check delegations that are already marked as revoked
  2016-09-20 16:55       ` [PATCH v6 04/29] NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is invalid Trond Myklebust
@ 2016-09-20 16:55         ` Trond Myklebust
  2016-09-20 16:55           ` [PATCH v6 06/29] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 06/29] NFSv4.1: Allow test_stateid to handle session errors without waiting
  2016-09-20 16:55         ` [PATCH v6 05/29] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
@ 2016-09-20 16:55           ` Trond Myklebust
  2016-09-20 16:55             ` [PATCH v6 07/29] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 07/29] NFSv4.1: Add a helper function to deal with expired stateids
  2016-09-20 16:55           ` [PATCH v6 06/29] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
@ 2016-09-20 16:55             ` Trond Myklebust
  2016-09-20 16:55               ` [PATCH v6 08/29] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 08/29] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid
  2016-09-20 16:55             ` [PATCH v6 07/29] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
@ 2016-09-20 16:55               ` Trond Myklebust
  2016-09-20 16:55                 ` [PATCH v6 09/29] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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 b9c65421ed81..e5212e5c73d2 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -652,23 +652,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] 34+ messages in thread

* [PATCH v6 09/29] NFSv4.1: Test delegation stateids when server declares "some state revoked"
  2016-09-20 16:55               ` [PATCH v6 08/29] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
@ 2016-09-20 16:55                 ` Trond Myklebust
  2016-09-20 16:55                   ` [PATCH v6 10/29] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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 e5212e5c73d2..dfb300901f7e 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -812,8 +812,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);
+	}
 }
 
 /**
@@ -877,6 +884,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] 34+ messages in thread

* [PATCH v6 10/29] NFSv4.1: Deal with server reboots during delegation expiration recovery
  2016-09-20 16:55                 ` [PATCH v6 09/29] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
@ 2016-09-20 16:55                   ` Trond Myklebust
  2016-09-20 16:55                     ` [PATCH v6 11/29] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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 dfb300901f7e..0ffead281555 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -884,6 +884,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)
 {
@@ -892,6 +899,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;
@@ -964,6 +984,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] 34+ messages in thread

* [PATCH v6 11/29] NFSv4.1: Don't recheck delegations that have already been checked
  2016-09-20 16:55                   ` [PATCH v6 10/29] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
@ 2016-09-20 16:55                     ` Trond Myklebust
  2016-09-20 16:55                       ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
  2016-09-20 16:55                     ` [PATCH v6 11/29] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
@ 2016-09-20 16:55                       ` Trond Myklebust
  2016-09-20 16:55                         ` [PATCH v6 13/29] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
  2016-09-21 16:03                         ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Anna Schumaker
  0 siblings, 2 replies; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 13/29] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
  2016-09-20 16:55                       ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
@ 2016-09-20 16:55                         ` Trond Myklebust
  2016-09-20 16:55                           ` [PATCH v6 14/29] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
  2016-09-21 16:03                         ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Anna Schumaker
  1 sibling, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 14/29] NFSv4.1: FREE_STATEID can be asynchronous
  2016-09-20 16:55                         ` [PATCH v6 13/29] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
@ 2016-09-20 16:55                           ` Trond Myklebust
  2016-09-20 16:55                             ` [PATCH v6 15/29] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 15/29] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku
  2016-09-20 16:55                           ` [PATCH v6 14/29] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
@ 2016-09-20 16:55                             ` Trond Myklebust
  2016-09-20 16:55                               ` [PATCH v6 16/29] NFSv4: Ensure we don't re-test revoked and freed stateids Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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] 34+ messages in thread

* [PATCH v6 16/29] NFSv4: Ensure we don't re-test revoked and freed stateids
  2016-09-20 16:55                             ` [PATCH v6 15/29] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
@ 2016-09-20 16:55                               ` Trond Myklebust
  2016-09-20 16:55                                 ` [PATCH v6 17/29] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

This fixes a potential infinite loop in nfs_reap_expired_delegations.

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

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 0ffead281555..484f14700108 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -656,6 +656,7 @@ static void nfs_mark_delegation_revoked(struct nfs_server *server,
 		struct nfs_delegation *delegation)
 {
 	set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
+	delegation->stateid.type = NFS4_INVALID_STATEID_TYPE;
 	nfs_mark_return_delegation(server, delegation);
 }
 
@@ -894,6 +895,8 @@ static inline bool nfs4_server_rebooted(const struct nfs_client *clp)
 static void nfs_mark_test_expired_delegation(struct nfs_server *server,
 	    struct nfs_delegation *delegation)
 {
+	if (delegation->stateid.type == NFS4_INVALID_STATEID_TYPE)
+		return;
 	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);
-- 
2.7.4


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

* [PATCH v6 17/29] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids
  2016-09-20 16:55                               ` [PATCH v6 16/29] NFSv4: Ensure we don't re-test revoked and freed stateids Trond Myklebust
@ 2016-09-20 16:55                                 ` Trond Myklebust
  2016-09-20 16:56                                   ` [PATCH v6 18/29] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:55 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 484f14700108..7a0bf3d439ca 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -1001,6 +1001,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] 34+ messages in thread

* [PATCH v6 18/29] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case
  2016-09-20 16:55                                 ` [PATCH v6 17/29] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
@ 2016-09-20 16:56                                   ` Trond Myklebust
  2016-09-20 16:56                                     ` [PATCH v6 19/29] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 f79d4f55d83f..75dc389c9719 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] 34+ messages in thread

* [PATCH v6 19/29] NFSv4: nfs4_handle_setlk_error() handle expiration as revoke case
  2016-09-20 16:56                                   ` [PATCH v6 18/29] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
@ 2016-09-20 16:56                                     ` Trond Myklebust
  2016-09-20 16:56                                       ` [PATCH v6 20/29] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 75dc389c9719..f0c5442401d7 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] 34+ messages in thread

* [PATCH v6 20/29] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state
  2016-09-20 16:56                                     ` [PATCH v6 19/29] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
@ 2016-09-20 16:56                                       ` Trond Myklebust
  2016-09-20 16:56                                         ` [PATCH v6 21/29] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 f0c5442401d7..e5b35e57a8ff 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] 34+ messages in thread

* [PATCH v6 21/29] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb
  2016-09-20 16:56                                       ` [PATCH v6 20/29] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
@ 2016-09-20 16:56                                         ` Trond Myklebust
  2016-09-20 16:56                                           ` [PATCH v6 22/29] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 e5b35e57a8ff..1ddfb300addf 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] 34+ messages in thread

* [PATCH v6 22/29] NFSv4: Fix a race in nfs_inode_reclaim_delegation()
  2016-09-20 16:56                                         ` [PATCH v6 21/29] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
@ 2016-09-20 16:56                                           ` Trond Myklebust
  2016-09-20 16:56                                             ` [PATCH v6 23/29] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 7a0bf3d439ca..2a441ad4e226 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -195,15 +195,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] 34+ messages in thread

* [PATCH v6 23/29] NFSv4: Fix a race when updating an open_stateid
  2016-09-20 16:56                                           ` [PATCH v6 22/29] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
@ 2016-09-20 16:56                                             ` Trond Myklebust
  2016-09-20 16:56                                               ` [PATCH v6 24/29] NFS: Always call nfs_inode_find_state_and_recover() when revoking a delegation Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 1ddfb300addf..338684195e95 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] 34+ messages in thread

* [PATCH v6 24/29] NFS: Always call nfs_inode_find_state_and_recover() when revoking a delegation
  2016-09-20 16:56                                             ` [PATCH v6 23/29] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
@ 2016-09-20 16:56                                               ` Trond Myklebust
  2016-09-20 16:56                                                 ` [PATCH v6 25/29] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

Don't rely on nfs_inode_detach_delegation() succeeding. That can race...

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

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 2a441ad4e226..0073848b5ad3 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -662,18 +662,24 @@ static bool nfs_revoke_delegation(struct inode *inode,
 		const nfs4_stateid *stateid)
 {
 	struct nfs_delegation *delegation;
+	nfs4_stateid tmp;
 	bool ret = false;
 
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
 	if (delegation == NULL)
 		goto out;
-	if (stateid && !nfs4_stateid_match(stateid, &delegation->stateid))
+	if (stateid == NULL) {
+		nfs4_stateid_copy(&tmp, &delegation->stateid);
+		stateid = &tmp;
+	} else if (!nfs4_stateid_match(stateid, &delegation->stateid))
 		goto out;
 	nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
 	ret = true;
 out:
 	rcu_read_unlock();
+	if (ret)
+		nfs_inode_find_state_and_recover(inode, stateid);
 	return ret;
 }
 
@@ -685,10 +691,8 @@ void nfs_remove_bad_delegation(struct inode *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);
+	if (delegation)
 		nfs_free_delegation(delegation);
-	}
 }
 EXPORT_SYMBOL_GPL(nfs_remove_bad_delegation);
 
-- 
2.7.4


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

* [PATCH v6 25/29] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid
  2016-09-20 16:56                                               ` [PATCH v6 24/29] NFS: Always call nfs_inode_find_state_and_recover() when revoking a delegation Trond Myklebust
@ 2016-09-20 16:56                                                 ` Trond Myklebust
  2016-09-20 16:56                                                   ` [PATCH v6 26/29] NFSv4: Don't test open_stateid unless it is set Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 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 | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 338684195e95..552bea2af811 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -397,13 +397,23 @@ static int nfs4_do_handle_exception(struct nfs_server *server,
 	exception->delay = 0;
 	exception->recovering = 0;
 	exception->retry = 0;
+
+	if (stateid == NULL && state != NULL)
+		stateid = &state->stateid;
+
 	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_state_and_recover(inode,
+						stateid);
+				goto wait_on_recovery;
+			}
+		case -NFS4ERR_OPENMODE:
 			if (inode) {
 				int err;
 
@@ -422,12 +432,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] 34+ messages in thread

* [PATCH v6 26/29] NFSv4: Don't test open_stateid unless it is set
  2016-09-20 16:56                                                 ` [PATCH v6 25/29] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
@ 2016-09-20 16:56                                                   ` Trond Myklebust
  2016-09-20 16:56                                                     ` [PATCH v6 27/29] NFSv4: Mark the lock and open stateids as invalid after freeing them Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

We need to test the NFS_OPEN_STATE flag for whether or not the
open_stateid is valid.

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 552bea2af811..995b646c9aea 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2587,6 +2587,11 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
 	struct rpc_cred *cred = state->owner->so_cred;
 	int status;
 
+	if (test_bit(NFS_OPEN_STATE, &state->flags) == 0) {
+		if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
+			return NFS_OK;
+		return -NFS4ERR_BAD_STATEID;
+	}
 	/* If a state reset has been done, test_stateid is unneeded */
 	if ((test_bit(NFS_O_RDONLY_STATE, &state->flags) == 0) &&
 	    (test_bit(NFS_O_WRONLY_STATE, &state->flags) == 0) &&
-- 
2.7.4


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

* [PATCH v6 27/29] NFSv4: Mark the lock and open stateids as invalid after freeing them
  2016-09-20 16:56                                                   ` [PATCH v6 26/29] NFSv4: Don't test open_stateid unless it is set Trond Myklebust
@ 2016-09-20 16:56                                                     ` Trond Myklebust
  2016-09-20 16:56                                                       ` [PATCH v6 28/29] NFSv4: Open state recovery must account for file permission changes Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 995b646c9aea..2e1f9c30b805 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2560,6 +2560,7 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
 			if (status == -NFS4ERR_EXPIRED ||
 			    status == -NFS4ERR_BAD_STATEID) {
 				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
+				lsp->ls_stateid.type = NFS4_INVALID_STATEID_TYPE;
 				if (!recover_lost_locks)
 					set_bit(NFS_LOCK_LOST, &lsp->ls_flags);
 			} else if (status != NFS_OK) {
@@ -2605,6 +2606,7 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
 		clear_bit(NFS_O_WRONLY_STATE, &state->flags);
 		clear_bit(NFS_O_RDWR_STATE, &state->flags);
 		clear_bit(NFS_OPEN_STATE, &state->flags);
+		stateid->type = NFS4_INVALID_STATEID_TYPE;
 	}
 	return status;
 }
-- 
2.7.4


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

* [PATCH v6 28/29] NFSv4: Open state recovery must account for file permission changes
  2016-09-20 16:56                                                     ` [PATCH v6 27/29] NFSv4: Mark the lock and open stateids as invalid after freeing them Trond Myklebust
@ 2016-09-20 16:56                                                       ` Trond Myklebust
  2016-09-20 16:56                                                         ` [PATCH v6 29/29] NFSv4: Fix retry issues with nfs41_test/free_stateid Trond Myklebust
  0 siblings, 1 reply; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

If the file permissions change on the server, then we may not be able to
recover open state. If so, we need to ensure that we mark the file
descriptor appropriately.

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

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 609643f31e7a..79366b69ae34 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1532,6 +1532,9 @@ restart:
 					__func__, status);
 			case -ENOENT:
 			case -ENOMEM:
+			case -EACCES:
+			case -EROFS:
+			case -EIO:
 			case -ESTALE:
 				/* Open state on this file cannot be recovered */
 				nfs4_state_mark_recovery_failed(state, status);
-- 
2.7.4


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

* [PATCH v6 29/29] NFSv4: Fix retry issues with nfs41_test/free_stateid
  2016-09-20 16:56                                                       ` [PATCH v6 28/29] NFSv4: Open state recovery must account for file permission changes Trond Myklebust
@ 2016-09-20 16:56                                                         ` Trond Myklebust
  0 siblings, 0 replies; 34+ messages in thread
From: Trond Myklebust @ 2016-09-20 16:56 UTC (permalink / raw)
  To: anna.schumaker; +Cc: linux-nfs, Oleg Drokin

_nfs41_free_stateid() needs to be cached by the session, but
nfs41_test_stateid() may return NFS4ERR_RETRY_UNCACHED_REP (in which
case we should just retry).

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 2e1f9c30b805..72858f900abf 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8730,6 +8730,7 @@ static void nfs4_handle_delay_or_session_error(struct nfs_server *server,
 	exception->retry = 0;
 	switch(err) {
 	case -NFS4ERR_DELAY:
+	case -NFS4ERR_RETRY_UNCACHED_REP:
 		nfs4_handle_exception(server, err, exception);
 		break;
 	case -NFS4ERR_BADSESSION:
@@ -8835,7 +8836,7 @@ static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server,
 
 	msg.rpc_argp = &data->args;
 	msg.rpc_resp = &data->res;
-	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 0);
+	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
 	if (privileged)
 		nfs4_set_sequence_privileged(&data->args.seq_args);
 
-- 
2.7.4


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

* Re: [PATCH v6 00/29] Fix delegation behaviour when server revokes some state
  2016-09-20 16:55 [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Trond Myklebust
  2016-09-20 16:55 ` [PATCH v6 01/29] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
@ 2016-09-20 22:06 ` Oleg Drokin
       [not found]   ` <104E1824-0235-41DF-AA9D-5C3F5560CA57@primarydata.com>
  1 sibling, 1 reply; 34+ messages in thread
From: Oleg Drokin @ 2016-09-20 22:06 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: anna.schumaker, linux-nfs


On Sep 20, 2016, at 12:55 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
> v5: Report revoked delegations as invalid in nfs_have_delegation()
>    Fix an infinite loop in nfs_reap_expired_delegations.
>        Fixes for other looping behaviour
> v6: Fix nfs4_do_handle_exception to handle all stateids, not just delegations
>    Stable fix for nfs4_copy_delegation_stateid
>    Marked fix "NFSv4: Don't report revoked delegations as valid in
>        nfs_have_delegation" for stable.
>    Stable fix for the inode mode/fileid corruption
> 
> Trond Myklebust (29):
>  NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
>  NFS: Fix inode corruption in nfs_prime_dcache()
>  NFSv4: Don't report revoked delegations as valid in
>    nfs_have_delegation()
>  NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is
>    invalid
>  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: Ensure we don't re-test revoked and freed stateids
>  NFSv4: nfs_inode_find_delegation_state_and_recover() should check all
>    stateids
>  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
>  NFS: Always call nfs_inode_find_state_and_recover() when revoking a
>    delegation
>  NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single
>    stateid
>  NFSv4: Don't test open_stateid unless it is set
>  NFSv4: Mark the lock and open stateids as invalid after freeing them
>  NFSv4: Open state recovery must account for file permission changes
>  NFSv4: Fix retry issues with nfs41_test/free_stateid

This one seems to fail in multiple ways.
This is applied on top of Linus' tree commit d2ffb0103aaefa9b169da042cf39ce27bfb6cdbb
One is similar to what we saw before:

[12374.572987] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
[12374.572988] --> nfs41_setup_sequence
[12374.572989] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[12374.572990] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[12374.572991] <-- nfs41_setup_sequence slotid=0 seqid=3873200
[12374.572998] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873200 slotid=0 max_slotid=0 cache_this=1
[12374.573228] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[12374.573229] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[12374.573230] nfs4_free_slot: slotid 1 highest_used_slotid 0
[12374.573231] nfs41_sequence_process: Error 0 free the slot 
[12374.573232] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[12374.573252] --> nfs_put_client({2})
[12374.573257] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
[12374.573258] --> nfs41_setup_sequence
[12374.573259] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[12374.573260] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[12374.573261] <-- nfs41_setup_sequence slotid=0 seqid=3873201
[12374.573268] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873201 slotid=0 max_slotid=0 cache_this=1
[12374.573525] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[12374.573526] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[12374.573527] nfs4_free_slot: slotid 1 highest_used_slotid 0
[12374.573527] nfs41_sequence_process: Error 0 free the slot 
[12374.573529] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[12374.573548] --> nfs_put_client({2})
[12374.573554] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
[12374.573555] --> nfs41_setup_sequence
[12374.573556] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[12374.573557] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[12374.573558] <-- nfs41_setup_sequence slotid=0 seqid=3873202
[12374.573565] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873202 slotid=0 max_slotid=0 cache_this=1
[12374.573794] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[12374.573795] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[12374.573796] nfs4_free_slot: slotid 1 highest_used_slotid 0
[12374.573797] nfs41_sequence_process: Error 0 free the slot 
[12374.573798] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[12374.573818] --> nfs_put_client({2})
[12374.573823] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
[12374.573824] --> nfs41_setup_sequence
[12374.573825] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
[12374.573826] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
[12374.573827] <-- nfs41_setup_sequence slotid=0 seqid=3873203
[12374.573835] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873203 slotid=0 max_slotid=0 cache_this=1
[12374.574103] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
[12374.574104] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
[12374.574105] nfs4_free_slot: slotid 1 highest_used_slotid 0
[12374.574106] nfs41_sequence_process: Error 0 free the slot 
[12374.574108] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
[12374.574128] --> nfs_put_client({2})

Then there are crashes/warnings, I have 4 starting with same warning, here's a sample:
[ 7338.195888] ------------[ cut here ]------------
[ 7338.210606] WARNING: CPU: 5 PID: 2310872 at /home/green/bk/linux-test/fs/dcache.c:768 dput+0x31f/0x410
[ 7338.211548] Modules linked in: loop rpcsec_gss_krb5 acpi_cpufreq tpm_tis tpm_tis_core joydev tpm i2c_piix4 pcspkr virtio_console nfsd ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm serio_raw floppy
[ 7338.213209] CPU: 5 PID: 2310872 Comm: ls Not tainted 4.8.0-rc7-vm-nfst+ #38
[ 7338.213707] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150714_191134- 04/01/2014
[ 7338.214614]  0000000000000286 0000000073f54649 ffff8800ab0039b0 ffffffff814ad883
[ 7338.215510]  0000000000000000 0000000000000000 ffff8800ab0039f0 ffffffff8108a0ab
[ 7338.217487]  000003005b1bdb20 ffff880060b2d090 ffffffff81c6e4f8 ffffffff81291fa7
[ 7338.218415] Call Trace:
[ 7338.219158]  [<ffffffff814ad883>] dump_stack+0x86/0xc3
[ 7338.219608]  [<ffffffff8108a0ab>] __warn+0xcb/0xf0
[ 7338.220109]  [<ffffffff81291fa7>] ? dput+0x37/0x410
[ 7338.220558]  [<ffffffff8108a1dd>] warn_slowpath_null+0x1d/0x20
[ 7338.230497]  [<ffffffff8129228f>] dput+0x31f/0x410
[ 7338.230961]  [<ffffffff81291fa7>] ? dput+0x37/0x410
[ 7338.231450]  [<ffffffff81294bbb>] ? d_lookup+0x9b/0xe0
[ 7338.231913]  [<ffffffff8138875b>] nfs_prime_dcache+0x16b/0x330
[ 7338.232416]  [<ffffffff81388d22>] nfs_readdir_page_filler+0x222/0x430
[ 7338.232903]  [<ffffffff813891f2>] nfs_readdir_xdr_to_array+0x2c2/0x450
[ 7338.233423]  [<ffffffff81108c08>] ? rcu_read_lock_sched_held+0x8/0x80
[ 7338.233914]  [<ffffffff813893a0>] nfs_readdir_filler+0x20/0x90
[ 7338.234424]  [<ffffffff811d088c>] do_read_cache_page+0x15c/0x2d0
[ 7338.234908]  [<ffffffff810db0c5>] ? wake_up_bit+0x25/0x30
[ 7338.235754]  [<ffffffff81389380>] ? nfs_readdir_xdr_to_array+0x450/0x450
[ 7338.236238]  [<ffffffff811d0a1c>] read_cache_page+0x1c/0x20
[ 7338.236746]  [<ffffffff81389f52>] nfs_readdir+0x182/0x7e0
[ 7338.237205]  [<ffffffff8104c1a5>] ? kvm_sched_clock_read+0x25/0x40
[ 7338.237736]  [<ffffffff8101fb79>] ? sched_clock+0x9/0x10
[ 7338.238205]  [<ffffffff813c77d0>] ? nfs4_xdr_dec_lookup_root+0xb0/0xb0
[ 7338.238728]  [<ffffffff8128c9e1>] iterate_dir+0x181/0x1b0
[ 7338.239191]  [<ffffffff8128cefc>] SyS_getdents+0x9c/0x130
[ 7338.240175]  [<ffffffff8128cc60>] ? fillonedir+0x100/0x100
[ 7338.240694]  [<ffffffff8189f03c>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[ 7338.241216] ---[ end trace 650001f792293ddc ]---
[ 7338.241712] ------------[ cut here ]------------
[ 7338.377788] WARNING: CPU: 6 PID: 2310872 at /home/green/bk/linux-test/fs/dcache.c:304 dentry_free+0x7d/0x80
[ 7338.377801] Modules linked in: loop rpcsec_gss_krb5 acpi_cpufreq tpm_tis tpm_tis_core joydev tpm i2c_piix4 pcspkr virtio_console nfsd ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm serio_raw floppy
[ 7338.377803] CPU: 6 PID: 2310872 Comm: ls Tainted: G        W       4.8.0-rc7-vm-nfst+ #38
[ 7338.377804] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150714_191134- 04/01/2014
[ 7338.377807]  0000000000000286 0000000073f54649 ffff8800ab003970 ffffffff814ad883
[ 7338.377809]  0000000000000000 0000000000000000 ffff8800ab0039b0 ffffffff8108a0ab
[ 7338.377811]  0000013000000246 ffff880060b2d000 ffff88000d822000 ffff880060b2d090
[ 7338.377811] Call Trace:
[ 7338.377815]  [<ffffffff814ad883>] dump_stack+0x86/0xc3
[ 7338.377817]  [<ffffffff8108a0ab>] __warn+0xcb/0xf0
[ 7338.377819]  [<ffffffff8108a1dd>] warn_slowpath_null+0x1d/0x20
[ 7338.377821]  [<ffffffff8128fbad>] dentry_free+0x7d/0x80
[ 7338.377822]  [<ffffffff812902cd>] __dentry_kill+0x10d/0x160
[ 7338.377824]  [<ffffffff81291fa7>] ? dput+0x37/0x410
[ 7338.377825]  [<ffffffff81292259>] dput+0x2e9/0x410
[ 7338.377826]  [<ffffffff81291fa7>] ? dput+0x37/0x410
[ 7338.377830]  [<ffffffff8138875b>] nfs_prime_dcache+0x16b/0x330
[ 7338.377833]  [<ffffffff81388d22>] nfs_readdir_page_filler+0x222/0x430
[ 7338.377835]  [<ffffffff813891f2>] nfs_readdir_xdr_to_array+0x2c2/0x450
[ 7338.377839]  [<ffffffff81108c08>] ? rcu_read_lock_sched_held+0x8/0x80
[ 7338.377841]  [<ffffffff813893a0>] nfs_readdir_filler+0x20/0x90
[ 7338.377843]  [<ffffffff811d088c>] do_read_cache_page+0x15c/0x2d0
[ 7338.377845]  [<ffffffff810db0c5>] ? wake_up_bit+0x25/0x30
[ 7338.377847]  [<ffffffff81389380>] ? nfs_readdir_xdr_to_array+0x450/0x450
[ 7338.377849]  [<ffffffff811d0a1c>] read_cache_page+0x1c/0x20
[ 7338.377851]  [<ffffffff81389f52>] nfs_readdir+0x182/0x7e0
[ 7338.377853]  [<ffffffff8104c1a5>] ? kvm_sched_clock_read+0x25/0x40
[ 7338.377855]  [<ffffffff8101fb79>] ? sched_clock+0x9/0x10
[ 7338.377857]  [<ffffffff813c77d0>] ? nfs4_xdr_dec_lookup_root+0xb0/0xb0
[ 7338.377859]  [<ffffffff8128c9e1>] iterate_dir+0x181/0x1b0
[ 7338.377860]  [<ffffffff8128cefc>] SyS_getdents+0x9c/0x130
[ 7338.377862]  [<ffffffff8128cc60>] ? fillonedir+0x100/0x100
[ 7338.377865]  [<ffffffff8189f03c>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[ 7338.377867] ---[ end trace 650001f792293ddd ]---
[ 7338.439443] BUG: spinlock bad magic on CPU#1, ls/2310745
[ 7338.439452] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 7338.439461] Modules linked in: loop rpcsec_gss_krb5 acpi_cpufreq tpm_tis tpm_tis_core joydev tpm i2c_piix4 pcspkr virtio_console nfsd ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm serio_raw floppy
[ 7338.439463] CPU: 1 PID: 2310745 Comm: ls Tainted: G        W       4.8.0-rc7-vm-nfst+ #38
[ 7338.439464] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150714_191134- 04/01/2014
[ 7338.439465] task: ffff8800abc28880 task.stack: ffff88005d39c000
[ 7338.439470] RIP: 0010:[<ffffffff810eede1>]  [<ffffffff810eede1>] spin_dump+0x51/0xd0
[ 7338.439471] RSP: 0018:ffff88005d39f930  EFLAGS: 00010202
[ 7338.439472] RAX: 000000000000002c RBX: 6b6b6b6b6b6b6b6b RCX: ffff8800abc28880
[ 7338.439472] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000296
[ 7338.439473] RBP: ffff88005d39f940 R08: 0000000000000001 R09: 000000006b6b6b6b
[ 7338.439474] R10: 0000000000000001 R11: 00000000000338e4 R12: ffff880060b2d090
[ 7338.439475] R13: 00000000e430028f R14: ffff880060b2d130 R15: ffff880060b2d090
[ 7338.439476] FS:  00007fbeab953800(0000) GS:ffff8800b8400000(0000) knlGS:0000000000000000
[ 7338.439477] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7338.439478] CR2: 00007f7e27b90008 CR3: 000000005df9a000 CR4: 00000000000006e0
[ 7338.439481] Stack:
[ 7338.439483]  ffff880060b2d090 0000000000000000 ffff88005d39f958 ffffffff810ef093
[ 7338.439485]  ffff880060b2d090 ffff88005d39f970 ffffffff8189e6d7 ffff880060b2d090
[ 7338.439487]  ffff88005d39f990 ffffffff814bd1cd ffff88000d822000 ffffffff833a1478
[ 7338.439487] Call Trace:
[ 7338.439491]  [<ffffffff810ef093>] do_raw_spin_unlock+0x63/0xc0
[ 7338.439494]  [<ffffffff8189e6d7>] _raw_spin_unlock+0x27/0x40
[ 7338.439496]  [<ffffffff814bd1cd>] lockref_get_not_dead+0x3d/0x50
[ 7338.439499]  [<ffffffff812941de>] d_alloc_parallel+0x27e/0x950
[ 7338.439502]  [<ffffffff813887e6>] ? nfs_prime_dcache+0x1f6/0x330
[ 7338.439504]  [<ffffffff81294bbb>] ? d_lookup+0x9b/0xe0
[ 7338.439506]  [<ffffffff813887e6>] nfs_prime_dcache+0x1f6/0x330
[ 7338.439508]  [<ffffffff81388d22>] nfs_readdir_page_filler+0x222/0x430
[ 7338.439511]  [<ffffffff813891f2>] nfs_readdir_xdr_to_array+0x2c2/0x450
[ 7338.439514]  [<ffffffff81108c08>] ? rcu_read_lock_sched_held+0x8/0x80
[ 7338.439516]  [<ffffffff813893a0>] nfs_readdir_filler+0x20/0x90
[ 7338.439518]  [<ffffffff811d088c>] do_read_cache_page+0x15c/0x2d0
[ 7338.439520]  [<ffffffff810db0c5>] ? wake_up_bit+0x25/0x30
[ 7338.439522]  [<ffffffff81389380>] ? nfs_readdir_xdr_to_array+0x450/0x450
[ 7338.439523]  [<ffffffff811d0a1c>] read_cache_page+0x1c/0x20
[ 7338.439525]  [<ffffffff81389f52>] nfs_readdir+0x182/0x7e0
[ 7338.439528]  [<ffffffff8104c1a5>] ? kvm_sched_clock_read+0x25/0x40
[ 7338.439530]  [<ffffffff8101fb79>] ? sched_clock+0x9/0x10
[ 7338.439533]  [<ffffffff813c77d0>] ? nfs4_xdr_dec_lookup_root+0xb0/0xb0
[ 7338.439534]  [<ffffffff8128c9e1>] iterate_dir+0x181/0x1b0
[ 7338.439536]  [<ffffffff8128cefc>] SyS_getdents+0x9c/0x130
[ 7338.439537]  [<ffffffff8128cc60>] ? fillonedir+0x100/0x100
[ 7338.439539]  [<ffffffff8189f03c>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[ 7338.439560] Code: 44 8b 80 80 08 00 00 48 8d 88 b8 0a 00 00 48 c7 c7 a8 a5 c4 81 65 8b 15 56 b3 f1 7e e8 75 e4 0d 00 48 85 db 45 8b 4c 24 08 74 6a <44> 8b 83 80 08 00 00 48 8d 8b b8 0a 00 00 41 8b 54 24 04 4c 89 
[ 7338.439562] RIP  [<ffffffff810eede1>] spin_dump+0x51/0xd0
[ 7338.439563]  RSP <ffff88005d39f930>


dcache.c:768 is:
        WARN_ON(d_in_lookup(dentry));

And two softlockups that start with the same warning, so I guess they are the same.



> 
> fs/nfs/delegation.c                    | 212 ++++++++++++++++--
> fs/nfs/delegation.h                    |   8 +-
> fs/nfs/dir.c                           |  14 +-
> fs/nfs/flexfilelayout/flexfilelayout.c |   2 +-
> fs/nfs/nfs4_fs.h                       |   5 +-
> fs/nfs/nfs4proc.c                      | 378 +++++++++++++++++++++++----------
> fs/nfs/nfs4session.h                   |   1 +
> fs/nfs/nfs4state.c                     |  76 +++++--
> include/linux/nfs4.h                   |   1 +
> 9 files changed, 538 insertions(+), 159 deletions(-)
> 
> -- 
> 2.7.4


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

* Re: [PATCH v6 00/29] Fix delegation behaviour when server revokes some state
       [not found]   ` <104E1824-0235-41DF-AA9D-5C3F5560CA57@primarydata.com>
@ 2016-09-21  1:07     ` Oleg Drokin
  0 siblings, 0 replies; 34+ messages in thread
From: Oleg Drokin @ 2016-09-21  1:07 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Schumaker Anna, List Linux NFS Mailing


On Sep 20, 2016, at 8:57 PM, Trond Myklebust wrote:

> 
>> On Sep 20, 2016, at 18:06, Oleg Drokin <green@linuxhacker.ru> wrote:
>> 
>> 
>> On Sep 20, 2016, at 12:55 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
>>> v5: Report revoked delegations as invalid in nfs_have_delegation()
>>>   Fix an infinite loop in nfs_reap_expired_delegations.
>>>       Fixes for other looping behaviour
>>> v6: Fix nfs4_do_handle_exception to handle all stateids, not just delegations
>>>   Stable fix for nfs4_copy_delegation_stateid
>>>   Marked fix "NFSv4: Don't report revoked delegations as valid in
>>>       nfs_have_delegation" for stable.
>>>   Stable fix for the inode mode/fileid corruption
>>> 
>>> Trond Myklebust (29):
>>> NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
>>> NFS: Fix inode corruption in nfs_prime_dcache()
>>> NFSv4: Don't report revoked delegations as valid in
>>>   nfs_have_delegation()
>>> NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is
>>>   invalid
>>> 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: Ensure we don't re-test revoked and freed stateids
>>> NFSv4: nfs_inode_find_delegation_state_and_recover() should check all
>>>   stateids
>>> 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
>>> NFS: Always call nfs_inode_find_state_and_recover() when revoking a
>>>   delegation
>>> NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single
>>>   stateid
>>> NFSv4: Don't test open_stateid unless it is set
>>> NFSv4: Mark the lock and open stateids as invalid after freeing them
>>> NFSv4: Open state recovery must account for file permission changes
>>> NFSv4: Fix retry issues with nfs41_test/free_stateid
>> 
>> This one seems to fail in multiple ways.
>> This is applied on top of Linus' tree commit d2ffb0103aaefa9b169da042cf39ce27bfb6cdbb
>> One is similar to what we saw before:
>> 
>> [12374.572987] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
>> [12374.572988] --> nfs41_setup_sequence
>> [12374.572989] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
>> [12374.572990] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
>> [12374.572991] <-- nfs41_setup_sequence slotid=0 seqid=3873200
>> [12374.572998] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873200 slotid=0 max_slotid=0 cache_this=1
>> [12374.573228] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
>> [12374.573229] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
>> [12374.573230] nfs4_free_slot: slotid 1 highest_used_slotid 0
>> [12374.573231] nfs41_sequence_process: Error 0 free the slot 
>> [12374.573232] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
>> [12374.573252] --> nfs_put_client({2})
>> [12374.573257] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
>> [12374.573258] --> nfs41_setup_sequence
>> [12374.573259] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
>> [12374.573260] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
>> [12374.573261] <-- nfs41_setup_sequence slotid=0 seqid=3873201
>> [12374.573268] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873201 slotid=0 max_slotid=0 cache_this=1
>> [12374.573525] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
>> [12374.573526] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
>> [12374.573527] nfs4_free_slot: slotid 1 highest_used_slotid 0
>> [12374.573527] nfs41_sequence_process: Error 0 free the slot 
>> [12374.573529] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
>> [12374.573548] --> nfs_put_client({2})
>> [12374.573554] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
>> [12374.573555] --> nfs41_setup_sequence
>> [12374.573556] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
>> [12374.573557] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
>> [12374.573558] <-- nfs41_setup_sequence slotid=0 seqid=3873202
>> [12374.573565] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873202 slotid=0 max_slotid=0 cache_this=1
>> [12374.573794] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
>> [12374.573795] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
>> [12374.573796] nfs4_free_slot: slotid 1 highest_used_slotid 0
>> [12374.573797] nfs41_sequence_process: Error 0 free the slot 
>> [12374.573798] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
>> [12374.573818] --> nfs_put_client({2})
>> [12374.573823] --> nfs41_call_sync_prepare data->seq_server ffff8800af6e3000
>> [12374.573824] --> nfs41_setup_sequence
>> [12374.573825] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31
>> [12374.573826] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
>> [12374.573827] <-- nfs41_setup_sequence slotid=0 seqid=3873203
>> [12374.573835] encode_sequence: sessionid=1474402413:3:4:0 seqid=3873203 slotid=0 max_slotid=0 cache_this=1
>> [12374.574103] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31
>> [12374.574104] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1
>> [12374.574105] nfs4_free_slot: slotid 1 highest_used_slotid 0
>> [12374.574106] nfs41_sequence_process: Error 0 free the slot 
>> [12374.574108] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295
>> [12374.574128] --> nfs_put_client({2})
> 
> Still not reproducing this. I’ve been trying for days… :-/

Well, I think I shared all my instructions.
I can show you around the system virtually if you want to explore it in case I missed
something.
I guess I can also fetch whatever data you think you might need with a debugger,
since all of that happens in a VM.
Come think of it, I can also force a crashdump and you can sift through it yourself
if you think this would work better for you.


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

* Re: [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
  2016-09-20 16:55                       ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
  2016-09-20 16:55                         ` [PATCH v6 13/29] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
@ 2016-09-21 16:03                         ` Anna Schumaker
  2016-09-21 16:39                           ` Trond Myklebust
  1 sibling, 1 reply; 34+ messages in thread
From: Anna Schumaker @ 2016-09-21 16:03 UTC (permalink / raw)
  To: Trond Myklebust, anna.schumaker; +Cc: linux-nfs, Oleg Drokin

Hi Trond,

On 09/20/2016 12:55 PM, Trond Myklebust wrote:
> 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.

Would you like to add a Signed-off-by to this patch? :)

Thanks,
Anna

> ---
>  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;
>  };
>  
> 


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

* Re: [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
  2016-09-21 16:03                         ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Anna Schumaker
@ 2016-09-21 16:39                           ` Trond Myklebust
  0 siblings, 0 replies; 34+ messages in thread
From: Trond Myklebust @ 2016-09-21 16:39 UTC (permalink / raw)
  To: Schumaker Anna; +Cc: Trond Myklebust, List Linux NFS Mailing, Oleg Drokin

DQo+IE9uIFNlcCAyMSwgMjAxNiwgYXQgMTI6MDMsIEFubmEgU2NodW1ha2VyIDxBbm5hLlNjaHVt
YWtlckBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+IEhpIFRyb25kLA0KPiANCj4gT24gMDkvMjAv
MjAxNiAxMjo1NSBQTSwgVHJvbmQgTXlrbGVidXN0IHdyb3RlOg0KPj4gSW4gc29tZSBjYXNlcyAo
ZS5nLiB3aGVuIHRoZSBTRVE0X1NUQVRVU19FWFBJUkVEX0FMTF9TVEFURV9SRVZPS0VEIHNlcXVl
bmNlDQo+PiBmbGFnIGlzIHNldCkgd2UgbWF5IGFscmVhZHkga25vdyB0aGF0IHRoZSBzdGF0ZWlk
IHdhcyByZXZva2VkIGFuZCB0aGF0IHRoZQ0KPj4gb25seSB2YWxpZCBvcGVyYXRpb24gd2UgY2Fu
IGNhbGwgaXMgRlJFRV9TVEFURUlELiBJbiB0aG9zZSBjYXNlcywgYWxsb3cNCj4+IHRoZSBzdGF0
ZWlkIHRvIGNhcnJ5IHRoZSBpbmZvcm1hdGlvbiBpbiB0aGUgdHlwZSBmaWVsZCwgc28gdGhhdCB3
ZSBza2lwDQo+PiB0aGUgcmVkdW5kYW50IGNhbGwgdG8gVEVTVF9TVEFURUlELg0KPiANCj4gV291
bGQgeW91IGxpa2UgdG8gYWRkIGEgU2lnbmVkLW9mZi1ieSB0byB0aGlzIHBhdGNoPyA6KQ0KDQpV
bW3igKYuIFllc+KApiBTb3JyeS4uLg0KDQo=


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

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

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 16:55 [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Trond Myklebust
2016-09-20 16:55 ` [PATCH v6 01/29] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
2016-09-20 16:55   ` [PATCH v6 02/29] NFS: Fix inode corruption in nfs_prime_dcache() Trond Myklebust
2016-09-20 16:55     ` [PATCH v6 03/29] NFSv4: Don't report revoked delegations as valid in nfs_have_delegation() Trond Myklebust
2016-09-20 16:55       ` [PATCH v6 04/29] NFSv4: nfs4_copy_delegation_stateid() must fail if the delegation is invalid Trond Myklebust
2016-09-20 16:55         ` [PATCH v6 05/29] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust
2016-09-20 16:55           ` [PATCH v6 06/29] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust
2016-09-20 16:55             ` [PATCH v6 07/29] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust
2016-09-20 16:55               ` [PATCH v6 08/29] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust
2016-09-20 16:55                 ` [PATCH v6 09/29] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust
2016-09-20 16:55                   ` [PATCH v6 10/29] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust
2016-09-20 16:55                     ` [PATCH v6 11/29] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust
2016-09-20 16:55                       ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust
2016-09-20 16:55                         ` [PATCH v6 13/29] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust
2016-09-20 16:55                           ` [PATCH v6 14/29] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust
2016-09-20 16:55                             ` [PATCH v6 15/29] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust
2016-09-20 16:55                               ` [PATCH v6 16/29] NFSv4: Ensure we don't re-test revoked and freed stateids Trond Myklebust
2016-09-20 16:55                                 ` [PATCH v6 17/29] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids Trond Myklebust
2016-09-20 16:56                                   ` [PATCH v6 18/29] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case Trond Myklebust
2016-09-20 16:56                                     ` [PATCH v6 19/29] NFSv4: nfs4_handle_setlk_error() " Trond Myklebust
2016-09-20 16:56                                       ` [PATCH v6 20/29] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state Trond Myklebust
2016-09-20 16:56                                         ` [PATCH v6 21/29] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb Trond Myklebust
2016-09-20 16:56                                           ` [PATCH v6 22/29] NFSv4: Fix a race in nfs_inode_reclaim_delegation() Trond Myklebust
2016-09-20 16:56                                             ` [PATCH v6 23/29] NFSv4: Fix a race when updating an open_stateid Trond Myklebust
2016-09-20 16:56                                               ` [PATCH v6 24/29] NFS: Always call nfs_inode_find_state_and_recover() when revoking a delegation Trond Myklebust
2016-09-20 16:56                                                 ` [PATCH v6 25/29] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid Trond Myklebust
2016-09-20 16:56                                                   ` [PATCH v6 26/29] NFSv4: Don't test open_stateid unless it is set Trond Myklebust
2016-09-20 16:56                                                     ` [PATCH v6 27/29] NFSv4: Mark the lock and open stateids as invalid after freeing them Trond Myklebust
2016-09-20 16:56                                                       ` [PATCH v6 28/29] NFSv4: Open state recovery must account for file permission changes Trond Myklebust
2016-09-20 16:56                                                         ` [PATCH v6 29/29] NFSv4: Fix retry issues with nfs41_test/free_stateid Trond Myklebust
2016-09-21 16:03                         ` [PATCH v6 12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Anna Schumaker
2016-09-21 16:39                           ` Trond Myklebust
2016-09-20 22:06 ` [PATCH v6 00/29] Fix delegation behaviour when server revokes some state Oleg Drokin
     [not found]   ` <104E1824-0235-41DF-AA9D-5C3F5560CA57@primarydata.com>
2016-09-21  1:07     ` Oleg Drokin

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.