* [PATCH 00/14] Delegation bugfixes
@ 2019-10-23 23:55 Trond Myklebust
2019-10-23 23:55 ` [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation Trond Myklebust
2019-10-31 15:27 ` [PATCH 00/14] Delegation bugfixes Olga Kornievskaia
0 siblings, 2 replies; 19+ messages in thread
From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw)
To: linux-nfs
The following patchset fixes up a number of issues with delegations,
but mainly attempts to fix a race condition between open and
delegreturn, where the open and the delegreturn get re-ordered so
that the delegreturn ends up revoking the delegation that was returned
by the open.
The root cause is that in certain circumstances, we may currently end
up freeing the delegation from delegreturn, so when we later receive
the reply to the open, we've lost track of the fact that the seqid
predates the one that was returned.
This patchset fixes that case by ensuring that we always keep track
of the last delegation stateid that was returned for any given inode.
That way, if we later see a delegation stateid with the same opaque
field, but an older seqid, we know we cannot trust it, and so we
ask to replay the OPEN compound.
Trond Myklebust (14):
NFSv4: Don't allow a cached open with a revoked delegation
NFSv4: Fix delegation handling in update_open_stateid()
NFSv4: nfs4_callback_getattr() should ignore revoked delegations
NFSv4: Delegation recalls should not find revoked delegations
NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was
revoked
NFS: Rename nfs_inode_return_delegation_noreclaim()
NFSv4: Don't remove the delegation from the super_list more than once
NFSv4: Hold the delegation spinlock when updating the seqid
NFSv4: Clear the NFS_DELEGATION_REVOKED flag in
nfs_update_inplace_delegation()
NFSv4: Update the stateid seqid in nfs_revoke_delegation()
NFSv4: Revoke the delegation on success in nfs4_delegreturn_done()
NFSv4: Ignore requests to return the delegation if it was revoked
NFSv4: Don't reclaim delegations that have been returned or revoked
NFSv4: Fix races between open and delegreturn
fs/nfs/callback_proc.c | 2 +-
fs/nfs/delegation.c | 109 +++++++++++++++++++++++++++++------------
fs/nfs/delegation.h | 4 +-
fs/nfs/nfs4proc.c | 13 ++---
fs/nfs/nfs4super.c | 4 +-
5 files changed, 88 insertions(+), 44 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation 2019-10-23 23:55 [PATCH 00/14] Delegation bugfixes Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 02/14] NFSv4: Fix delegation handling in update_open_stateid() Trond Myklebust 2019-10-31 15:27 ` [PATCH 00/14] Delegation bugfixes Olga Kornievskaia 1 sibling, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation is marked as being revoked, we must not use it for cached opens. Fixes: 869f9dfa4d6d ("NFSv4: Fix races between nfs_remove_bad_delegation() and delegation return") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 3 +-- fs/nfs/delegation.h | 1 + fs/nfs/nfs4proc.c | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 071b90a45933..5f3eea926af5 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -42,8 +42,7 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation) set_bit(NFS_DELEGATION_REFERENCED, &delegation->flags); } -static bool -nfs4_is_valid_delegation(const struct nfs_delegation *delegation, +bool nfs4_is_valid_delegation(const struct nfs_delegation *delegation, fmode_t flags) { if (delegation != NULL && (delegation->type & flags) == flags && diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index 9eb87ae4c982..2b35a99929a0 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -69,6 +69,7 @@ bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_state bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode); void nfs_mark_delegation_referenced(struct nfs_delegation *delegation); +bool nfs4_is_valid_delegation(const struct nfs_delegation *delegation, fmode_t flags); int nfs4_have_delegation(struct inode *inode, fmode_t flags); int nfs4_check_delegation(struct inode *inode, fmode_t flags); bool nfs4_delegation_flush_on_close(const struct inode *inode); diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index ab8ca20fd579..294ea8c1a163 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1436,11 +1436,7 @@ static int can_open_cached(struct nfs4_state *state, fmode_t mode, static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode, enum open_claim_type4 claim) { - if (delegation == NULL) - return 0; - if ((delegation->type & fmode) != fmode) - return 0; - if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) + if (!nfs4_is_valid_delegation(delegation, fmode)) return 0; switch (claim) { case NFS4_OPEN_CLAIM_NULL: -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 02/14] NFSv4: Fix delegation handling in update_open_stateid() 2019-10-23 23:55 ` [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 03/14] NFSv4: nfs4_callback_getattr() should ignore revoked delegations Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation is marked as being revoked, then don't use it in the open state structure. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 294ea8c1a163..c407e2eed3d5 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1740,14 +1740,12 @@ static int update_open_stateid(struct nfs4_state *state, goto no_delegation; spin_lock(&deleg_cur->lock); - if (rcu_dereference(nfsi->delegation) != deleg_cur || - test_bit(NFS_DELEGATION_RETURNING, &deleg_cur->flags) || - (deleg_cur->type & fmode) != fmode) + if (!nfs4_is_valid_delegation(deleg_cur, fmode)) goto no_delegation_unlock; if (delegation == NULL) delegation = &deleg_cur->stateid; - else if (!nfs4_stateid_match(&deleg_cur->stateid, delegation)) + else if (!nfs4_stateid_match_other(&deleg_cur->stateid, delegation)) goto no_delegation_unlock; nfs_mark_delegation_referenced(deleg_cur); -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 03/14] NFSv4: nfs4_callback_getattr() should ignore revoked delegations 2019-10-23 23:55 ` [PATCH 02/14] NFSv4: Fix delegation handling in update_open_stateid() Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 04/14] NFSv4: Delegation recalls should not find " Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation has been revoked, ignore it. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/callback_proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c index db3e7771e597..58a77c41ff36 100644 --- a/fs/nfs/callback_proc.c +++ b/fs/nfs/callback_proc.c @@ -50,7 +50,7 @@ __be32 nfs4_callback_getattr(void *argp, void *resp, nfsi = NFS_I(inode); rcu_read_lock(); delegation = rcu_dereference(nfsi->delegation); - if (delegation == NULL || (delegation->type & FMODE_WRITE) == 0) + if (!nfs4_is_valid_delegation(delegation, FMODE_WRITE)) goto out_iput; res->size = i_size_read(inode); res->change_attr = delegation->change_attr; -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 04/14] NFSv4: Delegation recalls should not find revoked delegations 2019-10-23 23:55 ` [PATCH 03/14] NFSv4: nfs4_callback_getattr() should ignore revoked delegations Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 05/14] NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was revoked Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If we're processsing a delegation recall, ignore the delegations that have already been revoked or returned. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 5f3eea926af5..4bc40c27141b 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -830,7 +830,7 @@ int nfs_async_inode_return_delegation(struct inode *inode, rcu_read_lock(); delegation = rcu_dereference(NFS_I(inode)->delegation); - if (delegation == NULL) + if (!nfs4_is_valid_delegation(delegation, FMODE_READ)) goto out_enoent; if (stateid != NULL && !clp->cl_mvops->match_stateid(&delegation->stateid, stateid)) @@ -855,6 +855,7 @@ nfs_delegation_find_inode_server(struct nfs_server *server, list_for_each_entry_rcu(delegation, &server->delegations, super_list) { spin_lock(&delegation->lock); if (delegation->inode != NULL && + !test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) && nfs_compare_fh(fhandle, &NFS_I(delegation->inode)->fh) == 0) { freeme = igrab(delegation->inode); if (freeme && nfs_sb_active(freeme->i_sb)) -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 05/14] NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was revoked 2019-10-23 23:55 ` [PATCH 04/14] NFSv4: Delegation recalls should not find " Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 06/14] NFS: Rename nfs_inode_return_delegation_noreclaim() Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation was revoked, we don't want to retry the delegreturn. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 4bc40c27141b..f90c3cf82f8f 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -1179,6 +1179,7 @@ bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode) rcu_read_lock(); delegation = rcu_dereference(NFS_I(inode)->delegation); if (delegation != NULL && + !test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) && nfs4_stateid_match_other(dst, &delegation->stateid)) { dst->seqid = delegation->stateid.seqid; return ret; -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 06/14] NFS: Rename nfs_inode_return_delegation_noreclaim() 2019-10-23 23:55 ` [PATCH 05/14] NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was revoked Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 07/14] NFSv4: Don't remove the delegation from the super_list more than once Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs Rename nfs_inode_return_delegation_noreclaim() to nfs_inode_evict_delegation(), which better describes what it does. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 11 +++++++---- fs/nfs/delegation.h | 2 +- fs/nfs/nfs4super.c | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index f90c3cf82f8f..e60737be6f26 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -574,19 +574,22 @@ int nfs_client_return_marked_delegations(struct nfs_client *clp) } /** - * nfs_inode_return_delegation_noreclaim - return delegation, don't reclaim opens + * nfs_inode_evict_delegation - return delegation, don't reclaim opens * @inode: inode to process * * Does not protect against delegation reclaims, therefore really only safe - * to be called from nfs4_clear_inode(). + * to be called from nfs4_clear_inode(). Guaranteed to always free + * the delegation structure. */ -void nfs_inode_return_delegation_noreclaim(struct inode *inode) +void nfs_inode_evict_delegation(struct inode *inode) { struct nfs_delegation *delegation; delegation = nfs_inode_detach_delegation(inode); - if (delegation != NULL) + if (delegation != NULL) { + set_bit(NFS_DELEGATION_INODE_FREEING, &delegation->flags); nfs_do_return_delegation(inode, delegation, 1); + } } /** diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index 2b35a99929a0..9a14a7ca1df9 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -43,7 +43,7 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred, fmode_t type, const nfs4_stateid *stateid, unsigned long pagemod_limit); int nfs4_inode_return_delegation(struct inode *inode); int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid); -void nfs_inode_return_delegation_noreclaim(struct inode *inode); +void nfs_inode_evict_delegation(struct inode *inode); struct inode *nfs_delegation_find_inode(struct nfs_client *clp, const struct nfs_fh *fhandle); void nfs_server_return_all_delegations(struct nfs_server *); diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c index 04c57066a11a..2c9cbade561a 100644 --- a/fs/nfs/nfs4super.c +++ b/fs/nfs/nfs4super.c @@ -92,8 +92,8 @@ static void nfs4_evict_inode(struct inode *inode) { truncate_inode_pages_final(&inode->i_data); clear_inode(inode); - /* If we are holding a delegation, return it! */ - nfs_inode_return_delegation_noreclaim(inode); + /* If we are holding a delegation, return and free it */ + nfs_inode_evict_delegation(inode); /* Note that above delegreturn would trigger pnfs return-on-close */ pnfs_return_layout(inode); pnfs_destroy_layout(NFS_I(inode)); -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 07/14] NFSv4: Don't remove the delegation from the super_list more than once 2019-10-23 23:55 ` [PATCH 06/14] NFS: Rename nfs_inode_return_delegation_noreclaim() Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 08/14] NFSv4: Hold the delegation spinlock when updating the seqid Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs Add a check to ensure that we haven't already removed the delegation from the inode after we take all the relevant locks. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index e60737be6f26..b49faf1b3d91 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -287,6 +287,10 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi, return NULL; spin_lock(&delegation->lock); + if (!delegation->inode) { + spin_unlock(&delegation->lock); + return NULL; + } set_bit(NFS_DELEGATION_RETURNING, &delegation->flags); list_del_rcu(&delegation->super_list); delegation->inode = NULL; -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 08/14] NFSv4: Hold the delegation spinlock when updating the seqid 2019-10-23 23:55 ` [PATCH 07/14] NFSv4: Don't remove the delegation from the super_list more than once Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 09/14] NFSv4: Clear the NFS_DELEGATION_REVOKED flag in nfs_update_inplace_delegation() Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index b49faf1b3d91..d9c80871cecc 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -376,8 +376,10 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, /* Is this an update of the existing delegation? */ if (nfs4_stateid_match_other(&old_delegation->stateid, &delegation->stateid)) { + spin_lock(&old_delegation->lock); nfs_update_inplace_delegation(old_delegation, delegation); + spin_unlock(&old_delegation->lock); goto out; } /* -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 09/14] NFSv4: Clear the NFS_DELEGATION_REVOKED flag in nfs_update_inplace_delegation() 2019-10-23 23:55 ` [PATCH 08/14] NFSv4: Hold the delegation spinlock when updating the seqid Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 10/14] NFSv4: Update the stateid seqid in nfs_revoke_delegation() Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the server sent us a new delegation stateid that is more recent than the one that got revoked, then clear the NFS_DELEGATION_REVOKED flag. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index d9c80871cecc..b6cc2e71fcdb 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -332,6 +332,7 @@ nfs_update_inplace_delegation(struct nfs_delegation *delegation, delegation->stateid.seqid = update->stateid.seqid; smp_wmb(); delegation->type = update->type; + clear_bit(NFS_DELEGATION_REVOKED, &delegation->flags); } } -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 10/14] NFSv4: Update the stateid seqid in nfs_revoke_delegation() 2019-10-23 23:55 ` [PATCH 09/14] NFSv4: Clear the NFS_DELEGATION_REVOKED flag in nfs_update_inplace_delegation() Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 11/14] NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If we revoke a delegation, but the stateid's seqid is newer, then ensure we update the seqid when marking the delegation as revoked. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index b6cc2e71fcdb..5a87d32a8d7c 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -760,8 +760,19 @@ static bool nfs_revoke_delegation(struct inode *inode, if (stateid == NULL) { nfs4_stateid_copy(&tmp, &delegation->stateid); stateid = &tmp; - } else if (!nfs4_stateid_match(stateid, &delegation->stateid)) - goto out; + } else { + if (!nfs4_stateid_match_other(stateid, &delegation->stateid)) + goto out; + spin_lock(&delegation->lock); + if (stateid->seqid) { + if (nfs4_stateid_is_newer(&delegation->stateid, stateid)) { + spin_unlock(&delegation->lock); + goto out; + } + delegation->stateid.seqid = stateid->seqid; + } + spin_unlock(&delegation->lock); + } nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation); ret = true; out: -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 11/14] NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() 2019-10-23 23:55 ` [PATCH 10/14] NFSv4: Update the stateid seqid in nfs_revoke_delegation() Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 12/14] NFSv4: Ignore requests to return the delegation if it was revoked Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation was successfully returned, then mark it as revoked. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 32 ++++++++++++++++++++++++++++++++ fs/nfs/delegation.h | 1 + fs/nfs/nfs4proc.c | 1 + 3 files changed, 34 insertions(+) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 5a87d32a8d7c..b214b88b35b5 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -795,6 +795,38 @@ void nfs_remove_bad_delegation(struct inode *inode, } EXPORT_SYMBOL_GPL(nfs_remove_bad_delegation); +void nfs_delegation_mark_returned(struct inode *inode, + const nfs4_stateid *stateid) +{ + struct nfs_delegation *delegation; + + if (!inode) + return; + + rcu_read_lock(); + delegation = rcu_dereference(NFS_I(inode)->delegation); + if (!delegation) + goto out_rcu_unlock; + + spin_lock(&delegation->lock); + if (!nfs4_stateid_match_other(stateid, &delegation->stateid)) + goto out_spin_unlock; + if (stateid->seqid && + nfs4_stateid_is_newer(&delegation->stateid, stateid)) + goto out_clear_returning; + + set_bit(NFS_DELEGATION_REVOKED, &delegation->flags); + +out_clear_returning: + clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); +out_spin_unlock: + spin_unlock(&delegation->lock); +out_rcu_unlock: + rcu_read_unlock(); + + nfs_inode_find_state_and_recover(inode, stateid); +} + /** * nfs_expire_unused_delegation_types * @clp: client to process diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index 9a14a7ca1df9..7f0151c40d2f 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -53,6 +53,7 @@ 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, const nfs4_stateid *stateid); +void nfs_delegation_mark_returned(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/nfs4proc.c b/fs/nfs/nfs4proc.c index c407e2eed3d5..4375b07ede25 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -6211,6 +6211,7 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) if (exception.retry) goto out_restart; } + nfs_delegation_mark_returned(data->inode, data->args.stateid); data->rpc_status = task->tk_status; return; out_restart: -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 12/14] NFSv4: Ignore requests to return the delegation if it was revoked 2019-10-23 23:55 ` [PATCH 11/14] NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:55 ` [PATCH 13/14] NFSv4: Don't reclaim delegations that have been returned or revoked Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation was revoked, or is already being returned, just clear the NFS_DELEGATION_RETURN and NFS_DELEGATION_RETURN_IF_CLOSED flags and keep going. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index b214b88b35b5..150a3bf7b35c 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -465,8 +465,6 @@ static bool nfs_delegation_need_return(struct nfs_delegation *delegation) { bool ret = false; - if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) - goto out; if (test_and_clear_bit(NFS_DELEGATION_RETURN, &delegation->flags)) ret = true; if (test_and_clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags) && !ret) { @@ -478,7 +476,10 @@ static bool nfs_delegation_need_return(struct nfs_delegation *delegation) ret = true; spin_unlock(&delegation->lock); } -out: + if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags) || + test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) + ret = false; + return ret; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 13/14] NFSv4: Don't reclaim delegations that have been returned or revoked 2019-10-23 23:55 ` [PATCH 12/14] NFSv4: Ignore requests to return the delegation if it was revoked Trond Myklebust @ 2019-10-23 23:55 ` Trond Myklebust 2019-10-23 23:56 ` [PATCH 14/14] NFSv4: Fix races between open and delegreturn Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:55 UTC (permalink / raw) To: linux-nfs If the delegation has already been revoked, we want to avoid reclaiming it on reboot. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 150a3bf7b35c..00c6c343dced 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -188,7 +188,7 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred, delegation = rcu_dereference(NFS_I(inode)->delegation); if (delegation != NULL) { spin_lock(&delegation->lock); - if (delegation->inode != NULL) { + if (nfs4_is_valid_delegation(delegation, FMODE_READ)) { nfs4_stateid_copy(&delegation->stateid, stateid); delegation->type = type; delegation->pagemod_limit = pagemod_limit; -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 14/14] NFSv4: Fix races between open and delegreturn 2019-10-23 23:55 ` [PATCH 13/14] NFSv4: Don't reclaim delegations that have been returned or revoked Trond Myklebust @ 2019-10-23 23:56 ` Trond Myklebust 0 siblings, 0 replies; 19+ messages in thread From: Trond Myklebust @ 2019-10-23 23:56 UTC (permalink / raw) To: linux-nfs If the server returns the same delegation in an open that we just used in a delegreturn, we need to ensure we don't apply that stateid if the delegreturn has freed it on the server. To do so, we ensure that we do not free the storage for the delegation until either it is replaced by a new one, or we throw the inode out of cache. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/delegation.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 00c6c343dced..db7cf480c108 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -218,7 +218,6 @@ static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation * delegation->cred, &delegation->stateid, issync); - nfs_free_delegation(delegation); return res; } @@ -291,7 +290,6 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi, spin_unlock(&delegation->lock); return NULL; } - set_bit(NFS_DELEGATION_RETURNING, &delegation->flags); list_del_rcu(&delegation->super_list); delegation->inode = NULL; rcu_assign_pointer(nfsi->delegation, NULL); @@ -318,10 +316,12 @@ nfs_inode_detach_delegation(struct inode *inode) struct nfs_server *server = NFS_SERVER(inode); struct nfs_delegation *delegation; - delegation = nfs_start_delegation_return(nfsi); - if (delegation == NULL) - return NULL; - return nfs_detach_delegation(nfsi, delegation, server); + rcu_read_lock(); + delegation = rcu_dereference(nfsi->delegation); + if (delegation != NULL) + delegation = nfs_detach_delegation(nfsi, delegation, server); + rcu_read_unlock(); + return delegation; } static void @@ -420,8 +420,10 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, spin_unlock(&clp->cl_lock); if (delegation != NULL) nfs_free_delegation(delegation); - if (freeme != NULL) + if (freeme != NULL) { nfs_do_return_delegation(inode, freeme, 0); + nfs_free_delegation(freeme); + } return status; } @@ -431,7 +433,6 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred, static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation *delegation, int issync) { struct nfs_client *clp = NFS_SERVER(inode)->nfs_client; - struct nfs_inode *nfsi = NFS_I(inode); int err = 0; if (delegation == NULL) @@ -453,8 +454,6 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation nfs_abort_delegation_return(delegation, clp); goto out; } - if (!nfs_detach_delegation(nfsi, delegation, NFS_SERVER(inode))) - goto out; err = nfs_do_return_delegation(inode, delegation, issync); out: @@ -597,6 +596,7 @@ void nfs_inode_evict_delegation(struct inode *inode) if (delegation != NULL) { set_bit(NFS_DELEGATION_INODE_FREEING, &delegation->flags); nfs_do_return_delegation(inode, delegation, 1); + nfs_free_delegation(delegation); } } @@ -744,10 +744,9 @@ static void nfs_mark_delegation_revoked(struct nfs_server *server, { set_bit(NFS_DELEGATION_REVOKED, &delegation->flags); delegation->stateid.type = NFS4_INVALID_STATEID_TYPE; - nfs_mark_return_delegation(server, delegation); } -static bool nfs_revoke_delegation(struct inode *inode, +static void nfs_revoke_delegation(struct inode *inode, const nfs4_stateid *stateid) { struct nfs_delegation *delegation; @@ -780,19 +779,12 @@ static bool nfs_revoke_delegation(struct inode *inode, rcu_read_unlock(); if (ret) nfs_inode_find_state_and_recover(inode, stateid); - return ret; } void nfs_remove_bad_delegation(struct inode *inode, const nfs4_stateid *stateid) { - struct nfs_delegation *delegation; - - if (!nfs_revoke_delegation(inode, stateid)) - return; - delegation = nfs_inode_detach_delegation(inode); - if (delegation) - nfs_free_delegation(delegation); + nfs_revoke_delegation(inode, stateid); } EXPORT_SYMBOL_GPL(nfs_remove_bad_delegation); @@ -816,7 +808,7 @@ void nfs_delegation_mark_returned(struct inode *inode, nfs4_stateid_is_newer(&delegation->stateid, stateid)) goto out_clear_returning; - set_bit(NFS_DELEGATION_REVOKED, &delegation->flags); + nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation); out_clear_returning: clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags); -- 2.21.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 00/14] Delegation bugfixes 2019-10-23 23:55 [PATCH 00/14] Delegation bugfixes Trond Myklebust 2019-10-23 23:55 ` [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation Trond Myklebust @ 2019-10-31 15:27 ` Olga Kornievskaia 2019-10-31 15:49 ` Olga Kornievskaia 1 sibling, 1 reply; 19+ messages in thread From: Olga Kornievskaia @ 2019-10-31 15:27 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs Hi Trond, This patch set produces the following in my testing. Basically what I see the client is prevented from using a delegation at all. After I induce a race of DELEGRETURN/OPEN --- the racing OPEN gets a delegation (it returns the same seqid and other as the delegation being returned) but the client doesn't use it. --- the following (next) OPEN that also gets a delegation immediately has the client returning the given delegation. Disclaimer: in my testing the racing DELEGRETURN doesn't fail with OLD_STATEID, NetApp returns OK. On Thu, Oct 24, 2019 at 6:56 AM Trond Myklebust <trondmy@gmail.com> wrote: > > The following patchset fixes up a number of issues with delegations, > but mainly attempts to fix a race condition between open and > delegreturn, where the open and the delegreturn get re-ordered so > that the delegreturn ends up revoking the delegation that was returned > by the open. > The root cause is that in certain circumstances, we may currently end > up freeing the delegation from delegreturn, so when we later receive > the reply to the open, we've lost track of the fact that the seqid > predates the one that was returned. > > This patchset fixes that case by ensuring that we always keep track > of the last delegation stateid that was returned for any given inode. > That way, if we later see a delegation stateid with the same opaque > field, but an older seqid, we know we cannot trust it, and so we > ask to replay the OPEN compound. > > Trond Myklebust (14): > NFSv4: Don't allow a cached open with a revoked delegation > NFSv4: Fix delegation handling in update_open_stateid() > NFSv4: nfs4_callback_getattr() should ignore revoked delegations > NFSv4: Delegation recalls should not find revoked delegations > NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was > revoked > NFS: Rename nfs_inode_return_delegation_noreclaim() > NFSv4: Don't remove the delegation from the super_list more than once > NFSv4: Hold the delegation spinlock when updating the seqid > NFSv4: Clear the NFS_DELEGATION_REVOKED flag in > nfs_update_inplace_delegation() > NFSv4: Update the stateid seqid in nfs_revoke_delegation() > NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() > NFSv4: Ignore requests to return the delegation if it was revoked > NFSv4: Don't reclaim delegations that have been returned or revoked > NFSv4: Fix races between open and delegreturn > > fs/nfs/callback_proc.c | 2 +- > fs/nfs/delegation.c | 109 +++++++++++++++++++++++++++++------------ > fs/nfs/delegation.h | 4 +- > fs/nfs/nfs4proc.c | 13 ++--- > fs/nfs/nfs4super.c | 4 +- > 5 files changed, 88 insertions(+), 44 deletions(-) > > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/14] Delegation bugfixes 2019-10-31 15:27 ` [PATCH 00/14] Delegation bugfixes Olga Kornievskaia @ 2019-10-31 15:49 ` Olga Kornievskaia 2019-10-31 16:15 ` Olga Kornievskaia 0 siblings, 1 reply; 19+ messages in thread From: Olga Kornievskaia @ 2019-10-31 15:49 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs On Thu, Oct 31, 2019 at 11:27 AM Olga Kornievskaia <aglo@umich.edu> wrote: > > Hi Trond, > > This patch set produces the following in my testing. Basically what I > see the client is prevented from using a delegation at all. > > After I induce a race of DELEGRETURN/OPEN > --- the racing OPEN gets a delegation (it returns the same seqid and > other as the delegation being returned) but the client doesn't use it. > --- the following (next) OPEN that also gets a delegation immediately > has the client returning the given delegation. > > Disclaimer: in my testing the racing DELEGRETURN doesn't fail with > OLD_STATEID, NetApp returns OK. Testing the same against Linux. It prevents the client from using future delegation stateid. On the induced DELEGRETURN/OPEN race, the linux server doesn't give a new read delegation. The following open gets a read delegation and returns it right away. > On Thu, Oct 24, 2019 at 6:56 AM Trond Myklebust <trondmy@gmail.com> wrote: > > > > The following patchset fixes up a number of issues with delegations, > > but mainly attempts to fix a race condition between open and > > delegreturn, where the open and the delegreturn get re-ordered so > > that the delegreturn ends up revoking the delegation that was returned > > by the open. > > The root cause is that in certain circumstances, we may currently end > > up freeing the delegation from delegreturn, so when we later receive > > the reply to the open, we've lost track of the fact that the seqid > > predates the one that was returned. > > > > This patchset fixes that case by ensuring that we always keep track > > of the last delegation stateid that was returned for any given inode. > > That way, if we later see a delegation stateid with the same opaque > > field, but an older seqid, we know we cannot trust it, and so we > > ask to replay the OPEN compound. > > > > Trond Myklebust (14): > > NFSv4: Don't allow a cached open with a revoked delegation > > NFSv4: Fix delegation handling in update_open_stateid() > > NFSv4: nfs4_callback_getattr() should ignore revoked delegations > > NFSv4: Delegation recalls should not find revoked delegations > > NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was > > revoked > > NFS: Rename nfs_inode_return_delegation_noreclaim() > > NFSv4: Don't remove the delegation from the super_list more than once > > NFSv4: Hold the delegation spinlock when updating the seqid > > NFSv4: Clear the NFS_DELEGATION_REVOKED flag in > > nfs_update_inplace_delegation() > > NFSv4: Update the stateid seqid in nfs_revoke_delegation() > > NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() > > NFSv4: Ignore requests to return the delegation if it was revoked > > NFSv4: Don't reclaim delegations that have been returned or revoked > > NFSv4: Fix races between open and delegreturn > > > > fs/nfs/callback_proc.c | 2 +- > > fs/nfs/delegation.c | 109 +++++++++++++++++++++++++++++------------ > > fs/nfs/delegation.h | 4 +- > > fs/nfs/nfs4proc.c | 13 ++--- > > fs/nfs/nfs4super.c | 4 +- > > 5 files changed, 88 insertions(+), 44 deletions(-) > > > > -- > > 2.21.0 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/14] Delegation bugfixes 2019-10-31 15:49 ` Olga Kornievskaia @ 2019-10-31 16:15 ` Olga Kornievskaia 2019-10-31 22:54 ` Trond Myklebust 0 siblings, 1 reply; 19+ messages in thread From: Olga Kornievskaia @ 2019-10-31 16:15 UTC (permalink / raw) To: Trond Myklebust; +Cc: linux-nfs Hi Trond, Now that I recall the reason the Ontap server gives out the same delegation was to deal with the linux client behaviour who sends an open even though it holds a delegation (according to the server's view). So what server does is it gives out the same delegation. This patch series changes the semantics. Can you describe what you expect the server is supposed to do in this case? On Thu, Oct 31, 2019 at 11:49 AM Olga Kornievskaia <aglo@umich.edu> wrote: > > On Thu, Oct 31, 2019 at 11:27 AM Olga Kornievskaia <aglo@umich.edu> wrote: > > > > Hi Trond, > > > > This patch set produces the following in my testing. Basically what I > > see the client is prevented from using a delegation at all. > > > > After I induce a race of DELEGRETURN/OPEN > > --- the racing OPEN gets a delegation (it returns the same seqid and > > other as the delegation being returned) but the client doesn't use it. > > --- the following (next) OPEN that also gets a delegation immediately > > has the client returning the given delegation. > > > > Disclaimer: in my testing the racing DELEGRETURN doesn't fail with > > OLD_STATEID, NetApp returns OK. > > Testing the same against Linux. It prevents the client from using > future delegation stateid. On the induced DELEGRETURN/OPEN race, the > linux server doesn't give a new read delegation. The following open > gets a read delegation and returns it right away. > > > > On Thu, Oct 24, 2019 at 6:56 AM Trond Myklebust <trondmy@gmail.com> wrote: > > > > > > The following patchset fixes up a number of issues with delegations, > > > but mainly attempts to fix a race condition between open and > > > delegreturn, where the open and the delegreturn get re-ordered so > > > that the delegreturn ends up revoking the delegation that was returned > > > by the open. > > > The root cause is that in certain circumstances, we may currently end > > > up freeing the delegation from delegreturn, so when we later receive > > > the reply to the open, we've lost track of the fact that the seqid > > > predates the one that was returned. > > > > > > This patchset fixes that case by ensuring that we always keep track > > > of the last delegation stateid that was returned for any given inode. > > > That way, if we later see a delegation stateid with the same opaque > > > field, but an older seqid, we know we cannot trust it, and so we > > > ask to replay the OPEN compound. > > > > > > Trond Myklebust (14): > > > NFSv4: Don't allow a cached open with a revoked delegation > > > NFSv4: Fix delegation handling in update_open_stateid() > > > NFSv4: nfs4_callback_getattr() should ignore revoked delegations > > > NFSv4: Delegation recalls should not find revoked delegations > > > NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was > > > revoked > > > NFS: Rename nfs_inode_return_delegation_noreclaim() > > > NFSv4: Don't remove the delegation from the super_list more than once > > > NFSv4: Hold the delegation spinlock when updating the seqid > > > NFSv4: Clear the NFS_DELEGATION_REVOKED flag in > > > nfs_update_inplace_delegation() > > > NFSv4: Update the stateid seqid in nfs_revoke_delegation() > > > NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() > > > NFSv4: Ignore requests to return the delegation if it was revoked > > > NFSv4: Don't reclaim delegations that have been returned or revoked > > > NFSv4: Fix races between open and delegreturn > > > > > > fs/nfs/callback_proc.c | 2 +- > > > fs/nfs/delegation.c | 109 +++++++++++++++++++++++++++++------------ > > > fs/nfs/delegation.h | 4 +- > > > fs/nfs/nfs4proc.c | 13 ++--- > > > fs/nfs/nfs4super.c | 4 +- > > > 5 files changed, 88 insertions(+), 44 deletions(-) > > > > > > -- > > > 2.21.0 > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/14] Delegation bugfixes 2019-10-31 16:15 ` Olga Kornievskaia @ 2019-10-31 22:54 ` Trond Myklebust 0 siblings, 0 replies; 19+ messages in thread From: Trond Myklebust @ 2019-10-31 22:54 UTC (permalink / raw) To: aglo; +Cc: linux-nfs Hi Olga On Thu, 2019-10-31 at 12:15 -0400, Olga Kornievskaia wrote: > Hi Trond, > > Now that I recall the reason the Ontap server gives out the same > delegation was to deal with the linux client behaviour who sends an > open even though it holds a delegation (according to the server's > view). So what server does is it gives out the same delegation. This > patch series changes the semantics. Can you describe what you expect > the server is supposed to do in this case? If the client sends out a second OPEN for the same file (presumably because an application request caused the file to be opened again before the client could process the reply to the first open), then I think it should be OK for the server to send the delegation stateid again. If the new open caused a state change (for instance the delegation was upgraded from READ to WRITE) then the seqid for the delegation stateid should be bumped. If there was not state change, it really should be up to the server whether or not it bumps the seqid. However on the client side, we want to be able to consider that if the 2 open replies return the exact same delegation stateid, then we consider the second reply to be a no-op as far as our update of the internal state is concerned. If the seqid was bumped, the client needs to update its internal state. So all this patch series is doing, is setting up rules to allow us to enforce that case, and in particular to ensure that _if_ we send a DELEGRETURN before we process the reply to the second open, then we recognise that a successful DELEGRETURN means we no longer hold delegation state, no matter what the contents of the reply to the second open tells us. > On Thu, Oct 31, 2019 at 11:49 AM Olga Kornievskaia <aglo@umich.edu> > wrote: > > On Thu, Oct 31, 2019 at 11:27 AM Olga Kornievskaia <aglo@umich.edu> > > wrote: > > > Hi Trond, > > > > > > This patch set produces the following in my testing. Basically > > > what I > > > see the client is prevented from using a delegation at all. > > > > > > After I induce a race of DELEGRETURN/OPEN > > > --- the racing OPEN gets a delegation (it returns the same seqid > > > and > > > other as the delegation being returned) but the client doesn't > > > use it. > > > --- the following (next) OPEN that also gets a delegation > > > immediately > > > has the client returning the given delegation. > > > > > > Disclaimer: in my testing the racing DELEGRETURN doesn't fail > > > with > > > OLD_STATEID, NetApp returns OK. > > > > Testing the same against Linux. It prevents the client from using > > future delegation stateid. On the induced DELEGRETURN/OPEN race, > > the > > linux server doesn't give a new read delegation. The following open > > gets a read delegation and returns it right away. > > > > > > > On Thu, Oct 24, 2019 at 6:56 AM Trond Myklebust < > > > trondmy@gmail.com> wrote: > > > > The following patchset fixes up a number of issues with > > > > delegations, > > > > but mainly attempts to fix a race condition between open and > > > > delegreturn, where the open and the delegreturn get re-ordered > > > > so > > > > that the delegreturn ends up revoking the delegation that was > > > > returned > > > > by the open. > > > > The root cause is that in certain circumstances, we may > > > > currently end > > > > up freeing the delegation from delegreturn, so when we later > > > > receive > > > > the reply to the open, we've lost track of the fact that the > > > > seqid > > > > predates the one that was returned. > > > > > > > > This patchset fixes that case by ensuring that we always keep > > > > track > > > > of the last delegation stateid that was returned for any given > > > > inode. > > > > That way, if we later see a delegation stateid with the same > > > > opaque > > > > field, but an older seqid, we know we cannot trust it, and so > > > > we > > > > ask to replay the OPEN compound. > > > > > > > > Trond Myklebust (14): > > > > NFSv4: Don't allow a cached open with a revoked delegation > > > > NFSv4: Fix delegation handling in update_open_stateid() > > > > NFSv4: nfs4_callback_getattr() should ignore revoked > > > > delegations > > > > NFSv4: Delegation recalls should not find revoked delegations > > > > NFSv4: fail nfs4_refresh_delegation_stateid() when the > > > > delegation was > > > > revoked > > > > NFS: Rename nfs_inode_return_delegation_noreclaim() > > > > NFSv4: Don't remove the delegation from the super_list more > > > > than once > > > > NFSv4: Hold the delegation spinlock when updating the seqid > > > > NFSv4: Clear the NFS_DELEGATION_REVOKED flag in > > > > nfs_update_inplace_delegation() > > > > NFSv4: Update the stateid seqid in nfs_revoke_delegation() > > > > NFSv4: Revoke the delegation on success in > > > > nfs4_delegreturn_done() > > > > NFSv4: Ignore requests to return the delegation if it was > > > > revoked > > > > NFSv4: Don't reclaim delegations that have been returned or > > > > revoked > > > > NFSv4: Fix races between open and delegreturn > > > > > > > > fs/nfs/callback_proc.c | 2 +- > > > > fs/nfs/delegation.c | 109 +++++++++++++++++++++++++++++-- > > > > ---------- > > > > fs/nfs/delegation.h | 4 +- > > > > fs/nfs/nfs4proc.c | 13 ++--- > > > > fs/nfs/nfs4super.c | 4 +- > > > > 5 files changed, 88 insertions(+), 44 deletions(-) > > > > > > > > -- > > > > 2.21.0 > > > > -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-10-31 22:54 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-23 23:55 [PATCH 00/14] Delegation bugfixes Trond Myklebust 2019-10-23 23:55 ` [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation Trond Myklebust 2019-10-23 23:55 ` [PATCH 02/14] NFSv4: Fix delegation handling in update_open_stateid() Trond Myklebust 2019-10-23 23:55 ` [PATCH 03/14] NFSv4: nfs4_callback_getattr() should ignore revoked delegations Trond Myklebust 2019-10-23 23:55 ` [PATCH 04/14] NFSv4: Delegation recalls should not find " Trond Myklebust 2019-10-23 23:55 ` [PATCH 05/14] NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was revoked Trond Myklebust 2019-10-23 23:55 ` [PATCH 06/14] NFS: Rename nfs_inode_return_delegation_noreclaim() Trond Myklebust 2019-10-23 23:55 ` [PATCH 07/14] NFSv4: Don't remove the delegation from the super_list more than once Trond Myklebust 2019-10-23 23:55 ` [PATCH 08/14] NFSv4: Hold the delegation spinlock when updating the seqid Trond Myklebust 2019-10-23 23:55 ` [PATCH 09/14] NFSv4: Clear the NFS_DELEGATION_REVOKED flag in nfs_update_inplace_delegation() Trond Myklebust 2019-10-23 23:55 ` [PATCH 10/14] NFSv4: Update the stateid seqid in nfs_revoke_delegation() Trond Myklebust 2019-10-23 23:55 ` [PATCH 11/14] NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() Trond Myklebust 2019-10-23 23:55 ` [PATCH 12/14] NFSv4: Ignore requests to return the delegation if it was revoked Trond Myklebust 2019-10-23 23:55 ` [PATCH 13/14] NFSv4: Don't reclaim delegations that have been returned or revoked Trond Myklebust 2019-10-23 23:56 ` [PATCH 14/14] NFSv4: Fix races between open and delegreturn Trond Myklebust 2019-10-31 15:27 ` [PATCH 00/14] Delegation bugfixes Olga Kornievskaia 2019-10-31 15:49 ` Olga Kornievskaia 2019-10-31 16:15 ` Olga Kornievskaia 2019-10-31 22:54 ` Trond Myklebust
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).