On 18 Nov 2016, at 16:55, Anna Schumaker wrote:
Hi Ben,
On 11/18/2016 05:03 AM, Benjamin Coddington wrote:
While walking the list of lock_states, keep a reference on each
nfs4_lock_state to be checked, otherwise the lock state could be
removed
while the check performs TEST_STATEID and possible FREE_STATEID.
Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
---
fs/nfs/nfs4proc.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7897826d7c51..57d102d0f075 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2564,15 +2564,23 @@ static void
nfs41_check_delegation_stateid(struct nfs4_state *state)
static int nfs41_check_expired_locks(struct nfs4_state *state)
{
int status, ret = NFS_OK;
- struct nfs4_lock_state *lsp;
+ struct nfs4_lock_state *lsp, *prev = NULL;
struct nfs_server *server = NFS_SERVER(state->inode);
if (!test_bit(LK_STATE_IN_USE, &state->flags))
goto out;
+
+ spin_lock(&state->state_lock);
list_for_each_entry(lsp, &state->lock_states, ls_locks) {
if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) {
struct rpc_cred *cred = lsp->ls_state->owner->so_cred;
+ atomic_inc(&lsp->ls_count);
+ spin_unlock(&state->state_lock);
+
+ nfs4_put_lock_state(prev);
+ prev = lsp;
+
status = nfs41_test_and_free_expired_stateid(server,
&lsp->ls_stateid,
cred);
@@ -2587,8 +2595,11 @@ static int nfs41_check_expired_locks(struct
nfs4_state *state)
ret = status;
break;
Should this "break" be replaced with a "goto out" since we're no
longer holding the lock at this point?
Oh, yes otherwise we'd unlock twice. Thanks for catching that!
If it's changed from break to goto out then the last
nfs4_put_lock_state()
should be moved below the out: label as well in order to match the
atomic_inc(), and that makes the nfs4_put_lock_state() an additional
unnecessary call in the case that LK_STATE_IN_USE wasn't set.
So, maybe change the break to goto out, and add
nfs4_put_lock_state(prev)
just before it. I'll send that as a v2.
Ben
}
+ spin_lock(&state->state_lock);
}
- };
+ }
+ spin_unlock(&state->state_lock);
+ nfs4_put_lock_state(prev);
out:
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
