* [PATCH v3 00/12] Fix delegation behaviour when server revokes some state
@ 2016-09-09 18:04 Trond Myklebust
2016-09-09 18:04 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust
2016-09-10 0:49 ` [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Oleg Drokin
0 siblings, 2 replies; 16+ messages in thread
From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw)
To: linux-nfs; +Cc: 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
Trond Myklebust (12):
NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags
NFSv4.1: Don't check delegations that are already marked as revoked
NFSv4.1: Allow test_stateid to handle session errors without waiting
NFSv4.1: Add a helper function to deal with expired stateids
NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a
stateid
NFSv4.1: Test delegation stateids when server declares "some state
revoked"
NFSv4.1: Deal with server reboots during delegation expiration
recovery
NFSv4.1: Don't recheck delegations that have already been checked
NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks
NFSv4.1: FREE_STATEID can be asynchronous
NFSv4.1: Ensure we call FREE_STATEID if needed on
close/delegreturn/locku
fs/nfs/delegation.c | 153 ++++++++++++++++++-
fs/nfs/delegation.h | 6 +-
fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
fs/nfs/nfs4_fs.h | 5 +-
fs/nfs/nfs4proc.c | 258 ++++++++++++++++++++++-----------
fs/nfs/nfs4session.h | 1 +
fs/nfs/nfs4state.c | 29 ++--
include/linux/nfs4.h | 1 +
8 files changed, 348 insertions(+), 107 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags 2016-09-09 18:04 [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust 2016-09-09 18:26 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Olga Kornievskaia 2016-09-10 0:49 ` [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Oleg Drokin 1 sibling, 2 replies; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 25a2af707233..bd33777f03c4 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -616,6 +616,7 @@ int nfs40_setup_sequence(struct nfs4_slot_table *tbl, } spin_unlock(&tbl->slot_tbl_lock); + slot->privileged = args->sa_privileged ? 1 : 0; args->sa_slot = slot; res->sr_slot = slot; @@ -728,7 +729,8 @@ static int nfs41_sequence_process(struct rpc_task *task, clp = session->clp; do_renew_lease(clp, res->sr_timestamp); /* Check sequence flags */ - nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags); + nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags, + !!slot->privileged); nfs41_update_target_slotid(slot->table, slot, res); break; case 1: @@ -875,6 +877,7 @@ int nfs41_setup_sequence(struct nfs4_session *session, } spin_unlock(&tbl->slot_tbl_lock); + slot->privileged = args->sa_privileged ? 1 : 0; args->sa_slot = slot; dprintk("<-- %s slotid=%u seqid=%u\n", __func__, diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h index 3bb6af70973c..dae385500005 100644 --- a/fs/nfs/nfs4session.h +++ b/fs/nfs/nfs4session.h @@ -23,6 +23,7 @@ struct nfs4_slot { u32 slot_nr; u32 seq_nr; unsigned int interrupted : 1, + privileged : 1, seq_done : 1; }; diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index cada00aa5096..9801b5bb5fac 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -2227,13 +2227,22 @@ static void nfs41_handle_cb_path_down(struct nfs_client *clp) nfs4_schedule_state_manager(clp); } -void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags) +void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags, + bool recovery) { if (!flags) return; dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n", __func__, clp->cl_hostname, clp->cl_clientid, flags); + /* + * If we're called from the state manager thread, then assume we're + * already handling the RECLAIM_NEEDED and/or STATE_REVOKED. + * Those flags are expected to remain set until we're done + * recovering (see RFC5661, section 18.46.3). + */ + if (recovery) + goto out_recovery; if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED) nfs41_handle_server_reboot(clp); @@ -2246,6 +2255,7 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags) nfs4_schedule_lease_moved_recovery(clp); if (flags & SEQ4_STATUS_RECALLABLE_STATE_REVOKED) nfs41_handle_recallable_state_revoked(clp); +out_recovery: if (flags & SEQ4_STATUS_BACKCHANNEL_FAULT) nfs41_handle_backchannel_fault(clp); else if (flags & (SEQ4_STATUS_CB_PATH_DOWN | -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked 2016-09-09 18:04 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 03/12] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust 2016-09-09 18:26 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Olga Kornievskaia 1 sibling, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 bd33777f03c4..254ab545c391 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2423,6 +2423,11 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) rcu_read_unlock(); return; } + if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) { + rcu_read_unlock(); + nfs_finish_clear_delegation_stateid(state); + return; + } nfs4_stateid_copy(&stateid, &delegation->stateid); cred = get_rpccred(delegation->cred); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 03/12] NFSv4.1: Allow test_stateid to handle session errors without waiting 2016-09-09 18:04 ` [PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 04/12] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 254ab545c391..9b7ff26b63ff 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -8590,6 +8590,23 @@ static int _nfs41_test_stateid(struct nfs_server *server, return -res.status; } +static void nfs4_handle_delay_or_session_error(struct nfs_server *server, + int err, struct nfs4_exception *exception) +{ + exception->retry = 0; + switch(err) { + case -NFS4ERR_DELAY: + nfs4_handle_exception(server, err, exception); + break; + case -NFS4ERR_BADSESSION: + case -NFS4ERR_BADSLOT: + case -NFS4ERR_BAD_HIGH_SLOT: + case -NFS4ERR_CONN_NOT_BOUND_TO_SESSION: + case -NFS4ERR_DEADSESSION: + nfs4_do_handle_exception(server, err, exception); + } +} + /** * nfs41_test_stateid - perform a TEST_STATEID operation * @@ -8609,9 +8626,7 @@ static int nfs41_test_stateid(struct nfs_server *server, int err; do { err = _nfs41_test_stateid(server, stateid, cred); - if (err != -NFS4ERR_DELAY) - break; - nfs4_handle_exception(server, err, &exception); + nfs4_handle_delay_or_session_error(server, err, &exception); } while (exception.retry); return err; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 04/12] NFSv4.1: Add a helper function to deal with expired stateids 2016-09-09 18:04 ` [PATCH v3 03/12] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 05/12] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 9b7ff26b63ff..2fe3445cb7b4 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2408,6 +2408,26 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st } #if defined(CONFIG_NFS_V4_1) +static int nfs41_test_and_free_expired_stateid(struct nfs_server *server, + nfs4_stateid *stateid, + struct rpc_cred *cred) +{ + int status; + + status = nfs41_test_stateid(server, stateid, cred); + + switch (status) { + case -NFS4ERR_EXPIRED: + case -NFS4ERR_ADMIN_REVOKED: + case -NFS4ERR_DELEG_REVOKED: + /* Ack the revoked state to the server */ + nfs41_free_stateid(server, stateid, cred); + case -NFS4ERR_BAD_STATEID: + return status; + } + return NFS_OK; +} + static void nfs41_check_delegation_stateid(struct nfs4_state *state) { struct nfs_server *server = NFS_SERVER(state->inode); @@ -2432,16 +2452,10 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) nfs4_stateid_copy(&stateid, &delegation->stateid); cred = get_rpccred(delegation->cred); rcu_read_unlock(); - status = nfs41_test_stateid(server, &stateid, cred); + status = nfs41_test_and_free_expired_stateid(server, &stateid, cred); trace_nfs4_test_delegation_stateid(state, NULL, status); - - if (status != NFS_OK) { - /* Free the stateid unless the server explicitly - * informs us the stateid is unrecognized. */ - if (status != -NFS4ERR_BAD_STATEID) - nfs41_free_stateid(server, &stateid, cred); + if (status != NFS_OK) nfs_finish_clear_delegation_stateid(state); - } put_rpccred(cred); } @@ -2467,14 +2481,9 @@ static int nfs41_check_open_stateid(struct nfs4_state *state) (test_bit(NFS_O_RDWR_STATE, &state->flags) == 0)) return -NFS4ERR_BAD_STATEID; - status = nfs41_test_stateid(server, stateid, cred); + status = nfs41_test_and_free_expired_stateid(server, stateid, cred); trace_nfs4_test_open_stateid(state, NULL, status); if (status != NFS_OK) { - /* Free the stateid unless the server explicitly - * informs us the stateid is unrecognized. */ - if (status != -NFS4ERR_BAD_STATEID) - nfs41_free_stateid(server, stateid, cred); - clear_bit(NFS_O_RDONLY_STATE, &state->flags); clear_bit(NFS_O_WRONLY_STATE, &state->flags); clear_bit(NFS_O_RDWR_STATE, &state->flags); @@ -6109,17 +6118,11 @@ static int nfs41_check_expired_locks(struct nfs4_state *state) if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) { struct rpc_cred *cred = lsp->ls_state->owner->so_cred; - status = nfs41_test_stateid(server, + status = nfs41_test_and_free_expired_stateid(server, &lsp->ls_stateid, cred); trace_nfs4_test_lock_stateid(state, lsp, status); if (status != NFS_OK) { - /* Free the stateid unless the server - * informs us the stateid is unrecognized. */ - if (status != -NFS4ERR_BAD_STATEID) - nfs41_free_stateid(server, - &lsp->ls_stateid, - cred); clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags); ret = status; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 05/12] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid 2016-09-09 18:04 ` [PATCH v3 04/12] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 06/12] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: Oleg Drokin Allow the callers of nfs_remove_bad_delegation() to specify the stateid that needs to be marked as bad. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 30 +++++++++++++++++++++++------- fs/nfs/delegation.h | 2 +- fs/nfs/flexfilelayout/flexfilelayout.c | 2 +- fs/nfs/nfs4proc.c | 14 ++++++++------ 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 322c2585bc34..76dc11ecdedd 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -642,23 +642,39 @@ static void nfs_client_mark_return_unused_delegation_types(struct nfs_client *cl rcu_read_unlock(); } -static void nfs_revoke_delegation(struct inode *inode) +static void nfs_mark_delegation_revoked(struct nfs_server *server, + struct nfs_delegation *delegation) +{ + set_bit(NFS_DELEGATION_REVOKED, &delegation->flags); + nfs_mark_return_delegation(server, delegation); +} + +static bool nfs_revoke_delegation(struct inode *inode, + const nfs4_stateid *stateid) { struct nfs_delegation *delegation; + bool ret = false; + rcu_read_lock(); delegation = rcu_dereference(NFS_I(inode)->delegation); - if (delegation != NULL) { - set_bit(NFS_DELEGATION_REVOKED, &delegation->flags); - nfs_mark_return_delegation(NFS_SERVER(inode), delegation); - } + if (delegation == NULL) + goto out; + if (stateid && !nfs4_stateid_match(stateid, &delegation->stateid)) + goto out; + nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation); + ret = true; +out: rcu_read_unlock(); + return ret; } -void nfs_remove_bad_delegation(struct inode *inode) +void nfs_remove_bad_delegation(struct inode *inode, + const nfs4_stateid *stateid) { struct nfs_delegation *delegation; - nfs_revoke_delegation(inode); + if (!nfs_revoke_delegation(inode, stateid)) + return; delegation = nfs_inode_detach_delegation(inode); if (delegation) { nfs_inode_find_state_and_recover(inode, &delegation->stateid); diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index 64724d252a79..d40827af5913 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -47,7 +47,7 @@ void nfs_expire_unused_delegation_types(struct nfs_client *clp, fmode_t flags); void nfs_expire_unreferenced_delegations(struct nfs_client *clp); int nfs_client_return_marked_delegations(struct nfs_client *clp); int nfs_delegations_present(struct nfs_client *clp); -void nfs_remove_bad_delegation(struct inode *inode); +void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid); void nfs_delegation_mark_reclaim(struct nfs_client *clp); void nfs_delegation_reap_unclaimed(struct nfs_client *clp); diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c index 51b51369704c..98ace127bf86 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.c +++ b/fs/nfs/flexfilelayout/flexfilelayout.c @@ -1080,7 +1080,7 @@ static int ff_layout_async_handle_error_v4(struct rpc_task *task, case -NFS4ERR_BAD_STATEID: if (state == NULL) break; - nfs_remove_bad_delegation(state->inode); + nfs_remove_bad_delegation(state->inode, NULL); case -NFS4ERR_OPENMODE: if (state == NULL) break; diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 2fe3445cb7b4..0ae39e3b13e1 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2385,9 +2385,10 @@ static int nfs4_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *sta return ret; } -static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state) +static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state, + const nfs4_stateid *stateid) { - nfs_remove_bad_delegation(state->inode); + nfs_remove_bad_delegation(state->inode, stateid); write_seqlock(&state->seqlock); nfs4_stateid_copy(&state->stateid, &state->open_stateid); write_sequnlock(&state->seqlock); @@ -2397,7 +2398,7 @@ static void nfs_finish_clear_delegation_stateid(struct nfs4_state *state) static void nfs40_clear_delegation_stateid(struct nfs4_state *state) { if (rcu_access_pointer(NFS_I(state->inode)->delegation) != NULL) - nfs_finish_clear_delegation_stateid(state); + nfs_finish_clear_delegation_stateid(state, NULL); } static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *state) @@ -2443,19 +2444,20 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) rcu_read_unlock(); return; } + + nfs4_stateid_copy(&stateid, &delegation->stateid); if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) { rcu_read_unlock(); - nfs_finish_clear_delegation_stateid(state); + nfs_finish_clear_delegation_stateid(state, &stateid); return; } - nfs4_stateid_copy(&stateid, &delegation->stateid); cred = get_rpccred(delegation->cred); rcu_read_unlock(); status = nfs41_test_and_free_expired_stateid(server, &stateid, cred); trace_nfs4_test_delegation_stateid(state, NULL, status); if (status != NFS_OK) - nfs_finish_clear_delegation_stateid(state); + nfs_finish_clear_delegation_stateid(state, &stateid); put_rpccred(cred); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 06/12] NFSv4.1: Test delegation stateids when server declares "some state revoked" 2016-09-09 18:04 ` [PATCH v3 05/12] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 07/12] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: Oleg Drokin According to RFC5661, if any of the SEQUENCE status bits SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED, SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED, SEQ4_STATUS_ADMIN_STATE_REVOKED, or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to use TEST_STATEID to figure out which stateids have been revoked, so we can acknowledge the loss of state using FREE_STATEID. While we already do this for open and lock state, we have not been doing so for all the delegations. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++- fs/nfs/delegation.h | 4 +++ fs/nfs/nfs4_fs.h | 3 ++ fs/nfs/nfs4proc.c | 10 ++++++ fs/nfs/nfs4state.c | 17 +++++----- 5 files changed, 122 insertions(+), 9 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 76dc11ecdedd..ff0c1a0feb9d 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -802,8 +802,15 @@ static void nfs_delegation_mark_reclaim_server(struct nfs_server *server) { struct nfs_delegation *delegation; - list_for_each_entry_rcu(delegation, &server->delegations, super_list) + list_for_each_entry_rcu(delegation, &server->delegations, super_list) { + /* + * If the delegation may have been admin revoked, then we + * cannot reclaim it. + */ + if (test_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags)) + continue; set_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags); + } } /** @@ -867,6 +874,94 @@ restart: rcu_read_unlock(); } +static void nfs_mark_test_expired_delegation(struct nfs_server *server, + struct nfs_delegation *delegation) +{ + clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags); + set_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags); + set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state); +} + +static void nfs_delegation_mark_test_expired_server(struct nfs_server *server) +{ + struct nfs_delegation *delegation; + + list_for_each_entry_rcu(delegation, &server->delegations, super_list) + nfs_mark_test_expired_delegation(server, delegation); +} + +/** + * nfs_mark_test_expired_all_delegations - mark all delegations for testing + * @clp: nfs_client to process + * + * Iterates through all the delegations associated with this server and + * marks them as needing to be checked for validity. + */ +void nfs_mark_test_expired_all_delegations(struct nfs_client *clp) +{ + struct nfs_server *server; + + rcu_read_lock(); + list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) + nfs_delegation_mark_test_expired_server(server); + rcu_read_unlock(); +} + +/** + * nfs_reap_expired_delegations - reap expired delegations + * @clp: nfs_client to process + * + * Iterates through all the delegations associated with this server and + * checks if they have may have been revoked. This function is usually + * expected to be called in cases where the server may have lost its + * lease. + */ +void nfs_reap_expired_delegations(struct nfs_client *clp) +{ + const struct nfs4_minor_version_ops *ops = clp->cl_mvops; + struct nfs_delegation *delegation; + struct nfs_server *server; + struct inode *inode; + struct rpc_cred *cred; + nfs4_stateid stateid; + +restart: + rcu_read_lock(); + list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) { + list_for_each_entry_rcu(delegation, &server->delegations, + super_list) { + if (test_bit(NFS_DELEGATION_RETURNING, + &delegation->flags)) + continue; + if (test_bit(NFS_DELEGATION_TEST_EXPIRED, + &delegation->flags) == 0) + continue; + if (!nfs_sb_active(server->super)) + continue; + inode = nfs_delegation_grab_inode(delegation); + if (inode == NULL) { + rcu_read_unlock(); + nfs_sb_deactive(server->super); + goto restart; + } + cred = get_rpccred_rcu(delegation->cred); + nfs4_stateid_copy(&stateid, &delegation->stateid); + clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags); + rcu_read_unlock(); + if (cred != NULL && + ops->test_and_free_expired(server, &stateid, cred) < 0) { + nfs_revoke_delegation(inode, &stateid); + nfs_inode_find_state_and_recover(inode, &stateid); + } + put_rpccred(cred); + iput(inode); + nfs_sb_deactive(server->super); + goto restart; + } + } + rcu_read_unlock(); +} + /** * nfs_delegations_present - check for existence of delegations * @clp: client state handle diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index d40827af5913..1442e3b1521d 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -32,6 +32,7 @@ enum { NFS_DELEGATION_REFERENCED, NFS_DELEGATION_RETURNING, NFS_DELEGATION_REVOKED, + NFS_DELEGATION_TEST_EXPIRED, }; int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res); @@ -52,6 +53,9 @@ void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid) void nfs_delegation_mark_reclaim(struct nfs_client *clp); void nfs_delegation_reap_unclaimed(struct nfs_client *clp); +void nfs_mark_test_expired_all_delegations(struct nfs_client *clp); +void nfs_reap_expired_delegations(struct nfs_client *clp); + /* NFSv4 delegation-related procedures */ int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync); int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type); diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index 4390d73a92e5..b9083a6cefd6 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -39,6 +39,7 @@ enum nfs4_client_state { NFS4CLNT_BIND_CONN_TO_SESSION, NFS4CLNT_MOVED, NFS4CLNT_LEASE_MOVED, + NFS4CLNT_DELEGATION_EXPIRED, }; #define NFS4_RENEW_TIMEOUT 0x01 @@ -57,6 +58,8 @@ struct nfs4_minor_version_ops { struct nfs_fsinfo *); void (*free_lock_state)(struct nfs_server *, struct nfs4_lock_state *); + int (*test_and_free_expired)(struct nfs_server *, + nfs4_stateid *, struct rpc_cred *); struct nfs_seqid * (*alloc_seqid)(struct nfs_seqid_counter *, gfp_t); const struct rpc_call_ops *call_sync_ops; diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 0ae39e3b13e1..74dd9157539a 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2408,6 +2408,13 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st return nfs4_open_expired(sp, state); } +static int nfs40_test_and_free_expired_stateid(struct nfs_server *server, + nfs4_stateid *stateid, + struct rpc_cred *cred) +{ + return -NFS4ERR_BAD_STATEID; +} + #if defined(CONFIG_NFS_V4_1) static int nfs41_test_and_free_expired_stateid(struct nfs_server *server, nfs4_stateid *stateid, @@ -8854,6 +8861,7 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = { .match_stateid = nfs4_match_stateid, .find_root_sec = nfs4_find_root_sec, .free_lock_state = nfs4_release_lockowner, + .test_and_free_expired = nfs40_test_and_free_expired_stateid, .alloc_seqid = nfs_alloc_seqid, .call_sync_ops = &nfs40_call_sync_ops, .reboot_recovery_ops = &nfs40_reboot_recovery_ops, @@ -8881,6 +8889,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = { .match_stateid = nfs41_match_stateid, .find_root_sec = nfs41_find_root_sec, .free_lock_state = nfs41_free_lock_state, + .test_and_free_expired = nfs41_test_and_free_expired_stateid, .alloc_seqid = nfs_alloc_no_seqid, .call_sync_ops = &nfs41_call_sync_ops, .reboot_recovery_ops = &nfs41_reboot_recovery_ops, @@ -8910,6 +8919,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = { .find_root_sec = nfs41_find_root_sec, .free_lock_state = nfs41_free_lock_state, .call_sync_ops = &nfs41_call_sync_ops, + .test_and_free_expired = nfs41_test_and_free_expired_stateid, .alloc_seqid = nfs_alloc_no_seqid, .reboot_recovery_ops = &nfs41_reboot_recovery_ops, .nograce_recovery_ops = &nfs41_nograce_recovery_ops, diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 9801b5bb5fac..63da0411e2af 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1656,15 +1656,9 @@ static void nfs4_state_end_reclaim_reboot(struct nfs_client *clp) put_rpccred(cred); } -static void nfs_delegation_clear_all(struct nfs_client *clp) -{ - nfs_delegation_mark_reclaim(clp); - nfs_delegation_reap_unclaimed(clp); -} - static void nfs4_state_start_reclaim_nograce(struct nfs_client *clp) { - nfs_delegation_clear_all(clp); + nfs_mark_test_expired_all_delegations(clp); nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce); } @@ -2195,7 +2189,7 @@ static void nfs41_handle_all_state_revoked(struct nfs_client *clp) static void nfs41_handle_some_state_revoked(struct nfs_client *clp) { - nfs4_state_mark_reclaim_helper(clp, nfs4_state_mark_reclaim_nograce); + nfs4_state_start_reclaim_nograce(clp); nfs4_schedule_state_manager(clp); dprintk("%s: state revoked on server %s\n", __func__, clp->cl_hostname); @@ -2420,6 +2414,13 @@ static void nfs4_state_manager(struct nfs_client *clp) nfs4_state_end_reclaim_reboot(clp); } + /* Detect expired delegations... */ + if (test_and_clear_bit(NFS4CLNT_DELEGATION_EXPIRED, &clp->cl_state)) { + section = "detect expired delegations"; + nfs_reap_expired_delegations(clp); + continue; + } + /* Now recover expired state... */ if (test_and_clear_bit(NFS4CLNT_RECLAIM_NOGRACE, &clp->cl_state)) { section = "reclaim nograce"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 07/12] NFSv4.1: Deal with server reboots during delegation expiration recovery 2016-09-09 18:04 ` [PATCH v3 06/12] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 08/12] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: Oleg Drokin Ensure that if the server reboots while we're testing and recovering from revoked delegations, we exit to allow the state manager to handle matters. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index ff0c1a0feb9d..b873271c613a 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -874,6 +874,13 @@ restart: rcu_read_unlock(); } +static inline bool nfs4_server_rebooted(const struct nfs_client *clp) +{ + return (clp->cl_state & (BIT(NFS4CLNT_CHECK_LEASE) | + BIT(NFS4CLNT_LEASE_EXPIRED) | + BIT(NFS4CLNT_SESSION_RESET))) != 0; +} + static void nfs_mark_test_expired_delegation(struct nfs_server *server, struct nfs_delegation *delegation) { @@ -882,6 +889,19 @@ static void nfs_mark_test_expired_delegation(struct nfs_server *server, set_bit(NFS4CLNT_DELEGATION_EXPIRED, &server->nfs_client->cl_state); } +static void nfs_inode_mark_test_expired_delegation(struct nfs_server *server, + struct inode *inode) +{ + struct nfs_delegation *delegation; + + rcu_read_lock(); + delegation = rcu_dereference(NFS_I(inode)->delegation); + if (delegation) + nfs_mark_test_expired_delegation(server, delegation); + rcu_read_unlock(); + +} + static void nfs_delegation_mark_test_expired_server(struct nfs_server *server) { struct nfs_delegation *delegation; @@ -954,6 +974,12 @@ restart: nfs_inode_find_state_and_recover(inode, &stateid); } put_rpccred(cred); + if (nfs4_server_rebooted(clp)) { + nfs_inode_mark_test_expired_delegation(server,inode); + iput(inode); + nfs_sb_deactive(server->super); + return; + } iput(inode); nfs_sb_deactive(server->super); goto restart; -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 08/12] NFSv4.1: Don't recheck delegations that have already been checked 2016-09-09 18:04 ` [PATCH v3 07/12] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 09/12] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 74dd9157539a..6cae73a08ba0 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2459,6 +2459,11 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) return; } + if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED, &delegation->flags)) { + rcu_read_unlock(); + return; + } + cred = get_rpccred(delegation->cred); rcu_read_unlock(); status = nfs41_test_and_free_expired_stateid(server, &stateid, cred); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 09/12] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID 2016-09-09 18:04 ` [PATCH v3 08/12] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 10/12] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 6cae73a08ba0..6484148f1c3d 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2422,18 +2422,29 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server, { int status; - status = nfs41_test_stateid(server, stateid, cred); + switch (stateid->type) { + default: + break; + case NFS4_INVALID_STATEID_TYPE: + case NFS4_SPECIAL_STATEID_TYPE: + return -NFS4ERR_BAD_STATEID; + case NFS4_REVOKED_STATEID_TYPE: + goto out_free; + } + status = nfs41_test_stateid(server, stateid, cred); switch (status) { case -NFS4ERR_EXPIRED: case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_DELEG_REVOKED: - /* Ack the revoked state to the server */ - nfs41_free_stateid(server, stateid, cred); - case -NFS4ERR_BAD_STATEID: + break; + default: return status; } - return NFS_OK; +out_free: + /* Ack the revoked state to the server */ + nfs41_free_stateid(server, stateid, cred); + return -NFS4ERR_EXPIRED; } static void nfs41_check_delegation_stateid(struct nfs4_state *state) @@ -2468,7 +2479,7 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) rcu_read_unlock(); status = nfs41_test_and_free_expired_stateid(server, &stateid, cred); trace_nfs4_test_delegation_stateid(state, NULL, status); - if (status != NFS_OK) + if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) nfs_finish_clear_delegation_stateid(state, &stateid); put_rpccred(cred); @@ -2497,7 +2508,7 @@ static int nfs41_check_open_stateid(struct nfs4_state *state) status = nfs41_test_and_free_expired_stateid(server, stateid, cred); trace_nfs4_test_open_stateid(state, NULL, status); - if (status != NFS_OK) { + if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) { clear_bit(NFS_O_RDONLY_STATE, &state->flags); clear_bit(NFS_O_WRONLY_STATE, &state->flags); clear_bit(NFS_O_RDWR_STATE, &state->flags); @@ -6124,7 +6135,7 @@ out: */ static int nfs41_check_expired_locks(struct nfs4_state *state) { - int status, ret = -NFS4ERR_BAD_STATEID; + int status, ret = NFS_OK; struct nfs4_lock_state *lsp; struct nfs_server *server = NFS_SERVER(state->inode); @@ -6136,9 +6147,12 @@ static int nfs41_check_expired_locks(struct nfs4_state *state) &lsp->ls_stateid, cred); trace_nfs4_test_lock_stateid(state, lsp, status); - if (status != NFS_OK) { + if (status == -NFS4ERR_EXPIRED || + status == -NFS4ERR_BAD_STATEID) clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags); + else if (status != NFS_OK) { ret = status; + break; } } }; diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index c6564ada9beb..9094faf0699d 100644 --- a/include/linux/nfs4.h +++ b/include/linux/nfs4.h @@ -67,6 +67,7 @@ struct nfs4_stateid_struct { NFS4_DELEGATION_STATEID_TYPE, NFS4_LAYOUT_STATEID_TYPE, NFS4_PNFS_DS_STATEID_TYPE, + NFS4_REVOKED_STATEID_TYPE, } type; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 10/12] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks 2016-09-09 18:04 ` [PATCH v3 09/12] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 11/12] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 6484148f1c3d..c7e9f91c5cfb 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2486,6 +2486,45 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state) } /** + * nfs41_check_expired_locks - possibly free a lock stateid + * + * @state: NFSv4 state for an inode + * + * Returns NFS_OK if recovery for this stateid is now finished. + * Otherwise a negative NFS4ERR value is returned. + */ +static int nfs41_check_expired_locks(struct nfs4_state *state) +{ + int status, ret = NFS_OK; + struct nfs4_lock_state *lsp; + struct nfs_server *server = NFS_SERVER(state->inode); + + if (!test_bit(LK_STATE_IN_USE, &state->flags)) + goto out; + list_for_each_entry(lsp, &state->lock_states, ls_locks) { + if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) { + struct rpc_cred *cred = lsp->ls_state->owner->so_cred; + + status = nfs41_test_and_free_expired_stateid(server, + &lsp->ls_stateid, + cred); + trace_nfs4_test_lock_stateid(state, lsp, status); + if (status == -NFS4ERR_EXPIRED || + status == -NFS4ERR_BAD_STATEID) { + clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags); + if (!recover_lost_locks) + set_bit(NFS_LOCK_LOST, &lsp->ls_flags); + } else if (status != NFS_OK) { + ret = status; + break; + } + } + }; +out: + return ret; +} + +/** * nfs41_check_open_stateid - possibly free an open stateid * * @state: NFSv4 state for an inode @@ -2522,6 +2561,9 @@ static int nfs41_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st int status; nfs41_check_delegation_stateid(state); + status = nfs41_check_expired_locks(state); + if (status != NFS_OK) + return status; status = nfs41_check_open_stateid(state); if (status != NFS_OK) status = nfs4_open_expired(sp, state); @@ -6125,49 +6167,19 @@ out: } #if defined(CONFIG_NFS_V4_1) -/** - * nfs41_check_expired_locks - possibly free a lock stateid - * - * @state: NFSv4 state for an inode - * - * Returns NFS_OK if recovery for this stateid is now finished. - * Otherwise a negative NFS4ERR value is returned. - */ -static int nfs41_check_expired_locks(struct nfs4_state *state) -{ - int status, ret = NFS_OK; - struct nfs4_lock_state *lsp; - struct nfs_server *server = NFS_SERVER(state->inode); - - list_for_each_entry(lsp, &state->lock_states, ls_locks) { - if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) { - struct rpc_cred *cred = lsp->ls_state->owner->so_cred; - - status = nfs41_test_and_free_expired_stateid(server, - &lsp->ls_stateid, - cred); - trace_nfs4_test_lock_stateid(state, lsp, status); - if (status == -NFS4ERR_EXPIRED || - status == -NFS4ERR_BAD_STATEID) - clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags); - else if (status != NFS_OK) { - ret = status; - break; - } - } - }; - - return ret; -} - static int nfs41_lock_expired(struct nfs4_state *state, struct file_lock *request) { - int status = NFS_OK; + struct nfs4_lock_state *lsp; + int status; - if (test_bit(LK_STATE_IN_USE, &state->flags)) - status = nfs41_check_expired_locks(state); - if (status != NFS_OK) - status = nfs4_lock_expired(state, request); + status = nfs4_set_lock_state(state, request); + if (status != 0) + return status; + lsp = request->fl_u.nfs4_fl.owner; + if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) || + test_bit(NFS_LOCK_LOST, &lsp->ls_flags)) + return 0; + status = nfs4_lock_expired(state, request); return status; } #endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 11/12] NFSv4.1: FREE_STATEID can be asynchronous 2016-09-09 18:04 ` [PATCH v3 10/12] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 12/12] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust 0 siblings, 1 reply; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 c7e9f91c5cfb..5442e0e1de14 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -99,8 +99,8 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, #ifdef CONFIG_NFS_V4_1 static int nfs41_test_stateid(struct nfs_server *, nfs4_stateid *, struct rpc_cred *); -static int nfs41_free_stateid(struct nfs_server *, nfs4_stateid *, - struct rpc_cred *); +static int nfs41_free_stateid(struct nfs_server *, const nfs4_stateid *, + struct rpc_cred *, bool); #endif #ifdef CONFIG_NFS_V4_SECURITY_LABEL @@ -2443,7 +2443,7 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server, } out_free: /* Ack the revoked state to the server */ - nfs41_free_stateid(server, stateid, cred); + nfs41_free_stateid(server, stateid, cred, true); return -NFS4ERR_EXPIRED; } @@ -8714,7 +8714,7 @@ static const struct rpc_call_ops nfs41_free_stateid_ops = { }; static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server, - nfs4_stateid *stateid, + const nfs4_stateid *stateid, struct rpc_cred *cred, bool privileged) { @@ -8757,38 +8757,31 @@ static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server, * @server: server / transport on which to perform the operation * @stateid: state ID to release * @cred: credential + * @is_recovery: set to true if this call needs to be privileged * - * Returns NFS_OK if the server freed "stateid". Otherwise a - * negative NFS4ERR value is returned. + * Note: this function is always asynchronous. */ static int nfs41_free_stateid(struct nfs_server *server, - nfs4_stateid *stateid, - struct rpc_cred *cred) + const nfs4_stateid *stateid, + struct rpc_cred *cred, + bool is_recovery) { struct rpc_task *task; - int ret; - task = _nfs41_free_stateid(server, stateid, cred, true); + task = _nfs41_free_stateid(server, stateid, cred, is_recovery); if (IS_ERR(task)) return PTR_ERR(task); - ret = rpc_wait_for_completion_task(task); - if (!ret) - ret = task->tk_status; rpc_put_task(task); - return ret; + return 0; } static void nfs41_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp) { - struct rpc_task *task; struct rpc_cred *cred = lsp->ls_state->owner->so_cred; - task = _nfs41_free_stateid(server, &lsp->ls_stateid, cred, false); + nfs41_free_stateid(server, &lsp->ls_stateid, cred, false); nfs4_free_lock_state(server, lsp); - if (IS_ERR(task)) - return; - rpc_put_task(task); } static bool nfs41_match_stateid(const nfs4_stateid *s1, -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 12/12] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku 2016-09-09 18:04 ` [PATCH v3 11/12] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust @ 2016-09-09 18:04 ` Trond Myklebust 0 siblings, 0 replies; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:04 UTC (permalink / raw) To: linux-nfs; +Cc: 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 | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 5442e0e1de14..ffb3218a4a00 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -328,6 +328,18 @@ static void nfs4_setup_readdir(u64 cookie, __be32 *verifier, struct dentry *dent kunmap_atomic(start); } +static void nfs4_free_revoked_stateid(struct nfs_server *server, + const nfs4_stateid *stateid, + struct rpc_cred *cred) +{ + const struct nfs4_minor_version_ops *ops = server->nfs_client->cl_mvops; + nfs4_stateid tmp; + + nfs4_stateid_copy(&tmp, stateid); + tmp.type = NFS4_REVOKED_STATEID_TYPE; + ops->test_and_free_expired(server, &tmp, cred); +} + static long nfs4_update_delay(long *timeout) { long ret; @@ -2983,9 +2995,12 @@ static void nfs4_close_done(struct rpc_task *task, void *data) break; case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_STALE_STATEID: + case -NFS4ERR_EXPIRED: + nfs4_free_revoked_stateid(server, + &calldata->arg.stateid, + task->tk_msg.rpc_cred); case -NFS4ERR_OLD_STATEID: case -NFS4ERR_BAD_STATEID: - case -NFS4ERR_EXPIRED: if (!nfs4_stateid_match(&calldata->arg.stateid, &state->open_stateid)) { rpc_restart_call_prepare(task); @@ -5477,10 +5492,13 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) renew_lease(data->res.server, data->timestamp); case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_DELEG_REVOKED: + case -NFS4ERR_EXPIRED: + nfs4_free_revoked_stateid(data->res.server, + data->args.stateid, + task->tk_msg.rpc_cred); case -NFS4ERR_BAD_STATEID: case -NFS4ERR_OLD_STATEID: case -NFS4ERR_STALE_STATEID: - case -NFS4ERR_EXPIRED: task->tk_status = 0; if (data->roc) pnfs_roc_set_barrier(data->inode, data->roc_barrier); @@ -5745,10 +5763,14 @@ static void nfs4_locku_done(struct rpc_task *task, void *data) if (nfs4_update_lock_stateid(calldata->lsp, &calldata->res.stateid)) break; + case -NFS4ERR_ADMIN_REVOKED: + case -NFS4ERR_EXPIRED: + nfs4_free_revoked_stateid(calldata->server, + &calldata->arg.stateid, + task->tk_msg.rpc_cred); case -NFS4ERR_BAD_STATEID: case -NFS4ERR_OLD_STATEID: case -NFS4ERR_STALE_STATEID: - case -NFS4ERR_EXPIRED: if (!nfs4_stateid_match(&calldata->arg.stateid, &calldata->lsp->ls_stateid)) rpc_restart_call_prepare(task); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags 2016-09-09 18:04 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust @ 2016-09-09 18:26 ` Olga Kornievskaia 2016-09-09 18:32 ` Trond Myklebust 1 sibling, 1 reply; 16+ messages in thread From: Olga Kornievskaia @ 2016-09-09 18:26 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs, Oleg Drokin On Fri, Sep 9, 2016 at 2:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > As described in RFC5661, section 18.46, some of the status flags exist > in order to tell the client when it needs to acknowledge the existence of > revoked state on the server and/or to recover state. > Those flags will then remain set until the recovery procedure is done. > > In order to avoid looping, the client therefore needs to ignore > those particular flags while recovering. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/nfs4_fs.h | 2 +- > fs/nfs/nfs4proc.c | 5 ++++- > fs/nfs/nfs4session.h | 1 + > fs/nfs/nfs4state.c | 12 +++++++++++- > 4 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > index f230aa62ca59..4390d73a92e5 100644 > --- a/fs/nfs/nfs4_fs.h > +++ b/fs/nfs/nfs4_fs.h > @@ -439,7 +439,7 @@ extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp); > extern int nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *); > extern int nfs4_schedule_migration_recovery(const struct nfs_server *); > extern void nfs4_schedule_lease_moved_recovery(struct nfs_client *); > -extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags); > +extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags, bool); > extern void nfs41_handle_server_scope(struct nfs_client *, > struct nfs41_server_scope **); > extern void nfs4_put_lock_state(struct nfs4_lock_state *lsp); > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 25a2af707233..bd33777f03c4 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -616,6 +616,7 @@ int nfs40_setup_sequence(struct nfs4_slot_table *tbl, > } > spin_unlock(&tbl->slot_tbl_lock); > > + slot->privileged = args->sa_privileged ? 1 : 0; > args->sa_slot = slot; > res->sr_slot = slot; > > @@ -728,7 +729,8 @@ static int nfs41_sequence_process(struct rpc_task *task, > clp = session->clp; > do_renew_lease(clp, res->sr_timestamp); > /* Check sequence flags */ > - nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags); > + nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags, > + !!slot->privileged); Is this suppose to be "!!"? > nfs41_update_target_slotid(slot->table, slot, res); > break; > case 1: > @@ -875,6 +877,7 @@ int nfs41_setup_sequence(struct nfs4_session *session, > } > spin_unlock(&tbl->slot_tbl_lock); > > + slot->privileged = args->sa_privileged ? 1 : 0; > args->sa_slot = slot; > > dprintk("<-- %s slotid=%u seqid=%u\n", __func__, > diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h > index 3bb6af70973c..dae385500005 100644 > --- a/fs/nfs/nfs4session.h > +++ b/fs/nfs/nfs4session.h > @@ -23,6 +23,7 @@ struct nfs4_slot { > u32 slot_nr; > u32 seq_nr; > unsigned int interrupted : 1, > + privileged : 1, > seq_done : 1; > }; > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > index cada00aa5096..9801b5bb5fac 100644 > --- a/fs/nfs/nfs4state.c > +++ b/fs/nfs/nfs4state.c > @@ -2227,13 +2227,22 @@ static void nfs41_handle_cb_path_down(struct nfs_client *clp) > nfs4_schedule_state_manager(clp); > } > > -void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags) > +void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags, > + bool recovery) > { > if (!flags) > return; > > dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n", > __func__, clp->cl_hostname, clp->cl_clientid, flags); > + /* > + * If we're called from the state manager thread, then assume we're > + * already handling the RECLAIM_NEEDED and/or STATE_REVOKED. > + * Those flags are expected to remain set until we're done > + * recovering (see RFC5661, section 18.46.3). > + */ > + if (recovery) > + goto out_recovery; > > if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED) > nfs41_handle_server_reboot(clp); > @@ -2246,6 +2255,7 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags) > nfs4_schedule_lease_moved_recovery(clp); > if (flags & SEQ4_STATUS_RECALLABLE_STATE_REVOKED) > nfs41_handle_recallable_state_revoked(clp); > +out_recovery: > if (flags & SEQ4_STATUS_BACKCHANNEL_FAULT) > nfs41_handle_backchannel_fault(clp); > else if (flags & (SEQ4_STATUS_CB_PATH_DOWN | > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags 2016-09-09 18:26 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Olga Kornievskaia @ 2016-09-09 18:32 ` Trond Myklebust 0 siblings, 0 replies; 16+ messages in thread From: Trond Myklebust @ 2016-09-09 18:32 UTC (permalink / raw) To: Kornievskaia Olga; +Cc: Trond Myklebust, List Linux NFS Mailing, Oleg Drokin DQo+IE9uIFNlcCA5LCAyMDE2LCBhdCAxNDoyNiwgT2xnYSBLb3JuaWV2c2thaWEgPGFnbG9AdW1p Y2guZWR1PiB3cm90ZToNCj4gDQo+IE9uIEZyaSwgU2VwIDksIDIwMTYgYXQgMjowNCBQTSwgVHJv bmQgTXlrbGVidXN0DQo+IDx0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tPiB3cm90ZToN Cj4+IEFzIGRlc2NyaWJlZCBpbiBSRkM1NjYxLCBzZWN0aW9uIDE4LjQ2LCBzb21lIG9mIHRoZSBz dGF0dXMgZmxhZ3MgZXhpc3QNCj4+IGluIG9yZGVyIHRvIHRlbGwgdGhlIGNsaWVudCB3aGVuIGl0 IG5lZWRzIHRvIGFja25vd2xlZGdlIHRoZSBleGlzdGVuY2Ugb2YNCj4+IHJldm9rZWQgc3RhdGUg b24gdGhlIHNlcnZlciBhbmQvb3IgdG8gcmVjb3ZlciBzdGF0ZS4NCj4+IFRob3NlIGZsYWdzIHdp bGwgdGhlbiByZW1haW4gc2V0IHVudGlsIHRoZSByZWNvdmVyeSBwcm9jZWR1cmUgaXMgZG9uZS4N Cj4+IA0KPj4gSW4gb3JkZXIgdG8gYXZvaWQgbG9vcGluZywgdGhlIGNsaWVudCB0aGVyZWZvcmUg bmVlZHMgdG8gaWdub3JlDQo+PiB0aG9zZSBwYXJ0aWN1bGFyIGZsYWdzIHdoaWxlIHJlY292ZXJp bmcuDQo+PiANCj4+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVi dXN0QHByaW1hcnlkYXRhLmNvbT4NCj4+IC0tLQ0KPj4gZnMvbmZzL25mczRfZnMuaCAgICAgfCAg MiArLQ0KPj4gZnMvbmZzL25mczRwcm9jLmMgICAgfCAgNSArKysrLQ0KPj4gZnMvbmZzL25mczRz ZXNzaW9uLmggfCAgMSArDQo+PiBmcy9uZnMvbmZzNHN0YXRlLmMgICB8IDEyICsrKysrKysrKysr LQ0KPj4gNCBmaWxlcyBjaGFuZ2VkLCAxNyBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQ0K Pj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRfZnMuaCBiL2ZzL25mcy9uZnM0X2ZzLmgN Cj4+IGluZGV4IGYyMzBhYTYyY2E1OS4uNDM5MGQ3M2E5MmU1IDEwMDY0NA0KPj4gLS0tIGEvZnMv bmZzL25mczRfZnMuaA0KPj4gKysrIGIvZnMvbmZzL25mczRfZnMuaA0KPj4gQEAgLTQzOSw3ICs0 MzksNyBAQCBleHRlcm4gdm9pZCBuZnM0X3NjaGVkdWxlX3BhdGhfZG93bl9yZWNvdmVyeShzdHJ1 Y3QgbmZzX2NsaWVudCAqY2xwKTsNCj4+IGV4dGVybiBpbnQgbmZzNF9zY2hlZHVsZV9zdGF0ZWlk X3JlY292ZXJ5KGNvbnN0IHN0cnVjdCBuZnNfc2VydmVyICosIHN0cnVjdCBuZnM0X3N0YXRlICop Ow0KPj4gZXh0ZXJuIGludCBuZnM0X3NjaGVkdWxlX21pZ3JhdGlvbl9yZWNvdmVyeShjb25zdCBz dHJ1Y3QgbmZzX3NlcnZlciAqKTsNCj4+IGV4dGVybiB2b2lkIG5mczRfc2NoZWR1bGVfbGVhc2Vf bW92ZWRfcmVjb3Zlcnkoc3RydWN0IG5mc19jbGllbnQgKik7DQo+PiAtZXh0ZXJuIHZvaWQgbmZz NDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHUz MiBmbGFncyk7DQo+PiArZXh0ZXJuIHZvaWQgbmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJy b3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHUzMiBmbGFncywgYm9vbCk7DQo+PiBleHRlcm4g dm9pZCBuZnM0MV9oYW5kbGVfc2VydmVyX3Njb3BlKHN0cnVjdCBuZnNfY2xpZW50ICosDQo+PiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IG5mczQxX3NlcnZlcl9z Y29wZSAqKik7DQo+PiBleHRlcm4gdm9pZCBuZnM0X3B1dF9sb2NrX3N0YXRlKHN0cnVjdCBuZnM0 X2xvY2tfc3RhdGUgKmxzcCk7DQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRwcm9jLmMgYi9m cy9uZnMvbmZzNHByb2MuYw0KPj4gaW5kZXggMjVhMmFmNzA3MjMzLi5iZDMzNzc3ZjAzYzQgMTAw NjQ0DQo+PiAtLS0gYS9mcy9uZnMvbmZzNHByb2MuYw0KPj4gKysrIGIvZnMvbmZzL25mczRwcm9j LmMNCj4+IEBAIC02MTYsNiArNjE2LDcgQEAgaW50IG5mczQwX3NldHVwX3NlcXVlbmNlKHN0cnVj dCBuZnM0X3Nsb3RfdGFibGUgKnRibCwNCj4+ICAgICAgICB9DQo+PiAgICAgICAgc3Bpbl91bmxv Y2soJnRibC0+c2xvdF90YmxfbG9jayk7DQo+PiANCj4+ICsgICAgICAgc2xvdC0+cHJpdmlsZWdl ZCA9IGFyZ3MtPnNhX3ByaXZpbGVnZWQgPyAxIDogMDsNCj4+ICAgICAgICBhcmdzLT5zYV9zbG90 ID0gc2xvdDsNCj4+ICAgICAgICByZXMtPnNyX3Nsb3QgPSBzbG90Ow0KPj4gDQo+PiBAQCAtNzI4 LDcgKzcyOSw4IEBAIHN0YXRpYyBpbnQgbmZzNDFfc2VxdWVuY2VfcHJvY2VzcyhzdHJ1Y3QgcnBj X3Rhc2sgKnRhc2ssDQo+PiAgICAgICAgICAgICAgICBjbHAgPSBzZXNzaW9uLT5jbHA7DQo+PiAg ICAgICAgICAgICAgICBkb19yZW5ld19sZWFzZShjbHAsIHJlcy0+c3JfdGltZXN0YW1wKTsNCj4+ ICAgICAgICAgICAgICAgIC8qIENoZWNrIHNlcXVlbmNlIGZsYWdzICovDQo+PiAtICAgICAgICAg ICAgICAgbmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKGNscCwgcmVzLT5zcl9zdGF0 dXNfZmxhZ3MpOw0KPj4gKyAgICAgICAgICAgICAgIG5mczQxX2hhbmRsZV9zZXF1ZW5jZV9mbGFn X2Vycm9ycyhjbHAsIHJlcy0+c3Jfc3RhdHVzX2ZsYWdzLA0KPj4gKyAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAhIXNsb3QtPnByaXZpbGVnZWQpOw0KPiANCj4gSXMgdGhpcyBzdXBwb3Nl IHRvIGJlICIhIeKAnT8NCg0KWWVzLiBJdCBjb252ZXJ0cyB0byBhIGJvb2xlYW4uDQoNCj4gDQo+ PiAgICAgICAgICAgICAgICBuZnM0MV91cGRhdGVfdGFyZ2V0X3Nsb3RpZChzbG90LT50YWJsZSwg c2xvdCwgcmVzKTsNCj4+ICAgICAgICAgICAgICAgIGJyZWFrOw0KPj4gICAgICAgIGNhc2UgMToN Cj4+IEBAIC04NzUsNiArODc3LDcgQEAgaW50IG5mczQxX3NldHVwX3NlcXVlbmNlKHN0cnVjdCBu ZnM0X3Nlc3Npb24gKnNlc3Npb24sDQo+PiAgICAgICAgfQ0KPj4gICAgICAgIHNwaW5fdW5sb2Nr KCZ0YmwtPnNsb3RfdGJsX2xvY2spOw0KPj4gDQo+PiArICAgICAgIHNsb3QtPnByaXZpbGVnZWQg PSBhcmdzLT5zYV9wcml2aWxlZ2VkID8gMSA6IDA7DQo+PiAgICAgICAgYXJncy0+c2Ffc2xvdCA9 IHNsb3Q7DQo+PiANCj4+ICAgICAgICBkcHJpbnRrKCI8LS0gJXMgc2xvdGlkPSV1IHNlcWlkPSV1 XG4iLCBfX2Z1bmNfXywNCj4+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNHNlc3Npb24uaCBiL2Zz L25mcy9uZnM0c2Vzc2lvbi5oDQo+PiBpbmRleCAzYmI2YWY3MDk3M2MuLmRhZTM4NTUwMDAwNSAx MDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9uZnM0c2Vzc2lvbi5oDQo+PiArKysgYi9mcy9uZnMvbmZz NHNlc3Npb24uaA0KPj4gQEAgLTIzLDYgKzIzLDcgQEAgc3RydWN0IG5mczRfc2xvdCB7DQo+PiAg ICAgICAgdTMyICAgICAgICAgICAgICAgICAgICAgc2xvdF9ucjsNCj4+ICAgICAgICB1MzIgICAg ICAgICAgICAgICAgICAgICBzZXFfbnI7DQo+PiAgICAgICAgdW5zaWduZWQgaW50ICAgICAgICAg ICAgaW50ZXJydXB0ZWQgOiAxLA0KPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBw cml2aWxlZ2VkIDogMSwNCj4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzZXFfZG9u ZSA6IDE7DQo+PiB9Ow0KPj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL25mczRzdGF0ZS5jIGIv ZnMvbmZzL25mczRzdGF0ZS5jDQo+PiBpbmRleCBjYWRhMDBhYTUwOTYuLjk4MDFiNWJiNWZhYyAx MDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9uZnM0c3RhdGUuYw0KPj4gKysrIGIvZnMvbmZzL25mczRz dGF0ZS5jDQo+PiBAQCAtMjIyNywxMyArMjIyNywyMiBAQCBzdGF0aWMgdm9pZCBuZnM0MV9oYW5k bGVfY2JfcGF0aF9kb3duKHN0cnVjdCBuZnNfY2xpZW50ICpjbHApDQo+PiAgICAgICAgICAgICAg ICBuZnM0X3NjaGVkdWxlX3N0YXRlX21hbmFnZXIoY2xwKTsNCj4+IH0NCj4+IA0KPj4gLXZvaWQg bmZzNDFfaGFuZGxlX3NlcXVlbmNlX2ZsYWdfZXJyb3JzKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAs IHUzMiBmbGFncykNCj4+ICt2b2lkIG5mczQxX2hhbmRsZV9zZXF1ZW5jZV9mbGFnX2Vycm9ycyhz dHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCB1MzIgZmxhZ3MsDQo+PiArICAgICAgICAgICAgICAgYm9v bCByZWNvdmVyeSkNCj4+IHsNCj4+ICAgICAgICBpZiAoIWZsYWdzKQ0KPj4gICAgICAgICAgICAg ICAgcmV0dXJuOw0KPj4gDQo+PiAgICAgICAgZHByaW50aygiJXM6IFwiJXNcIiAoY2xpZW50IElE ICVsbHgpIGZsYWdzPTB4JTA4eFxuIiwNCj4+ICAgICAgICAgICAgICAgIF9fZnVuY19fLCBjbHAt PmNsX2hvc3RuYW1lLCBjbHAtPmNsX2NsaWVudGlkLCBmbGFncyk7DQo+PiArICAgICAgIC8qDQo+ PiArICAgICAgICAqIElmIHdlJ3JlIGNhbGxlZCBmcm9tIHRoZSBzdGF0ZSBtYW5hZ2VyIHRocmVh ZCwgdGhlbiBhc3N1bWUgd2UncmUNCj4+ICsgICAgICAgICogYWxyZWFkeSBoYW5kbGluZyB0aGUg UkVDTEFJTV9ORUVERUQgYW5kL29yIFNUQVRFX1JFVk9LRUQuDQo+PiArICAgICAgICAqIFRob3Nl IGZsYWdzIGFyZSBleHBlY3RlZCB0byByZW1haW4gc2V0IHVudGlsIHdlJ3JlIGRvbmUNCj4+ICsg ICAgICAgICogcmVjb3ZlcmluZyAoc2VlIFJGQzU2NjEsIHNlY3Rpb24gMTguNDYuMykuDQo+PiAr ICAgICAgICAqLw0KPj4gKyAgICAgICBpZiAocmVjb3ZlcnkpDQo+PiArICAgICAgICAgICAgICAg Z290byBvdXRfcmVjb3Zlcnk7DQo+PiANCj4+ICAgICAgICBpZiAoZmxhZ3MgJiBTRVE0X1NUQVRV U19SRVNUQVJUX1JFQ0xBSU1fTkVFREVEKQ0KPj4gICAgICAgICAgICAgICAgbmZzNDFfaGFuZGxl X3NlcnZlcl9yZWJvb3QoY2xwKTsNCj4+IEBAIC0yMjQ2LDYgKzIyNTUsNyBAQCB2b2lkIG5mczQx X2hhbmRsZV9zZXF1ZW5jZV9mbGFnX2Vycm9ycyhzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCB1MzIg ZmxhZ3MpDQo+PiAgICAgICAgICAgICAgICBuZnM0X3NjaGVkdWxlX2xlYXNlX21vdmVkX3JlY292 ZXJ5KGNscCk7DQo+PiAgICAgICAgaWYgKGZsYWdzICYgU0VRNF9TVEFUVVNfUkVDQUxMQUJMRV9T VEFURV9SRVZPS0VEKQ0KPj4gICAgICAgICAgICAgICAgbmZzNDFfaGFuZGxlX3JlY2FsbGFibGVf c3RhdGVfcmV2b2tlZChjbHApOw0KPj4gK291dF9yZWNvdmVyeToNCj4+ICAgICAgICBpZiAoZmxh Z3MgJiBTRVE0X1NUQVRVU19CQUNLQ0hBTk5FTF9GQVVMVCkNCj4+ICAgICAgICAgICAgICAgIG5m czQxX2hhbmRsZV9iYWNrY2hhbm5lbF9mYXVsdChjbHApOw0KPj4gICAgICAgIGVsc2UgaWYgKGZs YWdzICYgKFNFUTRfU1RBVFVTX0NCX1BBVEhfRE9XTiB8DQo+PiAtLQ0KPj4gMi43LjQNCj4+IA0K Pj4gLS0NCj4+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1 bnN1YnNjcmliZSBsaW51eC1uZnMiIGluDQo+PiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFq b3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDov L3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoNCg== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 00/12] Fix delegation behaviour when server revokes some state 2016-09-09 18:04 [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust @ 2016-09-10 0:49 ` Oleg Drokin 1 sibling, 0 replies; 16+ messages in thread From: Oleg Drokin @ 2016-09-10 0:49 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs On Sep 9, 2016, at 2:04 PM, Trond Myklebust wrote: > According to RFC5661, if any of the SEQUENCE status bits > SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED, > SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED, SEQ4_STATUS_ADMIN_STATE_REVOKED, > or SEQ4_STATUS_RECALLABLE_STATE_REVOKED are set, then we need to use > TEST_STATEID to figure out which stateids have been revoked, so we > can acknowledge the loss of state using FREE_STATEID. > > While we already do this for open and lock state, we have not been doing > so for all the delegations. > > v2: nfs_v4_2_minor_ops needs to set .test_and_free_expired too > v3: Now with added lock revoke fixes and close/delegreturn/locku fixes With this set there are changes in behavior of my reproducer. Change #1, server leaks dentries now, so once it's stopped, I cannot unmount the ext4 that was exported. I see your changes are just to the client, so I guess there's another nfsd bug that needs to be tracked here. Change #2, instead of client wedged in the old way, it's now wedges in another, but similar way: nfs client mountpoint is busy, but instead of server-ip-named thread spinning, it's one of actual userspace threads that's now wedged. -1 debug logs shows: [21122.814992] NFS call test_stateid ffff880069fb1f24 [21122.814997] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000 [21122.814997] --> nfs41_setup_sequence [21122.814999] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31 [21122.815000] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0 [21122.815001] <-- nfs41_setup_sequence slotid=0 seqid=3502621 [21122.815010] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502621 slotid=0 max_slotid=0 cache_this=0 [21122.815191] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31 [21122.815192] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1 [21122.815193] nfs4_free_slot: slotid 1 highest_used_slotid 0 [21122.815194] nfs41_sequence_process: Error 0 free the slot [21122.815195] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295 [21122.815226] NFS reply test_stateid: succeeded, 0 [21122.815235] --> nfs_put_client({3}) [21122.815253] --> nfs_put_client({2}) [21122.815260] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000 [21122.815260] --> nfs41_setup_sequence [21122.815262] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31 [21122.815263] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0 [21122.815264] <-- nfs41_setup_sequence slotid=0 seqid=3502622 [21122.815273] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502622 slotid=0 max_slotid=0 cache_this=1 [21122.815437] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31 [21122.815438] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1 [21122.815439] nfs4_free_slot: slotid 1 highest_used_slotid 0 [21122.815440] nfs41_sequence_process: Error 0 free the slot [21122.815442] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295 [21122.815462] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost [21122.815578] NFS call test_stateid ffff880069fb1f24 [21122.815583] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000 [21122.815583] --> nfs41_setup_sequence [21122.815585] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31 [21122.815586] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0 [21122.815587] <-- nfs41_setup_sequence slotid=0 seqid=3502623 [21122.815596] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502623 slotid=0 max_slotid=0 cache_this=0 [21122.819581] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31 [21122.819583] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1 [21122.819585] nfs4_free_slot: slotid 1 highest_used_slotid 0 [21122.819586] nfs41_sequence_process: Error 0 free the slot [21122.819588] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295 [21122.819613] NFS reply test_stateid: succeeded, 0 [21122.819629] --> nfs_put_client({3}) [21122.819658] --> nfs_put_client({2}) [21122.819669] --> nfs41_call_sync_prepare data->seq_server ffff88005fca9000 [21122.819669] --> nfs41_setup_sequence [21122.819671] --> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=31 [21122.819672] <-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0 [21122.819673] <-- nfs41_setup_sequence slotid=0 seqid=3502624 [21122.819688] encode_sequence: sessionid=1473458059:7:8:0 seqid=3502624 slotid=0 max_slotid=0 cache_this=1 [21122.819924] --> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=31 [21122.819926] <-- nfs4_alloc_slot used_slots=0003 highest_used=1 slotid=1 [21122.819927] nfs4_free_slot: slotid 1 highest_used_slotid 0 [21122.819928] nfs41_sequence_process: Error 0 free the slot [21122.819929] nfs4_free_slot: slotid 0 highest_used_slotid 4294967295 [21122.819952] nfs4_schedule_stateid_recovery: scheduling stateid recovery for server localhost [21122.823396] NFS call test_stateid ffff880069fb1f24 This is happening not as frequently as a previous problem, or possibly it's just masked by the dentry leakage server-side that I saw more frequently than this one. > Trond Myklebust (12): > NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags > NFSv4.1: Don't check delegations that are already marked as revoked > NFSv4.1: Allow test_stateid to handle session errors without waiting > NFSv4.1: Add a helper function to deal with expired stateids > NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a > stateid > NFSv4.1: Test delegation stateids when server declares "some state > revoked" > NFSv4.1: Deal with server reboots during delegation expiration > recovery > NFSv4.1: Don't recheck delegations that have already been checked > NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID > NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks > NFSv4.1: FREE_STATEID can be asynchronous > NFSv4.1: Ensure we call FREE_STATEID if needed on > close/delegreturn/locku > > fs/nfs/delegation.c | 153 ++++++++++++++++++- > fs/nfs/delegation.h | 6 +- > fs/nfs/flexfilelayout/flexfilelayout.c | 2 +- > fs/nfs/nfs4_fs.h | 5 +- > fs/nfs/nfs4proc.c | 258 ++++++++++++++++++++++----------- > fs/nfs/nfs4session.h | 1 + > fs/nfs/nfs4state.c | 29 ++-- > include/linux/nfs4.h | 1 + > 8 files changed, 348 insertions(+), 107 deletions(-) > > -- > 2.7.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-09-10 0:49 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-09 18:04 [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 02/12] NFSv4.1: Don't check delegations that are already marked as revoked Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 03/12] NFSv4.1: Allow test_stateid to handle session errors without waiting Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 04/12] NFSv4.1: Add a helper function to deal with expired stateids Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 05/12] NFSv4.x: Allow callers of nfs_remove_bad_delegation() to specify a stateid Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 06/12] NFSv4.1: Test delegation stateids when server declares "some state revoked" Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 07/12] NFSv4.1: Deal with server reboots during delegation expiration recovery Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 08/12] NFSv4.1: Don't recheck delegations that have already been checked Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 09/12] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 10/12] NFSv4.1: Ensure we always run TEST/FREE_STATEID on locks Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 11/12] NFSv4.1: FREE_STATEID can be asynchronous Trond Myklebust 2016-09-09 18:04 ` [PATCH v3 12/12] NFSv4.1: Ensure we call FREE_STATEID if needed on close/delegreturn/locku Trond Myklebust 2016-09-09 18:26 ` [PATCH v3 01/12] NFSv4.1: Don't deadlock the state manager on the SEQUENCE status flags Olga Kornievskaia 2016-09-09 18:32 ` Trond Myklebust 2016-09-10 0:49 ` [PATCH v3 00/12] Fix delegation behaviour when server revokes some state Oleg Drokin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).