From: NeilBrown <neilb@cse.unsw.edu.au>
From: "J. Bruce Fields" <bfields@fieldses.org>
From: Andros: Hold state_lock longer so the stateowner doesn't diseappear
out from under us before we get the chance to encode the replay. Don't
attempt to save replay if we failed to find a stateowner.
---
25-akpm/fs/nfsd/nfs4proc.c | 35 +++++++++++++++++++++++++++--------
25-akpm/fs/nfsd/nfs4state.c | 33 +++++++++++++++++++++++----------
25-akpm/fs/nfsd/nfs4xdr.c | 33 +++++++++++++++++++++------------
3 files changed, 71 insertions(+), 30 deletions(-)
diff -puN fs/nfsd/nfs4proc.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays fs/nfsd/nfs4proc.c
--- 25/fs/nfsd/nfs4proc.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays Thu Apr 8 14:32:50 2004
+++ 25-akpm/fs/nfsd/nfs4proc.c Thu Apr 8 14:32:50 2004
@@ -113,17 +113,24 @@ do_open_lookup(struct svc_rqst *rqstp, s
return status;
}
+/*
+ * nfs4_unlock_state() called in encode
+ */
static inline int
nfsd4_open(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open)
{
int status;
- dprintk("NFSD: nfsd4_open filename %.*s\n",
- (int)open->op_fname.len, open->op_fname.data);
+ dprintk("NFSD: nfsd4_open filename %.*s op_stateowner %p\n",
+ (int)open->op_fname.len, open->op_fname.data,
+ open->op_stateowner);
/* This check required by spec. */
if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
return nfserr_inval;
+ open->op_stateowner = NULL;
+ nfs4_lock_state();
+
/* check seqid for replay. set nfs4_owner */
status = nfsd4_process_open1(open);
if (status == NFSERR_REPLAY_ME) {
@@ -761,7 +768,9 @@ nfsd4_proc_compound(struct svc_rqst *rqs
break;
case OP_CLOSE:
op->status = nfsd4_close(rqstp, ¤t_fh, &op->u.close);
- op->replay = &op->u.close.cl_stateowner->so_replay;
+ if (op->u.close.cl_stateowner)
+ op->replay =
+ &op->u.close.cl_stateowner->so_replay;
break;
case OP_COMMIT:
op->status = nfsd4_commit(rqstp, ¤t_fh, &op->u.commit);
@@ -780,14 +789,18 @@ nfsd4_proc_compound(struct svc_rqst *rqs
break;
case OP_LOCK:
op->status = nfsd4_lock(rqstp, ¤t_fh, &op->u.lock);
- op->replay = &op->u.lock.lk_stateowner->so_replay;
+ if (op->u.lock.lk_stateowner)
+ op->replay =
+ &op->u.lock.lk_stateowner->so_replay;
break;
case OP_LOCKT:
op->status = nfsd4_lockt(rqstp, ¤t_fh, &op->u.lockt);
break;
case OP_LOCKU:
op->status = nfsd4_locku(rqstp, ¤t_fh, &op->u.locku);
- op->replay = &op->u.locku.lu_stateowner->so_replay;
+ if (op->u.locku.lu_stateowner)
+ op->replay =
+ &op->u.locku.lu_stateowner->so_replay;
break;
case OP_LOOKUP:
op->status = nfsd4_lookup(rqstp, ¤t_fh, &op->u.lookup);
@@ -802,15 +815,21 @@ nfsd4_proc_compound(struct svc_rqst *rqs
break;
case OP_OPEN:
op->status = nfsd4_open(rqstp, ¤t_fh, &op->u.open);
- op->replay = &op->u.open.op_stateowner->so_replay;
+ if (op->u.open.op_stateowner)
+ op->replay =
+ &op->u.open.op_stateowner->so_replay;
break;
case OP_OPEN_CONFIRM:
op->status = nfsd4_open_confirm(rqstp, ¤t_fh, &op->u.open_confirm);
- op->replay = &op->u.open_confirm.oc_stateowner->so_replay;
+ if (op->u.open_confirm.oc_stateowner)
+ op->replay =
+ &op->u.open_confirm.oc_stateowner->so_replay;
break;
case OP_OPEN_DOWNGRADE:
op->status = nfsd4_open_downgrade(rqstp, ¤t_fh, &op->u.open_downgrade);
- op->replay = &op->u.open_downgrade.od_stateowner->so_replay;
+ if (op->u.open_downgrade.od_stateowner)
+ op->replay =
+ &op->u.open_downgrade.od_stateowner->so_replay;
break;
case OP_PUTFH:
op->status = nfsd4_putfh(rqstp, ¤t_fh, &op->u.putfh);
diff -puN fs/nfsd/nfs4state.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays fs/nfsd/nfs4state.c
--- 25/fs/nfsd/nfs4state.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays Thu Apr 8 14:32:50 2004
+++ 25-akpm/fs/nfsd/nfs4state.c Thu Apr 8 14:32:50 2004
@@ -89,6 +89,9 @@ nfs4_lock_state(void)
down(&client_sema);
}
+/*
+ * nfs4_unlock_state(); called in encode
+ */
void
nfs4_unlock_state(void)
{
@@ -1069,6 +1072,8 @@ nfs4_file_downgrade(struct file *filp, u
* notfound:
* verify clientid
* create new owner
+ *
+ * called with nfs4_lock_state() held.
*/
int
nfsd4_process_open1(struct nfsd4_open *open)
@@ -1087,7 +1092,6 @@ nfsd4_process_open1(struct nfsd4_open *o
if (STALE_CLIENTID(&open->op_clientid))
goto out;
- nfs4_lock_state();
strhashval = ownerstr_hashval(clientid->cl_id, open->op_owner);
if (find_openstateowner_str(strhashval, open, &sop)) {
open->op_stateowner = sop;
@@ -1145,10 +1149,11 @@ instantiate_new_owner:
renew:
renew_client(sop->so_client);
out:
- nfs4_unlock_state();
return status;
}
-
+/*
+ * called with nfs4_lock_state() held.
+ */
int
nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open)
{
@@ -1170,7 +1175,6 @@ nfsd4_process_open2(struct svc_rqst *rqs
if (!TEST_ACCESS(open->op_share_access) || !TEST_DENY(open->op_share_deny))
goto out;
- nfs4_lock_state();
fi_hashval = file_hashval(ino);
if (find_file(fi_hashval, ino, &fp)) {
/* Search for conflicting share reservations */
@@ -1261,7 +1265,6 @@ out:
if (!open->op_stateowner->so_confirmed)
open->op_rflags |= NFS4_OPEN_RESULT_CONFIRM;
- nfs4_unlock_state();
return status;
out_free:
kfree(stp);
@@ -1576,11 +1579,15 @@ check_replay:
} else {
printk("NFSD: preprocess_seqid_op: bad seqid (expected %d, got %d\n", sop->so_seqid +1, seqid);
+ *sopp = NULL;
status = nfserr_bad_seqid;
}
goto out;
}
+/*
+ * nfs4_unlock_state(); called in encode
+ */
int
nfsd4_open_confirm(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open_confirm *oc)
{
@@ -1616,7 +1623,6 @@ nfsd4_open_confirm(struct svc_rqst *rqst
stp->st_stateid.si_generation);
status = nfs_ok;
out:
- nfs4_unlock_state();
return status;
}
@@ -1645,6 +1651,9 @@ reset_union_bmap_deny(unsigned long deny
}
}
+/*
+ * nfs4_unlock_state(); called in encode
+ */
int
nfsd4_open_downgrade(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open_downgrade *od)
@@ -1657,6 +1666,7 @@ nfsd4_open_downgrade(struct svc_rqst *rq
(int)current_fh->fh_dentry->d_name.len,
current_fh->fh_dentry->d_name.name);
+ od->od_stateowner = NULL;
status = nfserr_inval;
if (!TEST_ACCESS(od->od_share_access) || !TEST_DENY(od->od_share_deny))
goto out;
@@ -1690,10 +1700,12 @@ nfsd4_open_downgrade(struct svc_rqst *rq
memcpy(&od->od_stateid, &stp->st_stateid, sizeof(stateid_t));
status = nfs_ok;
out:
- nfs4_unlock_state();
return status;
}
+/*
+ * nfs4_unlock_state() called after encode
+ */
int
nfsd4_close(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_close *close)
{
@@ -1704,6 +1716,7 @@ nfsd4_close(struct svc_rqst *rqstp, stru
(int)current_fh->fh_dentry->d_name.len,
current_fh->fh_dentry->d_name.name);
+ close->cl_stateowner = NULL;
nfs4_lock_state();
/* check close_lru for replay */
if ((status = nfs4_preprocess_seqid_op(current_fh, close->cl_seqid,
@@ -1721,7 +1734,6 @@ nfsd4_close(struct svc_rqst *rqstp, stru
/* release_state_owner() calls nfsd_close() if needed */
release_state_owner(stp, &close->cl_stateowner, OPEN_STATE);
out:
- nfs4_unlock_state();
return status;
}
@@ -1924,6 +1936,8 @@ check_lock_length(u64 offset, u64 length
/*
* LOCK operation
+ *
+ * nfs4_unlock_state(); called in encode
*/
int
nfsd4_lock(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_lock *lock)
@@ -2088,7 +2102,6 @@ out_destroy_new_stateid:
release_state_owner(lock_stp, &lock->lk_stateowner, LOCK_STATE);
}
out:
- nfs4_unlock_state();
return status;
}
@@ -2199,6 +2212,7 @@ nfsd4_locku(struct svc_rqst *rqstp, stru
if (check_lock_length(locku->lu_offset, locku->lu_length))
return nfserr_inval;
+ locku->lu_stateowner = NULL;
nfs4_lock_state();
if ((status = nfs4_preprocess_seqid_op(current_fh,
@@ -2241,7 +2255,6 @@ nfsd4_locku(struct svc_rqst *rqstp, stru
memcpy(&locku->lu_stateid, &stp->st_stateid, sizeof(stateid_t));
out:
- nfs4_unlock_state();
return status;
out_nfserr:
diff -puN fs/nfsd/nfs4xdr.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays fs/nfsd/nfs4xdr.c
--- 25/fs/nfsd/nfs4xdr.c~kNFSdv4-8-of-10-Improve-how-locking-copes-with-replays Thu Apr 8 14:32:50 2004
+++ 25-akpm/fs/nfsd/nfs4xdr.c Thu Apr 8 14:32:50 2004
@@ -484,11 +484,14 @@ nfsd4_decode_access(struct nfsd4_compoun
DECODE_TAIL;
}
+#define NFS4_STATE_NOT_LOCKED ((void *)-1)
+
static int
nfsd4_decode_close(struct nfsd4_compoundargs *argp, struct nfsd4_close *close)
{
DECODE_HEAD;
+ close->cl_stateowner = NFS4_STATE_NOT_LOCKED;
READ_BUF(4 + sizeof(stateid_t));
READ32(close->cl_seqid);
READ32(close->cl_stateid.si_generation);
@@ -579,6 +582,7 @@ nfsd4_decode_lock(struct nfsd4_compounda
{
DECODE_HEAD;
+ lock->lk_stateowner = NFS4_STATE_NOT_LOCKED;
/*
* type, reclaim(boolean), offset, length, new_lock_owner(boolean)
*/
@@ -636,6 +640,7 @@ nfsd4_decode_locku(struct nfsd4_compound
{
DECODE_HEAD;
+ locku->lu_stateowner = NFS4_STATE_NOT_LOCKED;
READ_BUF(24 + sizeof(stateid_t));
READ32(locku->lu_type);
if ((locku->lu_type < NFS4_READ_LT) || (locku->lu_type > NFS4_WRITEW_LT))
@@ -671,6 +676,7 @@ nfsd4_decode_open(struct nfsd4_compounda
memset(open->op_bmval, 0, sizeof(open->op_bmval));
open->op_iattr.ia_valid = 0;
+ open->op_stateowner = NFS4_STATE_NOT_LOCKED;
/* seqid, share_access, share_deny, clientid, ownerlen */
READ_BUF(16 + sizeof(clientid_t));
@@ -746,6 +752,7 @@ nfsd4_decode_open_confirm(struct nfsd4_c
{
DECODE_HEAD;
+ open_conf->oc_stateowner = NFS4_STATE_NOT_LOCKED;
READ_BUF(4 + sizeof(stateid_t));
READ32(open_conf->oc_req_stateid.si_generation);
COPYMEM(&open_conf->oc_req_stateid.si_opaque, sizeof(stateid_opaque_t));
@@ -759,6 +766,7 @@ nfsd4_decode_open_downgrade(struct nfsd4
{
DECODE_HEAD;
+ open_down->od_stateowner = NFS4_STATE_NOT_LOCKED;
READ_BUF(4 + sizeof(stateid_t));
READ32(open_down->od_stateid.si_generation);
COPYMEM(&open_down->od_stateid.si_opaque, sizeof(stateid_opaque_t));
@@ -1259,7 +1267,8 @@ nfsd4_decode_compound(struct nfsd4_compo
*/
#define ENCODE_SEQID_OP_TAIL(stateowner) do { \
- if (seqid_mutating_err(nfserr) && stateowner) { \
+ if (seqid_mutating_err(nfserr) && stateowner \
+ && (stateowner != NFS4_STATE_NOT_LOCKED)) { \
if (stateowner->so_confirmed) \
stateowner->so_seqid++; \
stateowner->so_replay.rp_status = nfserr; \
@@ -1267,7 +1276,10 @@ nfsd4_decode_compound(struct nfsd4_compo
(((char *)(resp)->p - (char *)save)); \
memcpy(stateowner->so_replay.rp_buf, save, \
stateowner->so_replay.rp_buflen); \
- } } while(0)
+ } \
+ if (stateowner != NFS4_STATE_NOT_LOCKED) \
+ nfs4_unlock_state(); \
+ } while (0);
static u32 nfs4_ftypes[16] = {
@@ -1917,7 +1929,7 @@ nfsd4_encode_open(struct nfsd4_compoundr
ENCODE_SEQID_OP_HEAD;
if (nfserr)
- return;
+ goto out;
RESERVE_SPACE(36 + sizeof(stateid_t));
WRITE32(open->op_stateid.si_generation);
@@ -1972,7 +1984,7 @@ nfsd4_encode_open(struct nfsd4_compoundr
BUG();
}
/* XXX save filehandle here */
-
+out:
ENCODE_SEQID_OP_TAIL(open->op_stateowner);
}
@@ -2297,14 +2309,8 @@ nfsd4_encode_operation(struct nfsd4_comp
RESERVE_SPACE(8);
WRITE32(op->opnum);
- if ((op->opnum != OP_SETATTR) && (op->opnum != OP_LOCK) && (op->opnum != OP_LOCKT) && (op->opnum != OP_SETCLIENTID) && (op->status)) {
- *p++ = op->status;
- ADJUST_ARGS();
- return;
- } else {
- statp = p++; /* to be backfilled at the end */
- ADJUST_ARGS();
- }
+ statp = p++; /* to be backfilled at the end */
+ ADJUST_ARGS();
switch (op->opnum) {
case OP_ACCESS:
@@ -2408,6 +2414,8 @@ nfsd4_encode_operation(struct nfsd4_comp
*
* XDR note: do not encode rp->rp_buflen: the buffer contains the
* previously sent already encoded operation.
+ *
+ * called with nfs4_lock_state() held
*/
void
nfsd4_encode_replay(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
@@ -2425,6 +2433,7 @@ nfsd4_encode_replay(struct nfsd4_compoun
RESERVE_SPACE(rp->rp_buflen);
WRITEMEM(rp->rp_buf, rp->rp_buflen);
ADJUST_ARGS();
+ nfs4_unlock_state();
}
/*
_