The function can_open_cached() is used to determine if we can open a file
using cached information. It does this by reading values from the state
structure, which I think means we need to be holding the so_lock to get the
right answer. The current code calls this function twice - once without the
lock, and then a second time with the lock to check the answer. I think
this is technically correct, since false positives are verified and false
negatives lead to the normal open code.
Even if the code is correct, it's not immediately obvious to me why it works
so I think it's worth taking a second look at.
What do you all think?
Anna
Anna Schumaker (1):
NFS: Clear up state owner lock usage
fs/nfs/nfs4proc.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
--
2.1.0
can_open_cached() reads values out of the state structure, meaning that
we need the so_lock to have a correct return value. As a bonus, this
helps clear up some potentially confusing code.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7dd8aca..18eb31c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1307,15 +1307,13 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
int ret = -EAGAIN;
for (;;) {
+ spin_lock(&state->owner->so_lock);
if (can_open_cached(state, fmode, open_mode)) {
- spin_lock(&state->owner->so_lock);
- if (can_open_cached(state, fmode, open_mode)) {
- update_open_stateflags(state, fmode);
- spin_unlock(&state->owner->so_lock);
- goto out_return_state;
- }
+ update_open_stateflags(state, fmode);
spin_unlock(&state->owner->so_lock);
+ goto out_return_state;
}
+ spin_unlock(&state->owner->so_lock);
rcu_read_lock();
delegation = rcu_dereference(nfsi->delegation);
if (!can_open_delegated(delegation, fmode)) {
--
2.1.0