* [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