Commit 34310430 authored by Trond Myklebust's avatar Trond Myklebust

NFSv4: Fix up another delegation related race

When we can update_open_stateid(), we need to be certain that we don't
race with a delegation return. While we could do this by grabbing the
nfs_client->cl_lock, a dedicated spin lock in the delegation structure
will scale better.
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 0cb2659b
...@@ -140,13 +140,17 @@ static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfs ...@@ -140,13 +140,17 @@ static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfs
if (delegation == NULL) if (delegation == NULL)
goto nomatch; goto nomatch;
spin_lock(&delegation->lock);
if (stateid != NULL && memcmp(delegation->stateid.data, stateid->data, if (stateid != NULL && memcmp(delegation->stateid.data, stateid->data,
sizeof(delegation->stateid.data)) != 0) sizeof(delegation->stateid.data)) != 0)
goto nomatch; goto nomatch_unlock;
list_del_rcu(&delegation->super_list); list_del_rcu(&delegation->super_list);
nfsi->delegation_state = 0; nfsi->delegation_state = 0;
rcu_assign_pointer(nfsi->delegation, NULL); rcu_assign_pointer(nfsi->delegation, NULL);
spin_unlock(&delegation->lock);
return delegation; return delegation;
nomatch_unlock:
spin_unlock(&delegation->lock);
nomatch: nomatch:
return NULL; return NULL;
} }
...@@ -172,6 +176,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct ...@@ -172,6 +176,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
delegation->change_attr = nfsi->change_attr; delegation->change_attr = nfsi->change_attr;
delegation->cred = get_rpccred(cred); delegation->cred = get_rpccred(cred);
delegation->inode = inode; delegation->inode = inode;
spin_lock_init(&delegation->lock);
spin_lock(&clp->cl_lock); spin_lock(&clp->cl_lock);
if (rcu_dereference(nfsi->delegation) != NULL) { if (rcu_dereference(nfsi->delegation) != NULL) {
......
...@@ -22,6 +22,7 @@ struct nfs_delegation { ...@@ -22,6 +22,7 @@ struct nfs_delegation {
long flags; long flags;
loff_t maxsize; loff_t maxsize;
__u64 change_attr; __u64 change_attr;
spinlock_t lock;
struct rcu_head rcu; struct rcu_head rcu;
}; };
......
...@@ -388,9 +388,8 @@ static void nfs_set_open_stateid(struct nfs4_state *state, nfs4_stateid *stateid ...@@ -388,9 +388,8 @@ static void nfs_set_open_stateid(struct nfs4_state *state, nfs4_stateid *stateid
write_sequnlock(&state->seqlock); write_sequnlock(&state->seqlock);
} }
static void update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, nfs4_stateid *deleg_stateid, int open_flags) static void __update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, const nfs4_stateid *deleg_stateid, int open_flags)
{ {
open_flags &= (FMODE_READ|FMODE_WRITE);
/* /*
* Protect the call to nfs4_state_set_mode_locked and * Protect the call to nfs4_state_set_mode_locked and
* serialise the stateid update * serialise the stateid update
...@@ -408,6 +407,45 @@ static void update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_sta ...@@ -408,6 +407,45 @@ static void update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_sta
spin_unlock(&state->owner->so_lock); spin_unlock(&state->owner->so_lock);
} }
static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stateid, nfs4_stateid *delegation, int open_flags)
{
struct nfs_inode *nfsi = NFS_I(state->inode);
struct nfs_delegation *deleg_cur;
int ret = 0;
open_flags &= (FMODE_READ|FMODE_WRITE);
rcu_read_lock();
deleg_cur = rcu_dereference(nfsi->delegation);
if (deleg_cur == NULL)
goto no_delegation;
spin_lock(&deleg_cur->lock);
if (nfsi->delegation != deleg_cur ||
(deleg_cur->type & open_flags) != open_flags)
goto no_delegation_unlock;
if (delegation == NULL)
delegation = &deleg_cur->stateid;
else if (memcmp(deleg_cur->stateid.data, delegation->data, NFS4_STATEID_SIZE) != 0)
goto no_delegation_unlock;
__update_open_stateid(state, open_stateid, &deleg_cur->stateid, open_flags);
ret = 1;
no_delegation_unlock:
spin_unlock(&deleg_cur->lock);
no_delegation:
rcu_read_unlock();
if (!ret && open_stateid != NULL) {
__update_open_stateid(state, open_stateid, NULL, open_flags);
ret = 1;
}
return ret;
}
static void nfs4_return_incompatible_delegation(struct inode *inode, mode_t open_flags) static void nfs4_return_incompatible_delegation(struct inode *inode, mode_t open_flags)
{ {
struct nfs_delegation *delegation; struct nfs_delegation *delegation;
...@@ -431,23 +469,23 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata) ...@@ -431,23 +469,23 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
nfs4_stateid stateid; nfs4_stateid stateid;
int ret = -EAGAIN; int ret = -EAGAIN;
rcu_read_lock();
delegation = rcu_dereference(nfsi->delegation);
for (;;) { for (;;) {
if (can_open_cached(state, open_mode)) { if (can_open_cached(state, open_mode)) {
spin_lock(&state->owner->so_lock); spin_lock(&state->owner->so_lock);
if (can_open_cached(state, open_mode)) { if (can_open_cached(state, open_mode)) {
update_open_stateflags(state, open_mode); update_open_stateflags(state, open_mode);
spin_unlock(&state->owner->so_lock); spin_unlock(&state->owner->so_lock);
rcu_read_unlock();
goto out_return_state; goto out_return_state;
} }
spin_unlock(&state->owner->so_lock); spin_unlock(&state->owner->so_lock);
} }
if (delegation == NULL) rcu_read_lock();
break; delegation = rcu_dereference(nfsi->delegation);
if (!can_open_delegated(delegation, open_mode)) if (delegation == NULL ||
!can_open_delegated(delegation, open_mode)) {
rcu_read_unlock();
break; break;
}
/* Save the delegation */ /* Save the delegation */
memcpy(stateid.data, delegation->stateid.data, sizeof(stateid.data)); memcpy(stateid.data, delegation->stateid.data, sizeof(stateid.data));
rcu_read_unlock(); rcu_read_unlock();
...@@ -455,19 +493,11 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata) ...@@ -455,19 +493,11 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
if (ret != 0) if (ret != 0)
goto out; goto out;
ret = -EAGAIN; ret = -EAGAIN;
rcu_read_lock();
delegation = rcu_dereference(nfsi->delegation); /* Try to update the stateid using the delegation */
/* If no delegation, try a cached open */ if (update_open_stateid(state, NULL, &stateid, open_mode))
if (delegation == NULL) goto out_return_state;
continue;
/* Is the delegation still valid? */
if (memcmp(stateid.data, delegation->stateid.data, sizeof(stateid.data)) != 0)
continue;
rcu_read_unlock();
update_open_stateid(state, NULL, &stateid, open_mode);
goto out_return_state;
} }
rcu_read_unlock();
out: out:
return ERR_PTR(ret); return ERR_PTR(ret);
out_return_state: out_return_state:
...@@ -480,7 +510,6 @@ static struct nfs4_state *nfs4_opendata_to_nfs4_state(struct nfs4_opendata *data ...@@ -480,7 +510,6 @@ static struct nfs4_state *nfs4_opendata_to_nfs4_state(struct nfs4_opendata *data
struct inode *inode; struct inode *inode;
struct nfs4_state *state = NULL; struct nfs4_state *state = NULL;
struct nfs_delegation *delegation; struct nfs_delegation *delegation;
nfs4_stateid *deleg_stateid = NULL;
int ret; int ret;
if (!data->rpc_done) { if (!data->rpc_done) {
...@@ -516,12 +545,9 @@ static struct nfs4_state *nfs4_opendata_to_nfs4_state(struct nfs4_opendata *data ...@@ -516,12 +545,9 @@ static struct nfs4_state *nfs4_opendata_to_nfs4_state(struct nfs4_opendata *data
data->owner->so_cred, data->owner->so_cred,
&data->o_res); &data->o_res);
} }
rcu_read_lock();
delegation = rcu_dereference(NFS_I(inode)->delegation); update_open_stateid(state, &data->o_res.stateid, NULL,
if (delegation != NULL) data->o_arg.open_flags);
deleg_stateid = &delegation->stateid;
update_open_stateid(state, &data->o_res.stateid, deleg_stateid, data->o_arg.open_flags);
rcu_read_unlock();
iput(inode); iput(inode);
out: out:
return state; return state;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment