* [PATCH v2 0/5] Fix up delegation reclaim and recovery issues
@ 2014-11-12 22:31 Trond Myklebust
2014-11-12 22:31 ` [PATCH v2 1/5] NFSv4: Ensure that we remove NFSv4.0 delegations when state has expired Trond Myklebust
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: linux-nfs
The main change w.r.t. the first iteration of these patches is that
we no longer trust the NFS_DELEGATED_STATE flag to tell us when we need
to test and/or discard the delegation. Instead, we just assume that
if we have a delegation then it is part of the recovery process.
Trond Myklebust (5):
NFSv4: Ensure that we remove NFSv4.0 delegations when state has
expired
NFSv4.1: nfs41_clear_delegation_stateid shouldn't trust
NFS_DELEGATED_STATE
NFSv4: Fix races between nfs_remove_bad_delegation() and delegation
return
NFSv4: Ensure that we call FREE_STATEID when NFSv4.x stateids are
revoked
NFS: Don't try to reclaim delegation open state if recovery failed
fs/nfs/delegation.c | 25 ++++++++++++--
fs/nfs/delegation.h | 1 +
fs/nfs/filelayout/filelayout.c | 3 --
fs/nfs/nfs4proc.c | 76 +++++++++++++++++++++++-------------------
4 files changed, 65 insertions(+), 40 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/5] NFSv4: Ensure that we remove NFSv4.0 delegations when state has expired 2014-11-12 22:31 [PATCH v2 0/5] Fix up delegation reclaim and recovery issues Trond Myklebust @ 2014-11-12 22:31 ` Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 2/5] NFSv4.1: nfs41_clear_delegation_stateid shouldn't trust NFS_DELEGATED_STATE Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs NFSv4.0 does not have TEST_STATEID/FREE_STATEID functionality, so unlike NFSv4.1, the recovery procedure when stateids have expired or have been revoked requires us to just forget the delegation. http://lkml.kernel.org/r/CAN-5tyHwG=Cn2Q9KsHWadewjpTTy_K26ee+UnSvHvG4192p-Xw@mail.gmail.com Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index dca174ce8309..bdd880bddba4 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2109,6 +2109,28 @@ 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) +{ + nfs_remove_bad_delegation(state->inode); + write_seqlock(&state->seqlock); + nfs4_stateid_copy(&state->stateid, &state->open_stateid); + write_sequnlock(&state->seqlock); + clear_bit(NFS_DELEGATED_STATE, &state->flags); +} + +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); +} + +static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *state) +{ + /* NFSv4.0 doesn't allow for delegation recovery on open expire */ + nfs40_clear_delegation_stateid(state); + return nfs4_open_expired(sp, state); +} + #if defined(CONFIG_NFS_V4_1) static void nfs41_clear_delegation_stateid(struct nfs4_state *state) { @@ -8330,7 +8352,7 @@ static const struct nfs4_state_recovery_ops nfs41_reboot_recovery_ops = { static const struct nfs4_state_recovery_ops nfs40_nograce_recovery_ops = { .owner_flag_bit = NFS_OWNER_RECLAIM_NOGRACE, .state_flag_bit = NFS_STATE_RECLAIM_NOGRACE, - .recover_open = nfs4_open_expired, + .recover_open = nfs40_open_expired, .recover_lock = nfs4_lock_expired, .establish_clid = nfs4_init_clientid, }; -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/5] NFSv4.1: nfs41_clear_delegation_stateid shouldn't trust NFS_DELEGATED_STATE 2014-11-12 22:31 ` [PATCH v2 1/5] NFSv4: Ensure that we remove NFSv4.0 delegations when state has expired Trond Myklebust @ 2014-11-12 22:31 ` Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 3/5] NFSv4: Fix races between nfs_remove_bad_delegation() and delegation return Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs This patch removes the assumption made previously, that we only need to check the delegation stateid when it matches the stateid on a cached open. If we believe that we hold a delegation for this file, then we must assume that its stateid may have been revoked or expired too. If we don't test it then our state recovery process may end up caching open/lock state in a situation where it should not. We therefore rename the function nfs41_clear_delegation_stateid as nfs41_check_delegation_stateid, and change it to always run through the delegation stateid test and recovery process as outlined in RFC5661. http://lkml.kernel.org/r/CAN-5tyHwG=Cn2Q9KsHWadewjpTTy_K26ee+UnSvHvG4192p-Xw@mail.gmail.com Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index bdd880bddba4..3b98fe752ef8 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2132,45 +2132,37 @@ static int nfs40_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st } #if defined(CONFIG_NFS_V4_1) -static void nfs41_clear_delegation_stateid(struct nfs4_state *state) +static void nfs41_check_delegation_stateid(struct nfs4_state *state) { struct nfs_server *server = NFS_SERVER(state->inode); - nfs4_stateid *stateid = &state->stateid; + nfs4_stateid stateid; struct nfs_delegation *delegation; - struct rpc_cred *cred = NULL; - int status = -NFS4ERR_BAD_STATEID; - - /* If a state reset has been done, test_stateid is unneeded */ - if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0) - return; + struct rpc_cred *cred; + int status; /* Get the delegation credential for use by test/free_stateid */ rcu_read_lock(); delegation = rcu_dereference(NFS_I(state->inode)->delegation); - if (delegation != NULL && - nfs4_stateid_match(&delegation->stateid, stateid)) { - cred = get_rpccred(delegation->cred); - rcu_read_unlock(); - status = nfs41_test_stateid(server, stateid, cred); - trace_nfs4_test_delegation_stateid(state, NULL, status); - } else + if (delegation == NULL) { rcu_read_unlock(); + return; + } + + nfs4_stateid_copy(&stateid, &delegation->stateid); + cred = get_rpccred(delegation->cred); + rcu_read_unlock(); + status = nfs41_test_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); - nfs_remove_bad_delegation(state->inode); - - write_seqlock(&state->seqlock); - nfs4_stateid_copy(&state->stateid, &state->open_stateid); - write_sequnlock(&state->seqlock); - clear_bit(NFS_DELEGATED_STATE, &state->flags); + nfs41_free_stateid(server, &stateid, cred); + nfs_finish_clear_delegation_stateid(state); } - if (cred != NULL) - put_rpccred(cred); + put_rpccred(cred); } /** @@ -2214,7 +2206,7 @@ static int nfs41_open_expired(struct nfs4_state_owner *sp, struct nfs4_state *st { int status; - nfs41_clear_delegation_stateid(state); + nfs41_check_delegation_stateid(state); status = nfs41_check_open_stateid(state); if (status != NFS_OK) status = nfs4_open_expired(sp, state); -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/5] NFSv4: Fix races between nfs_remove_bad_delegation() and delegation return 2014-11-12 22:31 ` [PATCH v2 2/5] NFSv4.1: nfs41_clear_delegation_stateid shouldn't trust NFS_DELEGATED_STATE Trond Myklebust @ 2014-11-12 22:31 ` Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 4/5] NFSv4: Ensure that we call FREE_STATEID when NFSv4.x stateids are revoked Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs Any attempt to call nfs_remove_bad_delegation() while a delegation is being returned is currently a no-op. This means that we can end up looping forever in nfs_end_delegation_return() if something causes the delegation to be revoked. This patch adds a mechanism whereby the state recovery code can communicate to the delegation return code that the delegation is no longer valid and that it should not be used when reclaiming state. It also changes the return value for nfs4_handle_delegation_recall_error() to ensure that nfs_end_delegation_return() does not reattempt the lock reclaim before state recovery is done. http://lkml.kernel.org/r/CAN-5tyHwG=Cn2Q9KsHWadewjpTTy_K26ee+UnSvHvG4192p-Xw@mail.gmail.com Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 23 +++++++++++++++++++++-- fs/nfs/delegation.h | 1 + fs/nfs/nfs4proc.c | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 5853f53db732..e5f473d13e24 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -193,7 +193,11 @@ static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation * { int res = 0; - res = nfs4_proc_delegreturn(inode, delegation->cred, &delegation->stateid, issync); + if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) + res = nfs4_proc_delegreturn(inode, + delegation->cred, + &delegation->stateid, + issync); nfs_free_delegation(delegation); return res; } @@ -380,11 +384,13 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation { struct nfs_client *clp = NFS_SERVER(inode)->nfs_client; struct nfs_inode *nfsi = NFS_I(inode); - int err; + int err = 0; if (delegation == NULL) return 0; do { + if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) + break; err = nfs_delegation_claim_opens(inode, &delegation->stateid); if (!issync || err != -EAGAIN) break; @@ -605,10 +611,23 @@ static void nfs_client_mark_return_unused_delegation_types(struct nfs_client *cl rcu_read_unlock(); } +static void nfs_revoke_delegation(struct inode *inode) +{ + struct nfs_delegation *delegation; + 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); + } + rcu_read_unlock(); +} + void nfs_remove_bad_delegation(struct inode *inode) { struct nfs_delegation *delegation; + nfs_revoke_delegation(inode); 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 5c1cce39297f..e3c20a3ccc93 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -31,6 +31,7 @@ enum { NFS_DELEGATION_RETURN_IF_CLOSED, NFS_DELEGATION_REFERENCED, NFS_DELEGATION_RETURNING, + NFS_DELEGATION_REVOKED, }; int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res); diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 3b98fe752ef8..4b7166f4e1cf 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1654,7 +1654,7 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct nfs_inode_find_state_and_recover(state->inode, stateid); nfs4_schedule_stateid_recovery(server, state); - return 0; + return -EAGAIN; case -NFS4ERR_DELAY: case -NFS4ERR_GRACE: set_bit(NFS_DELEGATED_STATE, &state->flags); -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/5] NFSv4: Ensure that we call FREE_STATEID when NFSv4.x stateids are revoked 2014-11-12 22:31 ` [PATCH v2 3/5] NFSv4: Fix races between nfs_remove_bad_delegation() and delegation return Trond Myklebust @ 2014-11-12 22:31 ` Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 5/5] NFS: Don't try to reclaim delegation open state if recovery failed Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs NFSv4.x (x>0) requires us to call TEST_STATEID+FREE_STATEID if a stateid is revoked. We will currently fail to do this if the stateid is a delegation. http://lkml.kernel.org/r/CAN-5tyHwG=Cn2Q9KsHWadewjpTTy_K26ee+UnSvHvG4192p-Xw@mail.gmail.com Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/filelayout/filelayout.c | 3 --- fs/nfs/nfs4proc.c | 8 -------- 2 files changed, 11 deletions(-) diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c index 46fab1cb455a..7afb52f6a25a 100644 --- a/fs/nfs/filelayout/filelayout.c +++ b/fs/nfs/filelayout/filelayout.c @@ -145,9 +145,6 @@ static int filelayout_async_handle_error(struct rpc_task *task, case -NFS4ERR_DELEG_REVOKED: case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_BAD_STATEID: - if (state == NULL) - break; - nfs_remove_bad_delegation(state->inode); case -NFS4ERR_OPENMODE: if (state == NULL) break; diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 4b7166f4e1cf..69dc20a743f9 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -370,11 +370,6 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc case -NFS4ERR_DELEG_REVOKED: case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_BAD_STATEID: - if (inode != NULL && nfs4_have_delegation(inode, FMODE_READ)) { - nfs_remove_bad_delegation(inode); - exception->retry = 1; - break; - } if (state == NULL) break; ret = nfs4_schedule_stateid_recovery(server, state); @@ -4844,9 +4839,6 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server, case -NFS4ERR_DELEG_REVOKED: case -NFS4ERR_ADMIN_REVOKED: case -NFS4ERR_BAD_STATEID: - if (state == NULL) - break; - nfs_remove_bad_delegation(state->inode); case -NFS4ERR_OPENMODE: if (state == NULL) break; -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 5/5] NFS: Don't try to reclaim delegation open state if recovery failed 2014-11-12 22:31 ` [PATCH v2 4/5] NFSv4: Ensure that we call FREE_STATEID when NFSv4.x stateids are revoked Trond Myklebust @ 2014-11-12 22:31 ` Trond Myklebust 0 siblings, 0 replies; 6+ messages in thread From: Trond Myklebust @ 2014-11-12 22:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs If state recovery failed, then we should not attempt to reclaim delegated state. http://lkml.kernel.org/r/CAN-5tyHwG=Cn2Q9KsHWadewjpTTy_K26ee+UnSvHvG4192p-Xw@mail.gmail.com Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/delegation.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index e5f473d13e24..7f3f60641344 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -125,6 +125,8 @@ again: continue; if (!test_bit(NFS_DELEGATED_STATE, &state->flags)) continue; + if (!nfs4_valid_open_stateid(state)) + continue; if (!nfs4_stateid_match(&state->stateid, stateid)) continue; get_nfs_open_context(ctx); -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-12 22:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-12 22:31 [PATCH v2 0/5] Fix up delegation reclaim and recovery issues Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 1/5] NFSv4: Ensure that we remove NFSv4.0 delegations when state has expired Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 2/5] NFSv4.1: nfs41_clear_delegation_stateid shouldn't trust NFS_DELEGATED_STATE Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 3/5] NFSv4: Fix races between nfs_remove_bad_delegation() and delegation return Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 4/5] NFSv4: Ensure that we call FREE_STATEID when NFSv4.x stateids are revoked Trond Myklebust 2014-11-12 22:31 ` [PATCH v2 5/5] NFS: Don't try to reclaim delegation open state if recovery failed Trond Myklebust
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox