If we know that the delegation stateid is bad or revoked, we need to
remove that delegation as soon as possible, and then mark all the
stateids that relied on that delegation for recovery. We cannot use
the delegation as part of the recovery process.
Also note that NFSv4.1 uses a different error code (NFS4ERR_DELEG_REVOKED)
to indicate that the delegation was revoked.
Finally, ensure that setlk() and setattr() can both recover safely from
a revoked delegation.
Signed-off-by: Trond Myklebust <[email protected]>
Cc: [email protected]
---
v2: - Add handling of BAD_STATEID/DELEG_REVOKED/... in
nfs4_open_delegation_recall
- Remove redundant nfs_async_inode_return_delegation from
nfs4_schedule_stateid_recovery
fs/nfs/delegation.c | 11 +++++++++++
fs/nfs/delegation.h | 1 +
fs/nfs/nfs4_fs.h | 2 ++
fs/nfs/nfs4proc.c | 18 ++++++++++++++++--
fs/nfs/nfs4state.c | 29 +++++++++++++++++++++++++++--
5 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 2d06080..5eb3981 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -467,6 +467,17 @@ static void nfs_delegation_run_state_manager(struct nfs_client *clp)
nfs4_schedule_state_manager(clp);
}
+void nfs_remove_bad_delegation(struct inode *inode)
+{
+ struct nfs_delegation *delegation;
+
+ delegation = nfs_detach_delegation(NFS_I(inode), NFS_SERVER(inode));
+ if (delegation) {
+ nfs_inode_find_state_and_recover(inode, &delegation->stateid);
+ nfs_free_delegation(delegation);
+ }
+}
+
/**
* nfs_expire_all_delegation_types
* @clp: client to process
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index d9322e4..691a796 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -45,6 +45,7 @@ void nfs_expire_unreferenced_delegations(struct nfs_client *clp);
void nfs_handle_cb_pathdown(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_delegation_mark_reclaim(struct nfs_client *clp);
void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4d7d0ae..5c9b20b 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -327,6 +327,8 @@ extern void nfs4_put_open_state(struct nfs4_state *);
extern void nfs4_close_state(struct nfs4_state *, fmode_t);
extern void nfs4_close_sync(struct nfs4_state *, fmode_t);
extern void nfs4_state_set_mode_locked(struct nfs4_state *, fmode_t);
+extern void nfs_inode_find_state_and_recover(struct inode *inode,
+ const nfs4_stateid *stateid);
extern void nfs4_schedule_lease_recovery(struct nfs_client *);
extern void nfs4_schedule_state_manager(struct nfs_client *);
extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ec9f6ef..3d26bab 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -265,8 +265,11 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
switch(errorcode) {
case 0:
return 0;
+ case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_BAD_STATEID:
+ if (state != NULL)
+ nfs_remove_bad_delegation(state->inode);
case -NFS4ERR_OPENMODE:
if (state == NULL)
break;
@@ -1319,8 +1322,11 @@ int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state
* The show must go on: exit, but mark the
* stateid as needing recovery.
*/
+ case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_BAD_STATEID:
+ nfs_inode_find_state_and_recover(state->inode,
+ stateid);
nfs4_schedule_stateid_recovery(server, state);
case -EKEYEXPIRED:
/*
@@ -1900,7 +1906,9 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
struct nfs4_state *state)
{
struct nfs_server *server = NFS_SERVER(inode);
- struct nfs4_exception exception = { };
+ struct nfs4_exception exception = {
+ .state = state,
+ };
int err;
do {
err = nfs4_handle_exception(server,
@@ -3714,8 +3722,11 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
if (task->tk_status >= 0)
return 0;
switch(task->tk_status) {
+ case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_BAD_STATEID:
+ if (state != NULL)
+ nfs_remove_bad_delegation(state->inode);
case -NFS4ERR_OPENMODE:
if (state == NULL)
break;
@@ -4533,7 +4544,9 @@ out:
static int nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
{
- struct nfs4_exception exception = { };
+ struct nfs4_exception exception = {
+ .state = state,
+ };
int err;
do {
@@ -4626,6 +4639,7 @@ int nfs4_lock_delegation_recall(struct nfs4_state *state, struct file_lock *fl)
* The show must go on: exit, but mark the
* stateid as needing recovery.
*/
+ case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_BAD_STATEID:
case -NFS4ERR_OPENMODE:
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4539203..0e60df1 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1132,12 +1132,37 @@ void nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4
{
struct nfs_client *clp = server->nfs_client;
- if (test_and_clear_bit(NFS_DELEGATED_STATE, &state->flags))
- nfs_async_inode_return_delegation(state->inode, &state->stateid);
nfs4_state_mark_reclaim_nograce(clp, state);
nfs4_schedule_state_manager(clp);
}
+void nfs_inode_find_state_and_recover(struct inode *inode,
+ const nfs4_stateid *stateid)
+{
+ struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+ struct nfs_inode *nfsi = NFS_I(inode);
+ struct nfs_open_context *ctx;
+ struct nfs4_state *state;
+ bool found = false;
+
+ spin_lock(&inode->i_lock);
+ list_for_each_entry(ctx, &nfsi->open_files, list) {
+ state = ctx->state;
+ if (state == NULL)
+ continue;
+ if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
+ continue;
+ if (memcmp(state->stateid.data, stateid->data, sizeof(state->stateid.data)) != 0)
+ continue;
+ nfs4_state_mark_reclaim_nograce(clp, state);
+ found = true;
+ }
+ spin_unlock(&inode->i_lock);
+ if (found)
+ nfs4_schedule_state_manager(clp);
+}
+
+
static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_recovery_ops *ops)
{
struct inode *inode = state->inode;
--
1.7.7.6
With "NFS: Properly handle the case where the delegation is revoked" patch added to Trond's origin/testign branch, my python test that removes the (delegation) stateid on a READ, and returns NFS4ERR_BAD_STATEID passes as the client recovers by testing and freeing the stateid, and then sending an OPEN with a CLAIM_NULL.
-->Andy
BTW: this patch cleans up some stateid compares and the second chunk is needed for the origin/testing branch to compile. You've probably already found this?..
8-------------------------------------------------------
>From 5898e8c4ca19f0c22407186b305d982d30ffd653 Mon Sep 17 00:00:00 2001
From: Andy Adamson <[email protected]>
Date: Tue, 6 Mar 2012 10:57:41 -0500
Subject: [PATCH 1/1] NFSv4.1 use stateid helpers
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/delegation.c | 4 ++--
fs/nfs/nfs4state.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 15a4e8e..b93b996 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -248,8 +248,8 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
old_delegation = rcu_dereference_protected(nfsi->delegation,
lockdep_is_held(&clp->cl_lock));
if (old_delegation != NULL) {
- if (memcmp(&delegation->stateid, &old_delegation->stateid,
- sizeof(old_delegation->stateid)) == 0 &&
+ if (nfs4_stateid_match(&delegation->stateid,
+ &old_delegation->stateid) &&
delegation->type == old_delegation->type) {
goto out;
}
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index e98b8c0..079c0aa 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1126,7 +1126,7 @@ void nfs_inode_find_state_and_recover(struct inode *inode,
continue;
if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
continue;
- if (memcmp(state->stateid.data, stateid->data, sizeof(state->stateid.data)) != 0)
+ if (!nfs4_stateid_match(state->stateid, stateid))
continue;
nfs4_state_mark_reclaim_nograce(clp, state);
found = true;
--
1.7.6.4
On Mar 6, 2012, at 10:03 AM, Trond Myklebust wrote:
> If we know that the delegation stateid is bad or revoked, we need to
> remove that delegation as soon as possible, and then mark all the
> stateids that relied on that delegation for recovery. We cannot use
> the delegation as part of the recovery process.
>
> Also note that NFSv4.1 uses a different error code (NFS4ERR_DELEG_REVOKED)
> to indicate that the delegation was revoked.
>
> Finally, ensure that setlk() and setattr() can both recover safely from
> a revoked delegation.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: [email protected]
> ---
> v2: - Add handling of BAD_STATEID/DELEG_REVOKED/... in
> nfs4_open_delegation_recall
> - Remove redundant nfs_async_inode_return_delegation from
> nfs4_schedule_stateid_recovery
>
>
> fs/nfs/delegation.c | 11 +++++++++++
> fs/nfs/delegation.h | 1 +
> fs/nfs/nfs4_fs.h | 2 ++
> fs/nfs/nfs4proc.c | 18 ++++++++++++++++--
> fs/nfs/nfs4state.c | 29 +++++++++++++++++++++++++++--
> 5 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 2d06080..5eb3981 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -467,6 +467,17 @@ static void nfs_delegation_run_state_manager(struct nfs_client *clp)
> nfs4_schedule_state_manager(clp);
> }
>
> +void nfs_remove_bad_delegation(struct inode *inode)
> +{
> + struct nfs_delegation *delegation;
> +
> + delegation = nfs_detach_delegation(NFS_I(inode), NFS_SERVER(inode));
> + if (delegation) {
> + nfs_inode_find_state_and_recover(inode, &delegation->stateid);
> + nfs_free_delegation(delegation);
> + }
> +}
> +
> /**
> * nfs_expire_all_delegation_types
> * @clp: client to process
> diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
> index d9322e4..691a796 100644
> --- a/fs/nfs/delegation.h
> +++ b/fs/nfs/delegation.h
> @@ -45,6 +45,7 @@ void nfs_expire_unreferenced_delegations(struct nfs_client *clp);
> void nfs_handle_cb_pathdown(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_delegation_mark_reclaim(struct nfs_client *clp);
> void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 4d7d0ae..5c9b20b 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -327,6 +327,8 @@ extern void nfs4_put_open_state(struct nfs4_state *);
> extern void nfs4_close_state(struct nfs4_state *, fmode_t);
> extern void nfs4_close_sync(struct nfs4_state *, fmode_t);
> extern void nfs4_state_set_mode_locked(struct nfs4_state *, fmode_t);
> +extern void nfs_inode_find_state_and_recover(struct inode *inode,
> + const nfs4_stateid *stateid);
> extern void nfs4_schedule_lease_recovery(struct nfs_client *);
> extern void nfs4_schedule_state_manager(struct nfs_client *);
> extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index ec9f6ef..3d26bab 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -265,8 +265,11 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
> switch(errorcode) {
> case 0:
> return 0;
> + case -NFS4ERR_DELEG_REVOKED:
> case -NFS4ERR_ADMIN_REVOKED:
> case -NFS4ERR_BAD_STATEID:
> + if (state != NULL)
> + nfs_remove_bad_delegation(state->inode);
> case -NFS4ERR_OPENMODE:
> if (state == NULL)
> break;
> @@ -1319,8 +1322,11 @@ int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state
> * The show must go on: exit, but mark the
> * stateid as needing recovery.
> */
> + case -NFS4ERR_DELEG_REVOKED:
> case -NFS4ERR_ADMIN_REVOKED:
> case -NFS4ERR_BAD_STATEID:
> + nfs_inode_find_state_and_recover(state->inode,
> + stateid);
> nfs4_schedule_stateid_recovery(server, state);
> case -EKEYEXPIRED:
> /*
> @@ -1900,7 +1906,9 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
> struct nfs4_state *state)
> {
> struct nfs_server *server = NFS_SERVER(inode);
> - struct nfs4_exception exception = { };
> + struct nfs4_exception exception = {
> + .state = state,
> + };
> int err;
> do {
> err = nfs4_handle_exception(server,
> @@ -3714,8 +3722,11 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
> if (task->tk_status >= 0)
> return 0;
> switch(task->tk_status) {
> + case -NFS4ERR_DELEG_REVOKED:
> case -NFS4ERR_ADMIN_REVOKED:
> case -NFS4ERR_BAD_STATEID:
> + if (state != NULL)
> + nfs_remove_bad_delegation(state->inode);
> case -NFS4ERR_OPENMODE:
> if (state == NULL)
> break;
> @@ -4533,7 +4544,9 @@ out:
>
> static int nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
> {
> - struct nfs4_exception exception = { };
> + struct nfs4_exception exception = {
> + .state = state,
> + };
> int err;
>
> do {
> @@ -4626,6 +4639,7 @@ int nfs4_lock_delegation_recall(struct nfs4_state *state, struct file_lock *fl)
> * The show must go on: exit, but mark the
> * stateid as needing recovery.
> */
> + case -NFS4ERR_DELEG_REVOKED:
> case -NFS4ERR_ADMIN_REVOKED:
> case -NFS4ERR_BAD_STATEID:
> case -NFS4ERR_OPENMODE:
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 4539203..0e60df1 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1132,12 +1132,37 @@ void nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4
> {
> struct nfs_client *clp = server->nfs_client;
>
> - if (test_and_clear_bit(NFS_DELEGATED_STATE, &state->flags))
> - nfs_async_inode_return_delegation(state->inode, &state->stateid);
> nfs4_state_mark_reclaim_nograce(clp, state);
> nfs4_schedule_state_manager(clp);
> }
>
> +void nfs_inode_find_state_and_recover(struct inode *inode,
> + const nfs4_stateid *stateid)
> +{
> + struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> + struct nfs_inode *nfsi = NFS_I(inode);
> + struct nfs_open_context *ctx;
> + struct nfs4_state *state;
> + bool found = false;
> +
> + spin_lock(&inode->i_lock);
> + list_for_each_entry(ctx, &nfsi->open_files, list) {
> + state = ctx->state;
> + if (state == NULL)
> + continue;
> + if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
> + continue;
> + if (memcmp(state->stateid.data, stateid->data, sizeof(state->stateid.data)) != 0)
> + continue;
> + nfs4_state_mark_reclaim_nograce(clp, state);
> + found = true;
> + }
> + spin_unlock(&inode->i_lock);
> + if (found)
> + nfs4_schedule_state_manager(clp);
> +}
> +
> +
> static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_recovery_ops *ops)
> {
> struct inode *inode = state->inode;
> --
> 1.7.7.6
>
T24gVHVlLCAyMDEyLTAzLTA2IGF0IDE2OjI0ICswMDAwLCBBZGFtc29uLCBBbmR5IHdyb3RlOg0K
PiBXaXRoICAiTkZTOiBQcm9wZXJseSBoYW5kbGUgdGhlIGNhc2Ugd2hlcmUgdGhlIGRlbGVnYXRp
b24gaXMgcmV2b2tlZCIgcGF0Y2ggYWRkZWQgdG8gVHJvbmQncyBvcmlnaW4vdGVzdGlnbiBicmFu
Y2gsICBteSBweXRob24gdGVzdCB0aGF0IHJlbW92ZXMgdGhlIChkZWxlZ2F0aW9uKSBzdGF0ZWlk
IG9uIGEgUkVBRCwgYW5kIHJldHVybnMgTkZTNEVSUl9CQURfU1RBVEVJRCBwYXNzZXMgYXMgdGhl
IGNsaWVudCByZWNvdmVycyBieSB0ZXN0aW5nIGFuZCBmcmVlaW5nIHRoZSBzdGF0ZWlkLCBhbmQg
dGhlbiBzZW5kaW5nIGFuIE9QRU4gd2l0aCBhIENMQUlNX05VTEwuDQo+IA0KPiAtLT5BbmR5DQo+
IA0KPiANCj4gQlRXOiB0aGlzIHBhdGNoIGNsZWFucyB1cCBzb21lIHN0YXRlaWQgY29tcGFyZXMg
YW5kIHRoZSBzZWNvbmQgY2h1bmsgaXMgbmVlZGVkIGZvciB0aGUgb3JpZ2luL3Rlc3RpbmcgYnJh
bmNoIHRvIGNvbXBpbGUuICBZb3UndmUgcHJvYmFibHkgYWxyZWFkeSBmb3VuZCB0aGlz4oCmLi4N
Cg0KWWVzLiBJbiBvcmRlciB0byBtYWtlIHRoZSBwYXRjaCBhcHBseSB0byB0aGUgc3RhYmxlIGtl
cm5lbHMsIEkgYWN0dWFsbHkNCmluc2VydGVkIGl0IGJlZm9yZSB0aGUgc3RhdGVpZCBjaGFuZ2Vz
IGluIHRoZSAnZGV2ZWwnIGJyYW5jaCwgYW5kIHRoZW4NCmZpeGVkIHRoZSBzdGF0ZWlkIHBhdGNo
ZXMgdXAgaW5zdGVhZC4gVGhlIGN1cnJlbnQgJ3Rlc3RpbmcnIGJyYW5jaA0Kc2hvdWxkIGhhdmUg
YSB3b3JraW5nIHZlcnNpb24gb2YgYm90aC4NCg0KQlRXOiB3aXRoIHRoaXMgcGF0Y2gsIHRoZSAi
TkZTdjQuMTogRml4IHRoZSBjaGVja2luZyBvZiB0aGUgc3RhdGVpZCB3aGVuDQpyZXR1cm5pbmcg
YSBkZWxlZ2F0aW9uIiBpcyBub3QgbmVlZGVkIGFzIGEgc3RhYmxlIGZpeCwgc2luY2Ugd2Ugbm8N
CmxvbmdlciB1c2UgbmZzX2FzeW5jX2lub2RlX3JldHVybl9kZWxlZ2F0aW9uKCkgaW4gc3RhdGUg
cmVjb3Zlcnkgc28gSSd2ZQ0KcmVtb3ZlZCB0aGUgQ2M6IHN0YWJsZSBmcm9tIHRoYXQgcGF0Y2gu
DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0K
TmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K