2016-09-15 16:46:04

by Trond Myklebust

Subject: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

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

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

v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
v3: Now with added lock revoke fixes and close/delegreturn/locku fixes
v4: Close a bunch of corner cases

Trond Myklebust (20):
NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
NFSv4.1: Don't check delegations that are already marked as revoked
NFSv4.1: Allow test_stateid to handle session errors without waiting
NFSv4.1: Add a helper function to deal with expired stateids
NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a
stateid
NFSv4.1: Test delegation stateids when server declares "some state
revoked"
NFSv4.1: Deal with server reboots during delegation expiration
recovery
NFSv4.1: Don't recheck delegations that have already been checked
NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
NFSv4.1: FREE_STATEID can be asynchronous
NFSv4.1: Ensure we call FREE_STATEID if needed on
close/delegreturn/locku
NFSv4: nfs_inode_find_delegation_state_and_recover() should check all
stateids
NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single
stateid
NFSv4: nfs4_handle_delegation_recall_error() handle expiration as
revoke case
NFSv4: nfs4_handle_setlk_error() handle expiration as revoke case
NFSv4.1: nfs4_layoutget_handle_exception handle revoked state
NFSv4: Pass the stateid to the exception handler in
nfs4_read/write_done_cb
NFSv4: Fix a race in nfs_inode_reclaim_delegation()
NFSv4: Fix a race when updating an open_stateid

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

--
2.7.4

2016-09-15 16:46:06

by Trond Myklebust

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

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 <[email protected]>
---
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

2016-09-15 16:46:07

by Trond Myklebust

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

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 <[email protected]>
---
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

2016-09-15 16:46:11

by Trond Myklebust

Subject: [PATCH v4 04/20] NFSv4.1: Add a helper function to deal with expired stateids

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 <[email protected]>
---
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

2016-09-15 16:46:13

by Trond Myklebust

Subject: [PATCH v4 06/20] NFSv4.1: Test delegation stateids when server declares "some state revoked"

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 <[email protected]>
---
fs/nfs/delegation.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
fs/nfs/delegation.h | 4 +++
fs/nfs/nfs4_fs.h | 3 ++
fs/nfs/nfs4proc.c | 10 ++++++
fs/nfs/nfs4state.c | 17 +++++-----
5 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 76dc11ecdedd..ff0c1a0feb9d 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -802,8 +802,15 @@ static void nfs_delegation_mark_reclaim_server(struct nfs_server *server)
{
struct nfs_delegation *delegation;

- list_for_each_entry_rcu(delegation, &server->delegations, super_list)
+ list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
+ /*
+ * If the delegation may have been admin revoked, then we
+ * cannot reclaim it.
+ */
+ if (test_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags))
+ continue;
set_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
+ }
}

/**
@@ -867,6 +874,94 @@ restart:
rcu_read_unlock();
}

+static void nfs_mark_test_expired_delegation(struct nfs_server *server,
+ struct nfs_delegation *delegation)
+{
+ clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
+ set_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags);
+ set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state);
+}
+
+static void nfs_delegation_mark_test_expired_server(struct nfs_server *server)
+{
+ struct nfs_delegation *delegation;
+
+ list_for_each_entry_rcu(delegation, &server->delegations, super_list)
+ nfs_mark_test_expired_delegation(server, delegation);
+}
+
+/**
+ * nfs_mark_test_expired_all_delegations - mark all delegations for testing
+ * @clp: nfs_client to process
+ *
+ * Iterates through all the delegations associated with this server and
+ * marks them as needing to be checked for validity.
+ */
+void nfs_mark_test_expired_all_delegations(struct nfs_client *clp)
+{
+ struct nfs_server *server;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link)
+ nfs_delegation_mark_test_expired_server(server);
+ rcu_read_unlock();
+}
+
+/**
+ * nfs_reap_expired_delegations - reap expired delegations
+ * @clp: nfs_client to process
+ *
+ * Iterates through all the delegations associated with this server and
+ * checks if they have may have been revoked. This function is usually
+ * expected to be called in cases where the server may have lost its
+ * lease.
+ */
+void nfs_reap_expired_delegations(struct nfs_client *clp)
+{
+ const struct nfs4_minor_version_ops *ops = clp->cl_mvops;
+ struct nfs_delegation *delegation;
+ struct nfs_server *server;
+ struct inode *inode;
+ struct rpc_cred *cred;
+ nfs4_stateid stateid;
+
+restart:
+ rcu_read_lock();
+ list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
+ list_for_each_entry_rcu(delegation, &server->delegations,
+ super_list) {
+ if (test_bit(NFS_DELEGATION_RETURNING,
+ &delegation->flags))
+ continue;
+ if (test_bit(NFS_DELEGATION_TEST_EXPIRED,
+ &delegation->flags) == 0)
+ continue;
+ if (!nfs_sb_active(server->super))
+ continue;
+ inode = nfs_delegation_grab_inode(delegation);
+ if (inode == NULL) {
+ rcu_read_unlock();
+ nfs_sb_deactive(server->super);
+ goto restart;
+ }
+ cred = get_rpccred_rcu(delegation->cred);
+ nfs4_stateid_copy(&stateid, &delegation->stateid);
+ clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags);
+ rcu_read_unlock();
+ if (cred != NULL &&
+ ops->test_and_free_expired(server, &stateid, cred) < 0) {
+ nfs_revoke_delegation(inode, &stateid);
+ nfs_inode_find_state_and_recover(inode, &stateid);
+ }
+ put_rpccred(cred);
+ iput(inode);
+ nfs_sb_deactive(server->super);
+ goto restart;
+ }
+ }
+ rcu_read_unlock();
+}
+
/**
* nfs_delegations_present - check for existence of delegations
* @clp: client state handle
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index d40827af5913..1442e3b1521d 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -32,6 +32,7 @@ enum {
NFS_DELEGATION_REFERENCED,
NFS_DELEGATION_RETURNING,
NFS_DELEGATION_REVOKED,
+ NFS_DELEGATION_TEST_EXPIRED,
};

int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res);
@@ -52,6 +53,9 @@ void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid)
void nfs_delegation_mark_reclaim(struct nfs_client *clp);
void nfs_delegation_reap_unclaimed(struct nfs_client *clp);

+void nfs_mark_test_expired_all_delegations(struct nfs_client *clp);
+void nfs_reap_expired_delegations(struct nfs_client *clp);
+
/* NFSv4 delegation-related procedures */
int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync);
int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4390d73a92e5..b9083a6cefd6 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -39,6 +39,7 @@ enum nfs4_client_state {
NFS4CLNT_BIND_CONN_TO_SESSION,
NFS4CLNT_MOVED,
NFS4CLNT_LEASE_MOVED,
+ NFS4CLNT_DELEGATION_EXPIRED,
};

#define NFS4_RENEW_TIMEOUT 0x01
@@ -57,6 +58,8 @@ struct nfs4_minor_version_ops {
struct nfs_fsinfo *);
void (*free_lock_state)(struct nfs_server *,
struct nfs4_lock_state *);
+ int (*test_and_free_expired)(struct nfs_server *,
+ nfs4_stateid *, struct rpc_cred *);
struct nfs_seqid *
(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
const struct rpc_call_ops *call_sync_ops;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 9c203c221876..3552f3a4ceb0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2408,6 +2408,13 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st
return nfs4_open_expired(sp, state);
}

+static int nfs40_test_and_free_expired_stateid(struct nfs_server *server,
+ nfs4_stateid *stateid,
+ struct rpc_cred *cred)
+{
+ return -NFS4ERR_BAD_STATEID;
+}
+
#if defined(CONFIG_NFS_V4_1)
static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
nfs4_stateid *stateid,
@@ -8862,6 +8869,7 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
.match_stateid = nfs4_match_stateid,
.find_root_sec = nfs4_find_root_sec,
.free_lock_state = nfs4_release_lockowner,
+ .test_and_free_expired = nfs40_test_and_free_expired_stateid,
.alloc_seqid = nfs_alloc_seqid,
.call_sync_ops = &nfs40_call_sync_ops,
.reboot_recovery_ops = &nfs40_reboot_recovery_ops,
@@ -8889,6 +8897,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
.match_stateid = nfs41_match_stateid,
.find_root_sec = nfs41_find_root_sec,
.free_lock_state = nfs41_free_lock_state,
+ .test_and_free_expired = nfs41_test_and_free_expired_stateid,
.alloc_seqid = nfs_alloc_no_seqid,
.call_sync_ops = &nfs41_call_sync_ops,
.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
@@ -8918,6 +8927,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
.find_root_sec = nfs41_find_root_sec,
.free_lock_state = nfs41_free_lock_state,
.call_sync_ops = &nfs41_call_sync_ops,
+ .test_and_free_expired = nfs41_test_and_free_expired_stateid,
.alloc_seqid = nfs_alloc_no_seqid,
.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9801b5bb5fac..63da0411e2af 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1656,15 +1656,9 @@ static void nfs4_state_end_reclaim_reboot(struct nfs_client *clp)
put_rpccred(cred);
}

-static void nfs_delegation_clear_all(struct nfs_client *clp)
-{
- nfs_delegation_mark_reclaim(clp);
- nfs_delegation_reap_unclaimed(clp);
-}
-
static void nfs4_state_start_reclaim_nograce(struct nfs_client *clp)
{
- nfs_delegation_clear_all(clp);
+ nfs_mark_test_expired_all_delegations(clp);
nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce);
}

@@ -2195,7 +2189,7 @@ static void nfs41_handle_all_state_revoked(struct nfs_client *clp)

static void nfs41_handle_some_state_revoked(struct nfs_client *clp)
{
- nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce);
+ nfs4_state_start_reclaim_nograce(clp);
nfs4_schedule_state_manager(clp);

dprintk("%s: state revoked on server %s\n", __func__, clp->cl_hostname);
@@ -2420,6 +2414,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
nfs4_state_end_reclaim_reboot(clp);
}

+ /* Detect expired delegations... */
+ if (test_and_clear_bit(NFS4CLNT_DELEGATION_EXPIRED, &clp->cl_state)) {
+ section = "detect expired delegations";
+ nfs_reap_expired_delegations(clp);
+ continue;
+ }
+
/* Now recover expired state... */
if (test_and_clear_bit(NFS4CLNT_RECLAIM_NOGRACE, &clp->cl_state)) {
section = "reclaim nograce";
--
2.7.4

2016-09-15 16:46:14

by Trond Myklebust

Subject: [PATCH v4 07/20] NFSv4.1: Deal with server reboots during delegation expiration recovery

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 <[email protected]>
---
fs/nfs/delegation.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index ff0c1a0feb9d..b873271c613a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -874,6 +874,13 @@ restart:
rcu_read_unlock();
}

+static inline bool nfs4_server_rebooted(const struct nfs_client *clp)
+{
+ return (clp->cl_state & (BIT(NFS4CLNT_CHECK_LEASE) |
+ BIT(NFS4CLNT_LEASE_EXPIRED) |
+ BIT(NFS4CLNT_SESSION_RESET))) != 0;
+}
+
static void nfs_mark_test_expired_delegation(struct nfs_server *server,
struct nfs_delegation *delegation)
{
@@ -882,6 +889,19 @@ static void nfs_mark_test_expired_delegation(struct nfs_server *server,
set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state);
}

+static void nfs_inode_mark_test_expired_delegation(struct nfs_server *server,
+ struct inode *inode)
+{
+ struct nfs_delegation *delegation;
+
+ rcu_read_lock();
+ delegation = rcu_dereference(NFS_I(inode)->delegation);
+ if (delegation)
+ nfs_mark_test_expired_delegation(server, delegation);
+ rcu_read_unlock();
+
+}
+
static void nfs_delegation_mark_test_expired_server(struct nfs_server *server)
{
struct nfs_delegation *delegation;
@@ -954,6 +974,12 @@ restart:
nfs_inode_find_state_and_recover(inode, &stateid);
}
put_rpccred(cred);
+ if (nfs4_server_rebooted(clp)) {
+ nfs_inode_mark_test_expired_delegation(server,inode);
+ iput(inode);
+ nfs_sb_deactive(server->super);
+ return;
+ }
iput(inode);
nfs_sb_deactive(server->super);
goto restart;
--
2.7.4

2016-09-15 16:46:08

by Trond Myklebust

Subject: [PATCH v4 03/20] NFSv4.1: Allow test_stateid to handle session errors without waiting

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 <[email protected]>
---
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

2016-09-15 16:46:15

by Trond Myklebust

Subject: [PATCH v4 08/20] NFSv4.1: Don't recheck delegations that have already been checked

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

Signed-off-by: Trond Myklebust <[email protected]>
---
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

2016-09-15 16:46:17

by Trond Myklebust

Subject: [PATCH v4 10/20] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks

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 <[email protected]>
---
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

2016-09-15 16:46:18

by Trond Myklebust

Subject: [PATCH v4 11/20] NFSv4.1: FREE_STATEID can be asynchronous

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 <[email protected]>
---
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

2016-09-15 16:46:19

by Trond Myklebust

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

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 <[email protected]>
---
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

2016-09-15 16:46:16

by Trond Myklebust

Subject: [PATCH v4 05/20] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid

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

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/delegation.c | 30 +++++++++++++++++++++++-------
fs/nfs/delegation.h | 2 +-
fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
fs/nfs/nfs4proc.c | 14 ++++++++------
4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 322c2585bc34..76dc11ecdedd 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -642,23 +642,39 @@ static void nfs_client_mark_return_unused_delegation_types(struct nfs_client *cl
rcu_read_unlock();
}

-static void nfs_revoke_delegation(struct inode *inode)
+static void nfs_mark_delegation_revoked(struct nfs_server *server,
+ struct nfs_delegation *delegation)
+{
+ set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
+ nfs_mark_return_delegation(server, delegation);
+}
+
+static bool nfs_revoke_delegation(struct inode *inode,
+ const nfs4_stateid *stateid)
{
struct nfs_delegation *delegation;
+ bool ret = false;
+
rcu_read_lock();
delegation = rcu_dereference(NFS_I(inode)->delegation);
- if (delegation != NULL) {
- set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
- nfs_mark_return_delegation(NFS_SERVER(inode), delegation);
- }
+ if (delegation == NULL)
+ goto out;
+ if (stateid && !nfs4_stateid_match(stateid, &delegation->stateid))
+ goto out;
+ nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
+ ret = true;
+out:
rcu_read_unlock();
+ return ret;
}

-void nfs_remove_bad_delegation(struct inode *inode)
+void nfs_remove_bad_delegation(struct inode *inode,
+ const nfs4_stateid *stateid)
{
struct nfs_delegation *delegation;

- nfs_revoke_delegation(inode);
+ if (!nfs_revoke_delegation(inode, stateid))
+ return;
delegation = nfs_inode_detach_delegation(inode);
if (delegation) {
nfs_inode_find_state_and_recover(inode, &delegation->stateid);
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 64724d252a79..d40827af5913 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -47,7 +47,7 @@ void nfs_expire_unused_delegation_types(struct nfs_client *clp, fmode_t flags);
void nfs_expire_unreferenced_delegations(struct nfs_client *clp);
int nfs_client_return_marked_delegations(struct nfs_client *clp);
int nfs_delegations_present(struct nfs_client *clp);
-void nfs_remove_bad_delegation(struct inode *inode);
+void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid);

void nfs_delegation_mark_reclaim(struct nfs_client *clp);
void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 51b51369704c..98ace127bf86 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1080,7 +1080,7 @@ static int ff_layout_async_handle_error_v4(struct rpc_task *task,
case -NFS4ERR_BAD_STATEID:
if (state == NULL)
break;
- nfs_remove_bad_delegation(state->inode);
+ nfs_remove_bad_delegation(state->inode, NULL);
case -NFS4ERR_OPENMODE:
if (state == NULL)
break;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0ade81441ac2..9c203c221876 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2385,9 +2385,10 @@ static int nfs4_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *sta
return ret;
}

-static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state)
+static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state,
+ const nfs4_stateid *stateid)
{
- nfs_remove_bad_delegation(state->inode);
+ nfs_remove_bad_delegation(state->inode, stateid);
write_seqlock(&state->seqlock);
nfs4_stateid_copy(&state->stateid, &state->open_stateid);
write_sequnlock(&state->seqlock);
@@ -2397,7 +2398,7 @@ static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state)
static void nfs40_clear_delegation_stateid(struct nfs4_state *state)
{
if (rcu_access_pointer(NFS_I(state->inode)->delegation) != NULL)
- nfs_finish_clear_delegation_stateid(state);
+ nfs_finish_clear_delegation_stateid(state, NULL);
}

static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *state)
@@ -2443,19 +2444,20 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
rcu_read_unlock();
return;
}
+
+ nfs4_stateid_copy(&stateid, &delegation->stateid);
if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
rcu_read_unlock();
- nfs_finish_clear_delegation_stateid(state);
+ nfs_finish_clear_delegation_stateid(state, &stateid);
return;
}

- nfs4_stateid_copy(&stateid, &delegation->stateid);
cred = get_rpccred(delegation->cred);
rcu_read_unlock();
status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
trace_nfs4_test_delegation_stateid(state, NULL, status);
if (status != NFS_OK)
- nfs_finish_clear_delegation_stateid(state);
+ nfs_finish_clear_delegation_stateid(state, &stateid);

put_rpccred(cred);
}
--
2.7.4

2016-09-15 16:46:22

by Trond Myklebust

Subject: [PATCH v4 14/20] NFSv4: nfs4_do_handle_exception() handle revoke/expiry of a single stateid

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 <[email protected]>
---
fs/nfs/nfs4proc.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

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

@@ -422,12 +428,6 @@ static int nfs4_do_handle_exception(struct nfs_server *server,
if (ret < 0)
break;
goto wait_on_recovery;
- case -NFS4ERR_EXPIRED:
- if (state != NULL) {
- ret = nfs4_schedule_stateid_recovery(server, state);
- if (ret < 0)
- break;
- }
case -NFS4ERR_STALE_STATEID:
case -NFS4ERR_STALE_CLIENTID:
nfs4_schedule_lease_recovery(clp);
--
2.7.4

2016-09-15 16:46:21

by Trond Myklebust

Subject: [PATCH v4 13/20] NFSv4: nfs_inode_find_delegation_state_and_recover() should check all stateids

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 <[email protected]>
---
fs/nfs/delegation.c | 18 ++++++++++++++++++
fs/nfs/delegation.h | 2 ++
fs/nfs/nfs4state.c | 44 +++++++++++++++++++++++++++++++++++++++-----
3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index b873271c613a..efd50159ab4a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -988,6 +988,24 @@ restart:
rcu_read_unlock();
}

+void nfs_inode_find_delegation_state_and_recover(struct inode *inode,
+ const nfs4_stateid *stateid)
+{
+ struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+ struct nfs_delegation *delegation;
+ bool found = false;
+
+ rcu_read_lock();
+ delegation = rcu_dereference(NFS_I(inode)->delegation);
+ if (delegation && nfs4_stateid_match(&delegation->stateid, stateid)) {
+ nfs_mark_test_expired_delegation(NFS_SERVER(inode), delegation);
+ found = true;
+ }
+ rcu_read_unlock();
+ if (found)
+ nfs4_schedule_state_manager(clp);
+}
+
/**
* nfs_delegations_present - check for existence of delegations
* @clp: client state handle
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 1442e3b1521d..e9d555796873 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -66,6 +66,8 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
int nfs4_have_delegation(struct inode *inode, fmode_t flags);
int nfs4_check_delegation(struct inode *inode, fmode_t flags);
bool nfs4_delegation_flush_on_close(const struct inode *inode);
+void nfs_inode_find_delegation_state_and_recover(struct inode *inode,
+ const nfs4_stateid *stateid);

#endif

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 63da0411e2af..609643f31e7a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1337,6 +1337,35 @@ int nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4_
}
EXPORT_SYMBOL_GPL(nfs4_schedule_stateid_recovery);

+static struct nfs4_lock_state *
+nfs_state_find_lock_state_by_stateid(struct nfs4_state *state,
+ const nfs4_stateid *stateid)
+{
+ struct nfs4_lock_state *pos;
+
+ list_for_each_entry(pos, &state->lock_states, ls_locks) {
+ if (!test_bit(NFS_LOCK_INITIALIZED, &pos->ls_flags))
+ continue;
+ if (nfs4_stateid_match(&pos->ls_stateid, stateid))
+ return pos;
+ }
+ return NULL;
+}
+
+static bool nfs_state_lock_state_matches_stateid(struct nfs4_state *state,
+ const nfs4_stateid *stateid)
+{
+ bool found = false;
+
+ if (test_bit(LK_STATE_IN_USE, &state->flags)) {
+ spin_lock(&state->state_lock);
+ if (nfs_state_find_lock_state_by_stateid(state, stateid))
+ found = true;
+ spin_unlock(&state->state_lock);
+ }
+ return found;
+}
+
void nfs_inode_find_state_and_recover(struct inode *inode,
const nfs4_stateid *stateid)
{
@@ -1351,14 +1380,19 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
state = ctx->state;
if (state == NULL)
continue;
- if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
- continue;
- if (!nfs4_stateid_match(&state->stateid, stateid))
+ if (nfs4_stateid_match(&state->stateid, stateid)) {
+ nfs4_state_mark_reclaim_nograce(clp, state);
+ found = true;
continue;
- nfs4_state_mark_reclaim_nograce(clp, state);
- found = true;
+ }
+ if (nfs_state_lock_state_matches_stateid(state, stateid)) {
+ nfs4_state_mark_reclaim_nograce(clp, state);
+ found = true;
+ }
}
spin_unlock(&inode->i_lock);
+
+ nfs_inode_find_delegation_state_and_recover(inode, stateid);
if (found)
nfs4_schedule_state_manager(clp);
}
--
2.7.4

2016-09-15 16:46:23

by Trond Myklebust

Subject: [PATCH v4 15/20] NFSv4: nfs4_handle_delegation_recall_error() handle expiration as revoke case

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

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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

2016-09-15 16:46:24

by Trond Myklebust

Subject: [PATCH v4 16/20] NFSv4: nfs4_handle_setlk_error() handle expiration as revoke case

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

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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

2016-09-15 16:46:16

by Trond Myklebust

Subject: [PATCH v4 09/20] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID

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

2016-09-15 16:46:25

by Trond Myklebust

Subject: [PATCH v4 17/20] NFSv4.1: nfs4_layoutget_handle_exception handle revoked state

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

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 3 +++
1 file changed, 3 insertions(+)

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

--
2.7.4

2016-09-15 16:46:26

by Trond Myklebust

Subject: [PATCH v4 18/20] NFSv4: Pass the stateid to the exception handler in nfs4_read/write_done_cb

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 <[email protected]>
---
fs/nfs/nfs4proc.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e2ab6f665dba..82449e578608 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4523,11 +4523,18 @@ static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_pgio_header *hdr)
struct nfs_server *server = NFS_SERVER(hdr->inode);

trace_nfs4_read(hdr, task->tk_status);
- if (nfs4_async_handle_error(task, server,
- hdr->args.context->state,
- NULL) == -EAGAIN) {
- rpc_restart_call_prepare(task);
- return -EAGAIN;
+ if (task->tk_status < 0) {
+ struct nfs4_exception exception = {
+ .inode = hdr->inode,
+ .state = hdr->args.context->state,
+ .stateid = &hdr->args.stateid,
+ };
+ task->tk_status = nfs4_async_handle_exception(task,
+ server, task->tk_status, &exception);
+ if (exception.retry) {
+ rpc_restart_call_prepare(task);
+ return -EAGAIN;
+ }
}

__nfs4_read_done_cb(hdr);
@@ -4596,11 +4603,19 @@ static int nfs4_write_done_cb(struct rpc_task *task,
struct inode *inode = hdr->inode;

trace_nfs4_write(hdr, task->tk_status);
- if (nfs4_async_handle_error(task, NFS_SERVER(inode),
- hdr->args.context->state,
- NULL) == -EAGAIN) {
- rpc_restart_call_prepare(task);
- return -EAGAIN;
+ if (task->tk_status < 0) {
+ struct nfs4_exception exception = {
+ .inode = hdr->inode,
+ .state = hdr->args.context->state,
+ .stateid = &hdr->args.stateid,
+ };
+ task->tk_status = nfs4_async_handle_exception(task,
+ NFS_SERVER(inode), task->tk_status,
+ &exception);
+ if (exception.retry) {
+ rpc_restart_call_prepare(task);
+ return -EAGAIN;
+ }
}
if (task->tk_status >= 0) {
renew_lease(NFS_SERVER(inode), hdr->timestamp);
--
2.7.4

2016-09-15 16:46:28

by Trond Myklebust

Subject: [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid

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 <[email protected]>
---
fs/nfs/nfs4proc.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 82449e578608..08c6e46c5b95 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1399,11 +1399,12 @@ static void nfs_test_and_clear_all_open_stateid(struct nfs4_state *state)
}

static bool nfs_need_update_open_stateid(struct nfs4_state *state,
- nfs4_stateid *stateid)
+ const nfs4_stateid *stateid, nfs4_stateid *freeme)
{
if (test_and_set_bit(NFS_OPEN_STATE, &state->flags) == 0)
return true;
if (!nfs4_stateid_match_other(stateid, &state->open_stateid)) {
+ nfs4_stateid_copy(freeme, &state->open_stateid);
nfs_test_and_clear_all_open_stateid(state);
return true;
}
@@ -1467,7 +1468,9 @@ static void nfs_clear_open_stateid(struct nfs4_state *state,
nfs4_schedule_state_manager(state->owner->so_server->nfs_client);
}

-static void nfs_set_open_stateid_locked(struct nfs4_state *state, nfs4_stateid *stateid, fmode_t fmode)
+static void nfs_set_open_stateid_locked(struct nfs4_state *state,
+ const nfs4_stateid *stateid, fmode_t fmode,
+ nfs4_stateid *freeme)
{
switch (fmode) {
case FMODE_READ:
@@ -1479,14 +1482,18 @@ static void nfs_set_open_stateid_locked(struct nfs4_state *state, nfs4_stateid *
case FMODE_READ|FMODE_WRITE:
set_bit(NFS_O_RDWR_STATE, &state->flags);
}
- if (!nfs_need_update_open_stateid(state, stateid))
+ if (!nfs_need_update_open_stateid(state, stateid, freeme))
return;
if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
nfs4_stateid_copy(&state->stateid, stateid);
nfs4_stateid_copy(&state->open_stateid, stateid);
}

-static void __update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, const nfs4_stateid *deleg_stateid, fmode_t fmode)
+static void __update_open_stateid(struct nfs4_state *state,
+ const nfs4_stateid *open_stateid,
+ const nfs4_stateid *deleg_stateid,
+ fmode_t fmode,
+ nfs4_stateid *freeme)
{
/*
* Protect the call to nfs4_state_set_mode_locked and
@@ -1499,16 +1506,22 @@ static void __update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_s
set_bit(NFS_DELEGATED_STATE, &state->flags);
}
if (open_stateid != NULL)
- nfs_set_open_stateid_locked(state, open_stateid, fmode);
+ nfs_set_open_stateid_locked(state, open_stateid, fmode, freeme);
write_sequnlock(&state->seqlock);
update_open_stateflags(state, fmode);
spin_unlock(&state->owner->so_lock);
}

-static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, nfs4_stateid *delegation, fmode_t fmode)
+static int update_open_stateid(struct nfs4_state *state,
+ const nfs4_stateid *open_stateid,
+ const nfs4_stateid *delegation,
+ fmode_t fmode)
{
+ struct nfs_server *server = NFS_SERVER(state->inode);
+ struct nfs_client *clp = server->nfs_client;
struct nfs_inode *nfsi = NFS_I(state->inode);
struct nfs_delegation *deleg_cur;
+ nfs4_stateid freeme = {0};
int ret = 0;

fmode &= (FMODE_READ|FMODE_WRITE);
@@ -1530,7 +1543,8 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
goto no_delegation_unlock;

nfs_mark_delegation_referenced(deleg_cur);
- __update_open_stateid(state, open_stateid, &deleg_cur->stateid, fmode);
+ __update_open_stateid(state, open_stateid, &deleg_cur->stateid,
+ fmode, &freeme);
ret = 1;
no_delegation_unlock:
spin_unlock(&deleg_cur->lock);
@@ -1538,11 +1552,14 @@ no_delegation:
rcu_read_unlock();

if (!ret && open_stateid != NULL) {
- __update_open_stateid(state, open_stateid, NULL, fmode);
+ __update_open_stateid(state, open_stateid, NULL, fmode, &freeme);
ret = 1;
}
if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags))
- nfs4_schedule_state_manager(state->owner->so_server->nfs_client);
+ nfs4_schedule_state_manager(clp);
+ if (freeme.type != 0)
+ nfs4_test_and_free_stateid(server, &freeme,
+ state->owner->so_cred);

return ret;
}
--
2.7.4

2016-09-15 16:46:27

by Trond Myklebust

Subject: [PATCH v4 19/20] NFSv4: Fix a race in nfs_inode_reclaim_delegation()

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

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/delegation.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

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

static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
--
2.7.4

2016-09-15 22:17:37

by kernel test robot

Subject: Re: [PATCH v4 20/20] NFSv4: Fix a race when updating an open_stateid

Hi Trond,

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

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

All warnings (new ones prefixed by >>):

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

vim +1524 fs/nfs/nfs4proc.c

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

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

Attachments:

2016-09-16 04:38:55

by Oleg Drokin

Subject: Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

On Sep 15, 2016, at 12:45 PM, Trond Myklebust wrote:

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

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

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

2016-09-16 15:37:19

by Trond Myklebust

Subject: Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

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

2016-09-16 21:16:15

by Oleg Drokin

Subject: Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

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

> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>> On Sep 15, 2016, at 12:45 PM, Trond Myklebust wrote:
>>
>>>
>>> According to RFC5661, if any of the SEQUENCE status bits
>>> SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
>>> SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED,
>>> SEQ4_STATUS_ADMIN_STATE_REVOKED,
>>> or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to
>>> use
>>> TEST_STATEID to figure out which stateids have been revoked, so we
>>> can acknowledge the loss of state using FREE_STATEID.
>>>
>>> While we already do this for open and lock state, we have not been
>>> doing
>>> so for all the delegations.
>>>
>>> v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
>>> v3: Now with added lock revoke fixes and close/delegreturn/locku
>>> fixes
>>> v4: Close a bunch of corner cases
>>
>> This one seems to be looping on the client in a way very similar
>> to the first failure in that it's the serverip-named process that's
>> using the cpu, but the debug log is very similar to the second
>> failure
>> in test stateid, except this time it's not "success 0", but "success
>> -10025":
>>
> Ah... I think I see what the issue is... Does the following help?

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

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

2016-09-16 21:21:37

by Oleg Drokin

Subject: Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

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

>
> On Sep 16, 2016, at 11:36 AM, Trond Myklebust wrote:
>
>> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>>> On Sep 15, 2016, at 12:45 PM, Trond Myklebust wrote:
>>>
>>>>
>>>> According to RFC5661, if any of the SEQUENCE status bits
>>>> SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
>>>> SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED,
>>>> SEQ4_STATUS_ADMIN_STATE_REVOKED,
>>>> or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to
>>>> use
>>>> TEST_STATEID to figure out which stateids have been revoked, so we
>>>> can acknowledge the loss of state using FREE_STATEID.
>>>>
>>>> While we already do this for open and lock state, we have not been
>>>> doing
>>>> so for all the delegations.
>>>>
>>>> v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
>>>> v3: Now with added lock revoke fixes and close/delegreturn/locku
>>>> fixes
>>>> v4: Close a bunch of corner cases
>>>
>>> This one seems to be looping on the client in a way very similar
>>> to the first failure in that it's the serverip-named process that's
>>> using the cpu, but the debug log is very similar to the second
>>> failure
>>> in test stateid, except this time it's not "success 0", but "success
>>> -10025":
>>>
>> Ah... I think I see what the issue is... Does the following help?
>
> Well, it changed the output back to what I had with the first patch set, I think:
> I.e. now it's nfsid test succeded, 0, and the process using the cpu is again
> a userspace one:

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

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

2016-09-16 21:28:53

by Trond Myklebust

Subject: Re: [PATCH v4 00/20] Fix delegation behaviour when server revokes some state

> On Sep 16, 2016, at 17:21, Oleg Drokin <[email protected]> wrote:
>=20
>=20
> On Sep 16, 2016, at 5:15 PM, Oleg Drokin wrote:
>=20
>>=20
>> On Sep 16, 2016, at 11:36 AM, Trond Myklebust wrote:
>>=20
>>> On Fri, 2016-09-16 at 00:38 -0400, Oleg Drokin wrote:
>>>> On Sep 15, 2016, at 12:45 PM, Trond Myklebust wrote:
>>>>=20
>>>>>=20
>>>>> According to RFC5661, if any of the SEQUENCE status bits
>>>>> SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED,
>>>>> SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED,
>>>>> SEQ4_STATUS_ADMIN_STATE_REVOKED,
>>>>> or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to
>>>>> use
>>>>> TEST_STATEID to figure out which stateids have been revoked, so we
>>>>> can acknowledge the loss of state using FREE_STATEID.
>>>>>=20
>>>>> While we already do this for open and lock state, we have not been
>>>>> doing
>>>>> so for all the delegations.
>>>>>=20
>>>>> v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too
>>>>> v3: Now with added lock revoke fixes and close/delegreturn/locku
>>>>> fixes
>>>>> v4: Close a bunch of corner cases
>>>>=20
>>>> This one seems to be looping on the client in a way very similar
>>>> to the first failure in that it's the serverip-named process that's
>>>> using the cpu, but the debug log is very similar to the second
>>>> failure
>>>> in test stateid, except this time it's not "success 0", but "success
>>>> -10025":
>>>>=20
>>> Ah... I think I see what the issue is... Does the following help?
>>=20
>> Well, it changed the output back to what I had with the first patch set,=
I think:
>> I.e. now it's nfsid test succeded, 0, and the process using the cpu is a=
gain
>> a userspace one:
>=20
> Ah, I did not realize you probably meant not to apply this on top of the =
previous
> batch, but replace patch #14 with this one.
> So I am trying it now.

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

Thanks!