Commit 24311f88 authored by Trond Myklebust's avatar Trond Myklebust

NFSv4: Recovery of recalled read delegations is broken

When a read delegation is being recalled, and we're reclaiming the
cached opens, we need to make sure that we only reclaim read-only
modes.
A previous attempt to do this, relied on retrieving the delegation
type from the nfs4_opendata structure. Unfortunately, as Kinglong
pointed out, this field can only be set when performing reboot recovery.

Furthermore, if we call nfs4_open_recover(), then we end up clobbering
the state->flags for all modes that we're not recovering...

The fix is to have the delegation recall code pass this information
to the recovery call, and then refactor the recovery code so that
nfs4_open_delegation_recall() does not need to call nfs4_open_recover().
Reported-by: default avatarKinglong Mee <kinglongmee@gmail.com>
Fixes: 39f897fd ("NFSv4: When returning a delegation, don't...")
Tested-by: default avatarKinglong Mee <kinglongmee@gmail.com>
Cc: NeilBrown <neilb@suse.com>
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
parent 8714d46d
...@@ -113,7 +113,8 @@ static int nfs_delegation_claim_locks(struct nfs_open_context *ctx, struct nfs4_ ...@@ -113,7 +113,8 @@ static int nfs_delegation_claim_locks(struct nfs_open_context *ctx, struct nfs4_
return status; return status;
} }
static int nfs_delegation_claim_opens(struct inode *inode, const nfs4_stateid *stateid) static int nfs_delegation_claim_opens(struct inode *inode,
const nfs4_stateid *stateid, fmode_t type)
{ {
struct nfs_inode *nfsi = NFS_I(inode); struct nfs_inode *nfsi = NFS_I(inode);
struct nfs_open_context *ctx; struct nfs_open_context *ctx;
...@@ -140,7 +141,7 @@ static int nfs_delegation_claim_opens(struct inode *inode, const nfs4_stateid *s ...@@ -140,7 +141,7 @@ static int nfs_delegation_claim_opens(struct inode *inode, const nfs4_stateid *s
/* Block nfs4_proc_unlck */ /* Block nfs4_proc_unlck */
mutex_lock(&sp->so_delegreturn_mutex); mutex_lock(&sp->so_delegreturn_mutex);
seq = raw_seqcount_begin(&sp->so_reclaim_seqcount); seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
err = nfs4_open_delegation_recall(ctx, state, stateid); err = nfs4_open_delegation_recall(ctx, state, stateid, type);
if (!err) if (!err)
err = nfs_delegation_claim_locks(ctx, state, stateid); err = nfs_delegation_claim_locks(ctx, state, stateid);
if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq)) if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
...@@ -411,7 +412,8 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation ...@@ -411,7 +412,8 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
do { do {
if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
break; break;
err = nfs_delegation_claim_opens(inode, &delegation->stateid); err = nfs_delegation_claim_opens(inode, &delegation->stateid,
delegation->type);
if (!issync || err != -EAGAIN) if (!issync || err != -EAGAIN)
break; break;
/* /*
......
...@@ -54,7 +54,7 @@ void nfs_delegation_reap_unclaimed(struct nfs_client *clp); ...@@ -54,7 +54,7 @@ void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
/* NFSv4 delegation-related procedures */ /* NFSv4 delegation-related procedures */
int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync); int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync);
int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid); int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid); int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
bool nfs4_copy_delegation_stateid(nfs4_stateid *dst, struct inode *inode, fmode_t flags); bool nfs4_copy_delegation_stateid(nfs4_stateid *dst, struct inode *inode, fmode_t flags);
......
...@@ -1127,6 +1127,21 @@ static int nfs4_wait_for_completion_rpc_task(struct rpc_task *task) ...@@ -1127,6 +1127,21 @@ static int nfs4_wait_for_completion_rpc_task(struct rpc_task *task)
return ret; return ret;
} }
static bool nfs4_mode_match_open_stateid(struct nfs4_state *state,
fmode_t fmode)
{
switch(fmode & (FMODE_READ|FMODE_WRITE)) {
case FMODE_READ|FMODE_WRITE:
return state->n_rdwr != 0;
case FMODE_WRITE:
return state->n_wronly != 0;
case FMODE_READ:
return state->n_rdonly != 0;
}
WARN_ON_ONCE(1);
return false;
}
static int can_open_cached(struct nfs4_state *state, fmode_t mode, int open_mode) static int can_open_cached(struct nfs4_state *state, fmode_t mode, int open_mode)
{ {
int ret = 0; int ret = 0;
...@@ -1571,17 +1586,13 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context ...@@ -1571,17 +1586,13 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
return opendata; return opendata;
} }
static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmode, struct nfs4_state **res) static int nfs4_open_recover_helper(struct nfs4_opendata *opendata,
fmode_t fmode)
{ {
struct nfs4_state *newstate; struct nfs4_state *newstate;
int ret; int ret;
if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR || if (!nfs4_mode_match_open_stateid(opendata->state, fmode))
opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
(opendata->o_arg.u.delegation_type & fmode) != fmode)
/* This mode can't have been delegated, so we must have
* a valid open_stateid to cover it - not need to reclaim.
*/
return 0; return 0;
opendata->o_arg.open_flags = 0; opendata->o_arg.open_flags = 0;
opendata->o_arg.fmode = fmode; opendata->o_arg.fmode = fmode;
...@@ -1597,14 +1608,14 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod ...@@ -1597,14 +1608,14 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
newstate = nfs4_opendata_to_nfs4_state(opendata); newstate = nfs4_opendata_to_nfs4_state(opendata);
if (IS_ERR(newstate)) if (IS_ERR(newstate))
return PTR_ERR(newstate); return PTR_ERR(newstate);
if (newstate != opendata->state)
ret = -ESTALE;
nfs4_close_state(newstate, fmode); nfs4_close_state(newstate, fmode);
*res = newstate; return ret;
return 0;
} }
static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state) static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state)
{ {
struct nfs4_state *newstate;
int ret; int ret;
/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */ /* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
...@@ -1615,27 +1626,15 @@ static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state * ...@@ -1615,27 +1626,15 @@ static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *
clear_bit(NFS_DELEGATED_STATE, &state->flags); clear_bit(NFS_DELEGATED_STATE, &state->flags);
clear_bit(NFS_OPEN_STATE, &state->flags); clear_bit(NFS_OPEN_STATE, &state->flags);
smp_rmb(); smp_rmb();
if (state->n_rdwr != 0) { ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE, &newstate); if (ret != 0)
if (ret != 0) return ret;
return ret; ret = nfs4_open_recover_helper(opendata, FMODE_WRITE);
if (newstate != state) if (ret != 0)
return -ESTALE; return ret;
} ret = nfs4_open_recover_helper(opendata, FMODE_READ);
if (state->n_wronly != 0) { if (ret != 0)
ret = nfs4_open_recover_helper(opendata, FMODE_WRITE, &newstate); return ret;
if (ret != 0)
return ret;
if (newstate != state)
return -ESTALE;
}
if (state->n_rdonly != 0) {
ret = nfs4_open_recover_helper(opendata, FMODE_READ, &newstate);
if (ret != 0)
return ret;
if (newstate != state)
return -ESTALE;
}
/* /*
* We may have performed cached opens for all three recoveries. * We may have performed cached opens for all three recoveries.
* Check if we need to update the current stateid. * Check if we need to update the current stateid.
...@@ -1759,18 +1758,32 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct ...@@ -1759,18 +1758,32 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
return err; return err;
} }
int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid) int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
struct nfs4_state *state, const nfs4_stateid *stateid,
fmode_t type)
{ {
struct nfs_server *server = NFS_SERVER(state->inode); struct nfs_server *server = NFS_SERVER(state->inode);
struct nfs4_opendata *opendata; struct nfs4_opendata *opendata;
int err; int err = 0;
opendata = nfs4_open_recoverdata_alloc(ctx, state, opendata = nfs4_open_recoverdata_alloc(ctx, state,
NFS4_OPEN_CLAIM_DELEG_CUR_FH); NFS4_OPEN_CLAIM_DELEG_CUR_FH);
if (IS_ERR(opendata)) if (IS_ERR(opendata))
return PTR_ERR(opendata); return PTR_ERR(opendata);
nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid); nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
err = nfs4_open_recover(opendata, state); clear_bit(NFS_DELEGATED_STATE, &state->flags);
switch (type & (FMODE_READ|FMODE_WRITE)) {
case FMODE_READ|FMODE_WRITE:
case FMODE_WRITE:
err = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
if (err)
break;
err = nfs4_open_recover_helper(opendata, FMODE_WRITE);
if (err)
break;
case FMODE_READ:
err = nfs4_open_recover_helper(opendata, FMODE_READ);
}
nfs4_opendata_put(opendata); nfs4_opendata_put(opendata);
return nfs4_handle_delegation_recall_error(server, state, stateid, err); return nfs4_handle_delegation_recall_error(server, state, stateid, err);
} }
......
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