[PATCH kNFSd 0 of 18] Introduction

NeilBrown

author

2004-09-24 04:55 UTC #0 lore raw

PATCH 1/1 Introduction

Following are 18 patches against nfsd in 2.6.lastest. They are mostly nfsv4 related with patch 2 being the exception (It is just a code-cleanup patch).

Subject: [PATCH kNFSd 1 of 18] nfsd4 fix: nfsd oopsed when encountering a conflict with a local lock
Subject: [PATCH kNFSd 2 of 18] separate a little of logic from fh_verify into new function
Subject: [PATCH kNFSd 3 of 18] nfsd4: don't take i_sem around call to ->getxattr
Subject: [PATCH kNFSd 4 of 18] make sure getxattr inode op is non-NULL before calling it
Subject: [PATCH kNFSd 5 of 18] nfsd4: reference count stateowners
Subject: [PATCH kNFSd 6 of 18] nfsd4: take a reference to preserve stateowner through xdr replay code
Subject: [PATCH kNFSd 7 of 18] nfsd4: revert awkward extension of state lock over xdr for replay encoding
Subject: [PATCH kNFSd 8 of 18] nfsd4: fix race in xdr encoding of lock_denied response.
Subject: [PATCH kNFSd 9 of 18] remove incorrect stateid modification in nfsv4 open upgrade
Subject: [PATCH kNFSd 10 of 18] nfsd4: move open owner checks from nfsd4_process_open2 into new function
Subject: [PATCH kNFSd 11 of 18] nfsd4_open_locktype_posix_flag
Subject: [PATCH kNFSd 12 of 18] nfsd4: move seqid decrement on reclaim to separate function
Subject: [PATCH kNFSd 13 of 18] nfsd4: reorganize "if" in nfsd4_process_open2 to make test clearer
Subject: [PATCH kNFSd 14 of 18] nfsd4: move open_upgrade code into a separate function
Subject: [PATCH kNFSd 15 of 18] nfsd4: move some nfsd4_process_open2 code to nfs4_new_open
Subject: [PATCH kNFSd 16 of 18] Clean up nfsd4_process_open2
Subject: [PATCH kNFSd 17 of 18] nfsd4: fix putrootfh return
Subject: [PATCH kNFSd 18 of 18] nfsd4: move code to truncate on open to separate function

NeilBrown

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #1 lore raw

PATCH 1/1 nfsd4: don't take i_sem around call to ->getxattr

The ->getxattr op doesn't take the i_sem (see
Documentation/filesystems/Locking)

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/vfs.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

fs/nfsd/vfs.c renamed from fs/nfsd/vfs.c~current~ +5 -9

diff ./fs/nfsd/vfs.c~current~ ./fs/nfsd/vfs.c
@@ -448,39 +448,35 @@ _get_posix_acl(struct dentry *dentry, ch
 	int buflen, error = 0;
 	struct posix_acl *pacl = NULL;
 
-	down(&inode->i_sem);
-
 	buflen = inode->i_op->getxattr(dentry, key, NULL, 0);
 	if (buflen <= 0) {
 		error = buflen < 0 ? buflen : -ENODATA;
-		goto out_sem;
+		goto out_err;
 	}
 
 	buf = kmalloc(buflen, GFP_KERNEL);
 	if (buf == NULL) {
 		error = -ENOMEM;
-		goto out_sem;
+		goto out_err;
 	}
 
 	error = -EOPNOTSUPP;
 	if (inode->i_op && inode->i_op->getxattr) {
 		error = security_inode_getxattr(dentry, key);
 		if (error)
-			goto out_sem;
+			goto out_err;
 		error = inode->i_op->getxattr(dentry, key, buf, buflen);
 	}
 	if (error < 0)
-		goto out_sem;
+		goto out_err;
 
 	error = 0;
-	up(&inode->i_sem);
 
 	pacl = posix_acl_from_xattr(buf, buflen);
  out:
 	kfree(buf);
 	return pacl;
- out_sem:
-	up(&inode->i_sem);
+ out_err:
 	pacl = ERR_PTR(error);
 	goto out;
 }

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #2 lore raw

PATCH 1/1 nfsd4_open_locktype_posix_flag

In reply to NeilBrown

We should set the OPEN_RESULT_LOCKTYPE_POSIX flag in the return to open, to indicate that we support locking.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c  |    2 +-
 ./include/linux/nfs4.h |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +1 -1

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1383,7 +1383,7 @@ out:
 	/*
 	* To finish the open response, we just need to set the rflags.
 	*/
-	open->op_rflags = 0;
+	open->op_rflags = NFS4_OPEN_RESULT_LOCKTYPE_POSIX;
 	if (!open->op_stateowner->so_confirmed)
 		open->op_rflags |= NFS4_OPEN_RESULT_CONFIRM;

include/linux/nfs4.h renamed from include/linux/nfs4.h~current~ +1 -0

diff ./include/linux/nfs4.h~current~ ./include/linux/nfs4.h
@@ -35,6 +35,7 @@
 #define NFS4_FH_VOL_RENAME		0x0008
 
 #define NFS4_OPEN_RESULT_CONFIRM 0x0002
+#define NFS4_OPEN_RESULT_LOCKTYPE_POSIX 0x0004
 
 #define NFS4_SHARE_ACCESS_READ	0x0001
 #define NFS4_SHARE_ACCESS_WRITE	0x0002

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #3 lore raw

PATCH 1/1 nfsd4: move seqid decrement on reclaim to separate function

In reply to NeilBrown

Move seqid decrement on reclaim to separate function

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +16 ~~-11~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1274,6 +1274,20 @@ out:
 	return status;
 }
 
+/* decrement seqid on successful reclaim, it will be bumped in encode_open */
+static void
+nfs4_set_claim_prev(struct nfsd4_open *open, int *status)
+{
+	if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS) {
+		if (*status)
+			*status = nfserr_reclaim_bad;
+		else {
+			open->op_stateowner->so_confirmed = 1;
+			open->op_stateowner->so_seqid--;
+		}
+	}
+}
+
 /*
  * called with nfs4_lock_state() held.
  */

@@ -1369,17 +1383,8 @@ out:
 	if (fp && list_empty(&fp->fi_perfile))
 		release_file(fp);
 
-	if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS) {
-		if (status)
-			status = nfserr_reclaim_bad;
-		else {
-		/* successful reclaim. so_seqid is decremented because
-		* it will be bumped in encode_open
-		*/
-			open->op_stateowner->so_confirmed = 1;
-			open->op_stateowner->so_seqid--;
-		}
-	}
+	/* CLAIM_PREVIOUS has different error returns */
+	nfs4_set_claim_prev(open, &status);
 	/*
 	* To finish the open response, we just need to set the rflags.
 	*/

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #4 lore raw

PATCH 1/1 nfsd4: take a reference to preserve stateowner through xdr replay code

In reply to NeilBrown

Take a reference to preserve the stateowner through the xdr replay code, and simplify nfsd4_proc_compound a little.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4proc.c  |   46 +++++++++++++++++++++-------------------------
 ./fs/nfsd/nfs4state.c |   11 +++++++++++
 2 files changed, 32 insertions(+), 25 deletions(-)

fs/nfsd/nfs4proc.c renamed from fs/nfsd/nfs4proc.c~current~ +21 ~~-25~~

diff ./fs/nfsd/nfs4proc.c~current~ ./fs/nfsd/nfs4proc.c
@@ -201,7 +201,7 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 			status = NFSERR_REPLAY_ME;
 	}
 	if (status)
-		return status;
+		goto out;
 	if (open->op_claim_type == NFS4_OPEN_CLAIM_NULL) {
 	/*
 	 * This block of code will (1) set CURRENT_FH to the file being opened,

@@ -211,7 +211,7 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 	 */
 		status = do_open_lookup(rqstp, current_fh, open);
 		if (status)
-			return status;
+			goto out;
 	} else if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS) {
 	/*
 	* The CURRENT_FH is already set to the file being opened. This

@@ -221,10 +221,11 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 	*/
 		status = do_open_fhandle(rqstp, current_fh, open);
 		if (status)
-			return status;
+			goto out;
 	} else {
 		printk("NFSD: unsupported OPEN claim type\n");
-		return nfserr_inval;
+		status = nfserr_inval;
+		goto out;
 	}
 	/*
 	 * nfsd4_process_open2() does the actual opening of the file.  If

@@ -232,9 +233,10 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, current_fh, open);
-	if (status)
-		return status;
-	return 0;
+out:
+	if (open->op_stateowner)
+		nfs4_get_stateowner(open->op_stateowner);
+	return status;
 }
 
 /*

@@ -785,6 +787,7 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 	struct nfsd4_op	*op;
 	struct svc_fh	*current_fh = NULL;
 	struct svc_fh	*save_fh = NULL;
+	struct nfs4_stateowner *replay_owner = NULL;
 	int		slack_space;    /* in words, not bytes! */
 	int		status;

@@ -864,9 +867,7 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 			break;
 		case OP_CLOSE:
 			op->status = nfsd4_close(rqstp, current_fh, &op->u.close);
-			if (op->u.close.cl_stateowner)
-				op->replay =
-					&op->u.close.cl_stateowner->so_replay;
+			replay_owner = op->u.close.cl_stateowner;
 			break;
 		case OP_COMMIT:
 			op->status = nfsd4_commit(rqstp, current_fh, &op->u.commit);

@@ -885,18 +886,14 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 			break;
 		case OP_LOCK:
 			op->status = nfsd4_lock(rqstp, current_fh, &op->u.lock);
-			if (op->u.lock.lk_stateowner)
-				op->replay =
-					&op->u.lock.lk_stateowner->so_replay;
+			replay_owner = op->u.lock.lk_stateowner;
 			break;
 		case OP_LOCKT:
 			op->status = nfsd4_lockt(rqstp, current_fh, &op->u.lockt);
 			break;
 		case OP_LOCKU:
 			op->status = nfsd4_locku(rqstp, current_fh, &op->u.locku);
-			if (op->u.locku.lu_stateowner)
-				op->replay =
-					&op->u.locku.lu_stateowner->so_replay;
+			replay_owner = op->u.locku.lu_stateowner;
 			break;
 		case OP_LOOKUP:
 			op->status = nfsd4_lookup(rqstp, current_fh, &op->u.lookup);

@@ -911,21 +908,15 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 			break;
 		case OP_OPEN:
 			op->status = nfsd4_open(rqstp, current_fh, &op->u.open);
-			if (op->u.open.op_stateowner)
-				op->replay =
-					&op->u.open.op_stateowner->so_replay;
+			replay_owner = op->u.open.op_stateowner;
 			break;
 		case OP_OPEN_CONFIRM:
 			op->status = nfsd4_open_confirm(rqstp, current_fh, &op->u.open_confirm);
-			if (op->u.open_confirm.oc_stateowner)
-				op->replay =
-					&op->u.open_confirm.oc_stateowner->so_replay;
+			replay_owner = op->u.open_confirm.oc_stateowner;
 			break;
 		case OP_OPEN_DOWNGRADE:
 			op->status = nfsd4_open_downgrade(rqstp, current_fh, &op->u.open_downgrade);
-			if (op->u.open_downgrade.od_stateowner)
-				op->replay =
-					&op->u.open_downgrade.od_stateowner->so_replay;
+			replay_owner = op->u.open_downgrade.od_stateowner;
 			break;
 		case OP_PUTFH:
 			op->status = nfsd4_putfh(rqstp, current_fh, &op->u.putfh);

@@ -984,12 +975,17 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 
 encode_op:
 		if (op->status == NFSERR_REPLAY_ME) {
+			op->replay = &replay_owner->so_replay;
 			nfsd4_encode_replay(resp, op);
 			status = op->status = op->replay->rp_status;
 		} else {
 			nfsd4_encode_operation(resp, op);
 			status = op->status;
 		}
+		if (replay_owner && (replay_owner != (void *)(-1))) {
+			nfs4_put_stateowner(replay_owner);
+			replay_owner = NULL;
+		}
 	}
 
 out:

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +11 -0

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1749,6 +1749,8 @@ nfsd4_open_confirm(struct svc_rqst *rqst
 	status = nfs_ok;
 	first_state(sop->so_client);
 out:
+	if (oc->oc_stateowner)
+		nfs4_get_stateowner(oc->oc_stateowner);
 	return status;
 }

@@ -1826,6 +1828,8 @@ nfsd4_open_downgrade(struct svc_rqst *rq
 	memcpy(&od->od_stateid, &stp->st_stateid, sizeof(stateid_t));
 	status = nfs_ok;
 out:
+	if (od->od_stateowner)
+		nfs4_get_stateowner(od->od_stateowner);
 	return status;
 }

@@ -1860,6 +1864,8 @@ 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:
+	if (close->cl_stateowner)
+		nfs4_get_stateowner(close->cl_stateowner);
 	return status;
 }

@@ -2160,6 +2166,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struc
 		if ((lock_stp = alloc_init_lock_stateid(lock->lk_stateowner, 
 						fp, open_stp)) == NULL) {
 			release_stateowner(lock->lk_stateowner);
+			lock->lk_stateowner = NULL;
 			goto out;
 		}
 		/* bump the open seqid used to create the lock */

@@ -2258,6 +2265,8 @@ out_destroy_new_stateid:
 		release_state_owner(lock_stp, &lock->lk_stateowner, LOCK_STATE);
 	}
 out:
+	if (lock->lk_stateowner)
+		nfs4_get_stateowner(lock->lk_stateowner);
 	return status;
 }

@@ -2409,6 +2418,8 @@ nfsd4_locku(struct svc_rqst *rqstp, stru
 	memcpy(&locku->lu_stateid, &stp->st_stateid, sizeof(stateid_t));
 
 out:
+	if (locku->lu_stateowner)
+		nfs4_get_stateowner(locku->lu_stateowner);
 	return status;
 
 out_nfserr:

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #5 lore raw

PATCH 1/1 make sure getxattr inode op is non-NULL before calling it

In reply to NeilBrown

Make sure getxattr inode op is non-NULL before calling it.

Also, security hook should probably be called before calling the getxattr op the first time.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/vfs.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

fs/nfsd/vfs.c renamed from fs/nfsd/vfs.c~current~ +11 -9

diff ./fs/nfsd/vfs.c~current~ ./fs/nfsd/vfs.c
@@ -448,6 +448,16 @@ _get_posix_acl(struct dentry *dentry, ch
 	int buflen, error = 0;
 	struct posix_acl *pacl = NULL;
 
+	error = -EOPNOTSUPP;
+	if (inode->i_op == NULL)
+		goto out_err;
+	if (inode->i_op->getxattr == NULL)
+		goto out_err;
+
+	error = security_inode_getxattr(dentry, key);
+	if (error)
+		goto out_err;
+
 	buflen = inode->i_op->getxattr(dentry, key, NULL, 0);
 	if (buflen <= 0) {
 		error = buflen < 0 ? buflen : -ENODATA;

@@ -460,18 +470,10 @@ _get_posix_acl(struct dentry *dentry, ch
 		goto out_err;
 	}
 
-	error = -EOPNOTSUPP;
-	if (inode->i_op && inode->i_op->getxattr) {
-		error = security_inode_getxattr(dentry, key);
-		if (error)
-			goto out_err;
-		error = inode->i_op->getxattr(dentry, key, buf, buflen);
-	}
+	error = inode->i_op->getxattr(dentry, key, buf, buflen);
 	if (error < 0)
 		goto out_err;
 
-	error = 0;
-
 	pacl = posix_acl_from_xattr(buf, buflen);
  out:
 	kfree(buf);

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #6 lore raw

PATCH 1/1 nfsd4: revert awkward extension of state lock over xdr for replay encoding

In reply to NeilBrown

With the addition of a reference count, we no longer need the code that conditionally extended the nfs4 state lock over the xdr encode stage.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4proc.c  |    5 +----
 ./fs/nfsd/nfs4state.c |   25 ++++++-------------------
 ./fs/nfsd/nfs4xdr.c   |   23 ++++++++---------------
 3 files changed, 15 insertions(+), 38 deletions(-)

fs/nfsd/nfs4proc.c renamed from fs/nfsd/nfs4proc.c~current~ +1 -4

diff ./fs/nfsd/nfs4proc.c~current~ ./fs/nfsd/nfs4proc.c
@@ -161,9 +161,6 @@ do_open_fhandle(struct svc_rqst *rqstp, 
 }
 
 
-/*
- * nfs4_unlock_state() called in encode
- */
 static inline int
 nfsd4_open(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open)
 {

@@ -182,7 +179,6 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 	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 */

@@ -236,6 +232,7 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 out:
 	if (open->op_stateowner)
 		nfs4_get_stateowner(open->op_stateowner);
+	nfs4_unlock_state();
 	return status;
 }

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +6 ~~-19~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -93,9 +93,6 @@ nfs4_lock_state(void)
 	down(&client_sema);
 }
 
-/*
- * nfs4_unlock_state(); called in encode
- */
 void
 nfs4_unlock_state(void)
 {

@@ -1710,9 +1707,6 @@ first_state(struct nfs4_client *clp)
 		clp->cl_first_state = get_seconds();
 }
 
-/*
- * nfs4_unlock_state(); called in encode
- */
 int
 nfsd4_open_confirm(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open_confirm *oc)
 {

@@ -1727,7 +1721,6 @@ nfsd4_open_confirm(struct svc_rqst *rqst
 	if ((status = fh_verify(rqstp, current_fh, S_IFREG, 0)))
 		goto out;
 
-	oc->oc_stateowner = NULL;
 	nfs4_lock_state();
 
 	if ((status = nfs4_preprocess_seqid_op(current_fh, oc->oc_seqid,

@@ -1751,6 +1744,7 @@ nfsd4_open_confirm(struct svc_rqst *rqst
 out:
 	if (oc->oc_stateowner)
 		nfs4_get_stateowner(oc->oc_stateowner);
+	nfs4_unlock_state();
 	return status;
 }

@@ -1779,10 +1773,6 @@ 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)
 {

@@ -1794,10 +1784,8 @@ 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;
+		return nfserr_inval;
 
 	nfs4_lock_state();
 	if ((status = nfs4_preprocess_seqid_op(current_fh, od->od_seqid,

@@ -1830,6 +1818,7 @@ nfsd4_open_downgrade(struct svc_rqst *rq
 out:
 	if (od->od_stateowner)
 		nfs4_get_stateowner(od->od_stateowner);
+	nfs4_unlock_state();
 	return status;
 }

@@ -1846,7 +1835,6 @@ 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,

@@ -1866,6 +1854,7 @@ nfsd4_close(struct svc_rqst *rqstp, stru
 out:
 	if (close->cl_stateowner)
 		nfs4_get_stateowner(close->cl_stateowner);
+	nfs4_unlock_state();
 	return status;
 }

@@ -2084,8 +2073,6 @@ 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)

@@ -2110,7 +2097,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struc
 	if (check_lock_length(lock->lk_offset, lock->lk_length))
 		 return nfserr_inval;
 
-	lock->lk_stateowner = NULL;
 	nfs4_lock_state();
 
 	if (lock->lk_is_new) {

@@ -2267,6 +2253,7 @@ out_destroy_new_stateid:
 out:
 	if (lock->lk_stateowner)
 		nfs4_get_stateowner(lock->lk_stateowner);
+	nfs4_unlock_state();
 	return status;
 }

@@ -2375,7 +2362,6 @@ 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,

@@ -2420,6 +2406,7 @@ nfsd4_locku(struct svc_rqst *rqstp, stru
 out:
 	if (locku->lu_stateowner)
 		nfs4_get_stateowner(locku->lu_stateowner);
+	nfs4_unlock_state();
 	return status;
 
 out_nfserr:

fs/nfsd/nfs4xdr.c renamed from fs/nfsd/nfs4xdr.c~current~ +8 ~~-15~~

diff ./fs/nfsd/nfs4xdr.c~current~ ./fs/nfsd/nfs4xdr.c
@@ -545,14 +545,12 @@ 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;
+	close->cl_stateowner = NULL;
 	READ_BUF(4 + sizeof(stateid_t));
 	READ32(close->cl_seqid);
 	READ32(close->cl_stateid.si_generation);

@@ -642,7 +640,7 @@ nfsd4_decode_lock(struct nfsd4_compounda
 {
 	DECODE_HEAD;
 
-	lock->lk_stateowner = NFS4_STATE_NOT_LOCKED;
+	lock->lk_stateowner = NULL;
 	/*
 	* type, reclaim(boolean), offset, length, new_lock_owner(boolean)
 	*/

@@ -700,7 +698,7 @@ nfsd4_decode_locku(struct nfsd4_compound
 {
 	DECODE_HEAD;
 
-	locku->lu_stateowner = NFS4_STATE_NOT_LOCKED;
+	locku->lu_stateowner = NULL;
 	READ_BUF(24 + sizeof(stateid_t));
 	READ32(locku->lu_type);
 	if ((locku->lu_type < NFS4_READ_LT) || (locku->lu_type > NFS4_WRITEW_LT))

@@ -736,7 +734,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;
+	open->op_stateowner = NULL;
 
 	/* seqid, share_access, share_deny, clientid, ownerlen */
 	READ_BUF(16 + sizeof(clientid_t));

@@ -812,7 +810,7 @@ nfsd4_decode_open_confirm(struct nfsd4_c
 {
 	DECODE_HEAD;
 		    
-	open_conf->oc_stateowner = NFS4_STATE_NOT_LOCKED;
+	open_conf->oc_stateowner = NULL;
 	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));

@@ -826,7 +824,7 @@ nfsd4_decode_open_downgrade(struct nfsd4
 {
 	DECODE_HEAD;
 		    
-	open_down->od_stateowner = NFS4_STATE_NOT_LOCKED;
+	open_down->od_stateowner = NULL;
 	READ_BUF(4 + sizeof(stateid_t));
 	READ32(open_down->od_stateid.si_generation);
 	COPYMEM(&open_down->od_stateid.si_opaque, sizeof(stateid_opaque_t));

@@ -1327,8 +1325,7 @@ nfsd4_decode_compound(struct nfsd4_compo
  */
 
 #define ENCODE_SEQID_OP_TAIL(stateowner) do {			\
-	if (seqid_mutating_err(nfserr) && stateowner		\
-	    && (stateowner != NFS4_STATE_NOT_LOCKED)) { 	\
+	if (seqid_mutating_err(nfserr) && stateowner) { 	\
 		if (stateowner->so_confirmed)			\
 			stateowner->so_seqid++;			\
 		stateowner->so_replay.rp_status = nfserr;   	\

@@ -1336,10 +1333,7 @@ nfsd4_decode_compound(struct nfsd4_compo
 			  (((char *)(resp)->p - (char *)save)); \
 		memcpy(stateowner->so_replay.rp_buf, save,      \
  			stateowner->so_replay.rp_buflen); 	\
-	}							\
-	if (stateowner != NFS4_STATE_NOT_LOCKED)		\
-		nfs4_unlock_state();				\
-	} while (0);
+	} } while (0);
 
 
 static u32 nfs4_ftypes[16] = {

@@ -2573,7 +2567,6 @@ nfsd4_encode_replay(struct nfsd4_compoun
 	RESERVE_SPACE(rp->rp_buflen);
 	WRITEMEM(rp->rp_buf, rp->rp_buflen);
 	ADJUST_ARGS();
-	nfs4_unlock_state();
 }
 
 /*

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #7 lore raw

PATCH 1/1 remove incorrect stateid modification in nfsv4 open upgrade

In reply to NeilBrown

When we upgrade an open, we keep the same stateid and bump only the seqid.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |    2 --
 1 files changed, 2 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +0 -2

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1332,8 +1332,6 @@ nfsd4_process_open2(struct svc_rqst *rqs
 		/* remember the open */
 		set_bit(open->op_share_access, &stp->st_access_bmap);
 		set_bit(open->op_share_deny, &stp->st_deny_bmap);
-		/* bump the stateid */
-		update_stateid(&stp->st_stateid);
 	}
 	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n\n",
 	            stp->st_stateid.si_boot, stp->st_stateid.si_stateownerid,

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #8 lore raw

PATCH 1/1 nfsd4 fix: nfsd oopsed when encountering a conflict with a local lock

In reply to NeilBrown

Bug Fix: Non NFSD conflicting byte-range locks were causing an Oops.

Encode a zero length owner and zero clientid for non NFSD conflicting locks in the lock_denied response.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4xdr.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

fs/nfsd/nfs4xdr.c renamed from fs/nfsd/nfs4xdr.c~current~ +9 -4

diff ./fs/nfsd/nfs4xdr.c~current~ ./fs/nfsd/nfs4xdr.c
@@ -1995,13 +1995,18 @@ nfsd4_encode_lock_denied(struct nfsd4_co
 {
 	ENCODE_HEAD;
 
-	RESERVE_SPACE(32 + XDR_LEN(ld->ld_sop->so_owner.len));
+	RESERVE_SPACE(32 + XDR_LEN(ld->ld_sop ? ld->ld_sop->so_owner.len : 0));
 	WRITE64(ld->ld_start);
 	WRITE64(ld->ld_length);
 	WRITE32(ld->ld_type);
-	WRITEMEM(&ld->ld_sop->so_client->cl_clientid, 8);
-	WRITE32(ld->ld_sop->so_owner.len);
-	WRITEMEM(ld->ld_sop->so_owner.data, ld->ld_sop->so_owner.len);
+	if (ld->ld_sop) {
+		WRITEMEM(&ld->ld_sop->so_client->cl_clientid, 8);
+		WRITE32(ld->ld_sop->so_owner.len);
+		WRITEMEM(ld->ld_sop->so_owner.data, ld->ld_sop->so_owner.len);
+	}  else {  /* non - nfsv4 lock in conflict, no clientid nor owner */
+		WRITE64((u64)0); /* clientid */
+		WRITE32(0); /* length of owner name */
+	}
 	ADJUST_ARGS();
 }

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #9 lore raw

PATCH 1/1 separate a little of logic from fh_verify into new function

In reply to NeilBrown

Separate out the little bit of logic in fh_verify that checks the i_mode into a separate function. This is nicely encapsulated, fh_verify() is overly long anyway, and this change helps make a subsequent nfs4 named attribute change clearer.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfsfh.c |   62 +++++++++++++++++++++++++++---------------------------
 1 files changed, 31 insertions(+), 31 deletions(-)

fs/nfsd/nfsfh.c renamed from fs/nfsd/nfsfh.c~current~ +31 ~~-31~~

diff ./fs/nfsd/nfsfh.c~current~ ./fs/nfsd/nfsfh.c
@@ -71,6 +71,35 @@ int nfsd_acceptable(void *expv, struct d
 	return rv;
 }
 
+/* Type check. The correct error return for type mismatches does not seem to be
+ * generally agreed upon. SunOS seems to use EISDIR if file isn't S_IFREG; a
+ * comment in the NFSv3 spec says this is incorrect (implementation notes for
+ * the write call).
+ */
+static inline int
+nfsd_mode_check(struct svc_rqst *rqstp, umode_t mode, int type)
+{
+	/* Type can be negative when creating hardlinks - not to a dir */
+	if (type > 0 && (mode & S_IFMT) != type) {
+		if (rqstp->rq_vers == 4 && (mode & S_IFMT) == S_IFLNK)
+			return nfserr_symlink;
+		else if (type == S_IFDIR)
+			return nfserr_notdir;
+		else if ((mode & S_IFMT) == S_IFDIR)
+			return nfserr_isdir;
+		else
+			return nfserr_inval;
+	}
+	if (type < 0 && (mode & S_IFMT) == -type) {
+		if (rqstp->rq_vers == 4 && (mode & S_IFMT) == S_IFLNK)
+			return nfserr_symlink;
+		else if (type == -S_IFDIR)
+			return nfserr_isdir;
+		else
+			return nfserr_notdir;
+	}
+	return 0;
+}
 
 /*
  * Perform sanity checks on the dentry in a client's file handle.

@@ -87,7 +116,6 @@ fh_verify(struct svc_rqst *rqstp, struct
 	struct knfsd_fh	*fh = &fhp->fh_handle;
 	struct svc_export *exp = NULL;
 	struct dentry	*dentry;
-	struct inode	*inode;
 	u32		error = 0;
 
 	dprintk("nfsd: fh_verify(%s)\n", SVCFH_fmt(fhp));

@@ -223,37 +251,9 @@ fh_verify(struct svc_rqst *rqstp, struct
 	}
 	cache_get(&exp->h);
 
-	inode = dentry->d_inode;
-
-
-	/* Type check. The correct error return for type mismatches
-	 * does not seem to be generally agreed upon. SunOS seems to
-	 * use EISDIR if file isn't S_IFREG; a comment in the NFSv3
-	 * spec says this is incorrect (implementation notes for the
-	 * write call).
-	 */
-
-	/* Type can be negative when creating hardlinks - not to a dir */
-	if (type > 0 && (inode->i_mode & S_IFMT) != type) {
-		if (rqstp->rq_vers == 4 && (inode->i_mode & S_IFMT) == S_IFLNK)
-			error = nfserr_symlink;
-		else if (type == S_IFDIR)
-			error = nfserr_notdir;
-		else if ((inode->i_mode & S_IFMT) == S_IFDIR)
-			error = nfserr_isdir;
-		else
-			error = nfserr_inval;
+	error = nfsd_mode_check(rqstp, dentry->d_inode->i_mode, type);
+	if (error)
 		goto out;
-	}
-	if (type < 0 && (inode->i_mode & S_IFMT) == -type) {
-		if (rqstp->rq_vers == 4 && (inode->i_mode & S_IFMT) == S_IFLNK)
-			error = nfserr_symlink;
-		else if (type == -S_IFDIR)
-			error = nfserr_isdir;
-		else
-			error = nfserr_notdir;
-		goto out;
-	}
 
 	/* Finally, check access permissions. */
 	error = nfsd_permission(exp, dentry, access);

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #10 lore raw

PATCH 1/1 nfsd4: reference count stateowners

In reply to NeilBrown

Reference-counting the nfsd4 stateowner structs will let us fix a race and simplify some of the xdr code a bit, and may also help us make the nfsd4 locking a little more fine-grained in the future.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c        |   26 +++++++++++++-------------
 ./include/linux/nfsd/state.h |   16 ++++++++++++++++
 2 files changed, 29 insertions(+), 13 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +13 ~~-13~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -833,6 +833,16 @@ release_all_files(void)
 	}
 }
 
+/* should use a slab cache */
+void
+nfs4_free_stateowner(struct kref *kref) {
+	struct nfs4_stateowner *sop =
+		container_of(kref, struct nfs4_stateowner, so_ref);
+	kfree(sop->so_owner.data);
+	kfree(sop);
+	free_sowner++;
+}
+
 static inline struct nfs4_stateowner *
 alloc_stateowner(struct xdr_netobj *owner)
 {

@@ -842,6 +852,7 @@ alloc_stateowner(struct xdr_netobj *owne
 		if ((sop->so_owner.data = kmalloc(owner->len, GFP_KERNEL))) {
 			memcpy(sop->so_owner.data, owner->data, owner->len);
 			sop->so_owner.len = owner->len;
+			kref_init(&sop->so_ref);
 			return sop;
 		} 
 		kfree(sop);

@@ -849,17 +860,6 @@ alloc_stateowner(struct xdr_netobj *owne
 	return NULL;
 }
 
-/* should use a slab cache */
-static void
-free_stateowner(struct nfs4_stateowner *sop) {
-	if (sop) {
-		kfree(sop->so_owner.data);
-		kfree(sop);
-		sop = NULL;
-		free_sowner++;
-	}
-}
-
 static struct nfs4_stateowner *
 alloc_init_open_stateowner(unsigned int strhashval, struct nfs4_client *clp, struct nfsd4_open *open) {
 	struct nfs4_stateowner *sop;

@@ -932,7 +932,7 @@ release_stateowner(struct nfs4_stateowne
 {
 	unhash_stateowner(sop);
 	list_del(&sop->so_close_lru);
-	free_stateowner(sop);
+	nfs4_put_stateowner(sop);
 }
 
 static inline void

@@ -1460,7 +1460,7 @@ nfs4_laundromat(void)
 		dprintk("NFSD: purging unused open stateowner (so_id %d)\n",
 			sop->so_id);
 		list_del(&sop->so_close_lru);
-		free_stateowner(sop);
+		nfs4_put_stateowner(sop);
 	}
 	if (clientid_val < NFSD_LAUNDROMAT_MINTIMEOUT)
 		clientid_val = NFSD_LAUNDROMAT_MINTIMEOUT;

include/linux/nfsd/state.h renamed from include/linux/nfsd/state.h~current~ +16 -0

diff ./include/linux/nfsd/state.h~current~ ./include/linux/nfsd/state.h
@@ -38,6 +38,7 @@
 #define _NFSD4_STATE_H
 
 #include <linux/list.h>
+#include <linux/kref.h>
 #include <linux/sunrpc/clnt.h>
 
 #define NFS4_OPAQUE_LIMIT 1024

@@ -168,6 +169,7 @@ struct nfs4_replay {
 *         reaped by laundramat thread after lease period.
 */
 struct nfs4_stateowner {
+	struct kref		so_ref;
 	struct list_head        so_idhash;   /* hash by so_id */
 	struct list_head        so_strhash;   /* hash by op_name */
 	struct list_head        so_perclient; /* nfs4_client->cl_perclient */

@@ -248,4 +250,18 @@ extern void nfs4_lock_state(void);
 extern void nfs4_unlock_state(void);
 extern int nfs4_in_grace(void);
 extern int nfs4_check_open_reclaim(clientid_t *clid);
+extern void nfs4_free_stateowner(struct kref *kref);
+
+static inline void
+nfs4_put_stateowner(struct nfs4_stateowner *so)
+{
+	kref_put(&so->so_ref, nfs4_free_stateowner);
+}
+
+static inline void
+nfs4_get_stateowner(struct nfs4_stateowner *so)
+{
+	kref_get(&so->so_ref);
+}
+
 #endif   /* NFSD4_STATE_H */

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #11 lore raw

PATCH 1/1 nfsd4: move open owner checks from nfsd4_process_open2 into new function

In reply to NeilBrown

nfsd4_process_open2 has become a bit long and contorted. The following patches break nfsd4_process_open2 into smaller functions and add comments to describe logic flow, in preparation for delegation state.

We begin by pulling out the code that searches for conflicting open owners into a separate function.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   49 +++++++++++++++++++++++++++++++++----------------
 1 files changed, 33 insertions(+), 16 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +33 ~~-16~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1249,6 +1249,31 @@ out:
 		status = nfserr_reclaim_bad;
 	return status;
 }
+
+static int
+nfs4_check_open(struct nfs4_file *fp, struct nfs4_stateowner *sop, struct nfsd4_open *open, struct nfs4_stateid **stpp)
+{
+	struct nfs4_stateid *local;
+	int status = nfserr_share_denied;
+
+	list_for_each_entry(local, &fp->fi_perfile, st_perfile) {
+		/* have we seen this open owner */
+		if (local->st_stateowner == sop) {
+			*stpp = local;
+			continue;
+		}
+		/* ignore lock owners */
+		if (local->st_stateowner->so_is_open_owner == 0)
+			continue;
+		/* check for conflicting share reservations */
+		if (!test_share(local, open))
+			goto out;
+	}
+	status = 0;
+out:
+	return status;
+}
+
 /*
  * called with nfs4_lock_state() held.
  */

@@ -1260,7 +1285,7 @@ nfsd4_process_open2(struct svc_rqst *rqs
 	struct nfs4_file *fp = NULL;
 	struct inode *ino;
 	unsigned int fi_hashval;
-	struct nfs4_stateid *stq, *stp = NULL;
+	struct nfs4_stateid *stp = NULL;
 	int status;
 
 	status = nfserr_resource;

@@ -1272,24 +1297,16 @@ nfsd4_process_open2(struct svc_rqst *rqs
 	status = nfserr_inval;
 	if (!TEST_ACCESS(open->op_share_access) || !TEST_DENY(open->op_share_deny))
 		goto out;
-
+	/*
+	 * Lookup file; if found, lookup stateid and check open request;
+	 * not found, create
+	 */
 	fi_hashval = file_hashval(ino);
 	if (find_file(fi_hashval, ino, &fp)) {
-		/* Search for conflicting share reservations */
-		status = nfserr_share_denied;
-		list_for_each_entry(stq, &fp->fi_perfile, st_perfile) {
-			if (stq->st_stateowner == sop) {
-				stp = stq;
-				continue;
-			}
-			/* ignore lock owners */
-			if (stq->st_stateowner->so_is_open_owner == 0)
-				continue;
-			if (!test_share(stq,open))	
-				goto out;
-		}
+		status = nfs4_check_open(fp, sop, open, &stp);
+		if (status)
+			goto out;
 	} else {
-	/* No nfs4_file found; allocate and init a new one */
 		status = nfserr_resource;
 		if ((fp = alloc_init_file(fi_hashval, ino)) == NULL)
 			goto out;

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #12 lore raw

PATCH 1/1 nfsd4: reorganize "if" in nfsd4_process_open2 to make test clearer

In reply to NeilBrown

Reorganize "if" in nfsd4_process_open2 to make test clearer; there was no real reason to test for the negative here.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +17 ~~-17~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1326,7 +1326,23 @@ nfsd4_process_open2(struct svc_rqst *rqs
 			goto out;
 	}
 
-	if (!stp) {
+	if (stp) {
+		/* This is an upgrade of an existing OPEN.
+		 * OR the incoming share with the existing
+		 * nfs4_stateid share */
+		unsigned int share_access;
+
+		set_access(&share_access, stp->st_access_bmap);
+		share_access = ~share_access;
+		share_access &= open->op_share_access;
+
+		/* update the struct file */
+		if ((status = nfs4_file_upgrade(stp->st_vfs_file, share_access)))
+			goto out;
+		/* remember the open */
+		set_bit(open->op_share_access, &stp->st_access_bmap);
+		set_bit(open->op_share_deny, &stp->st_deny_bmap);
+	} else {
 		int flags = 0;
 
 		status = nfserr_resource;

@@ -1347,22 +1363,6 @@ nfsd4_process_open2(struct svc_rqst *rqs
 
 		init_stateid(stp, fp, sop, open);
 		stp->st_vfs_set = 1;
-	} else {
-		/* This is an upgrade of an existing OPEN. 
-		 * OR the incoming share with the existing 
-		 * nfs4_stateid share */
-		unsigned int share_access;
-
-		set_access(&share_access, stp->st_access_bmap);
-		share_access = ~share_access;
-		share_access &= open->op_share_access;
-
-		/* update the struct file */
-		if ((status = nfs4_file_upgrade(stp->st_vfs_file, share_access)))
-			goto out;
-		/* remember the open */
-		set_bit(open->op_share_access, &stp->st_access_bmap);
-		set_bit(open->op_share_deny, &stp->st_deny_bmap);
 	}
 	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n\n",
 	            stp->st_stateid.si_boot, stp->st_stateid.si_stateownerid,

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #13 lore raw

PATCH 1/1 nfsd4: fix race in xdr encoding of lock_denied response.

In reply to NeilBrown

We take a reference on the stateowner, and copy the clientid, instead of just hoping no one destroys the stateowner before we reference it in nfsd4_encode_lock_denied.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c       |    5 ++++-
 ./fs/nfsd/nfs4xdr.c         |    3 ++-
 ./include/linux/nfsd/xdr4.h |    1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +4 -1

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1948,8 +1948,11 @@ nfs4_set_lock_denied(struct file_lock *f
 	unsigned int hval = lockownerid_hashval(sop->so_id);
 
 	deny->ld_sop = NULL;
-	if (nfs4_verify_lock_stateowner(sop, hval))
+	if (nfs4_verify_lock_stateowner(sop, hval)) {
+		kref_get(&sop->so_ref);
 		deny->ld_sop = sop;
+		deny->ld_clientid = sop->so_client->cl_clientid;
+	}
 	deny->ld_start = fl->fl_start;
 	deny->ld_length = ~(u64)0;
 	if (fl->fl_end != ~(u64)0)

fs/nfsd/nfs4xdr.c renamed from fs/nfsd/nfs4xdr.c~current~ +2 -1

diff ./fs/nfsd/nfs4xdr.c~current~ ./fs/nfsd/nfs4xdr.c
@@ -1994,9 +1994,10 @@ nfsd4_encode_lock_denied(struct nfsd4_co
 	WRITE64(ld->ld_length);
 	WRITE32(ld->ld_type);
 	if (ld->ld_sop) {
-		WRITEMEM(&ld->ld_sop->so_client->cl_clientid, 8);
+		WRITEMEM(&ld->ld_clientid, 8);
 		WRITE32(ld->ld_sop->so_owner.len);
 		WRITEMEM(ld->ld_sop->so_owner.data, ld->ld_sop->so_owner.len);
+		kref_put(&ld->ld_sop->so_ref, nfs4_free_stateowner);
 	}  else {  /* non - nfsv4 lock in conflict, no clientid nor owner */
 		WRITE64((u64)0); /* clientid */
 		WRITE32(0); /* length of owner name */

include/linux/nfsd/xdr4.h renamed from include/linux/nfsd/xdr4.h~current~ +1 -0

diff ./include/linux/nfsd/xdr4.h~current~ ./include/linux/nfsd/xdr4.h
@@ -116,6 +116,7 @@ struct nfsd4_link {
 };
 
 struct nfsd4_lock_denied {
+	clientid_t	ld_clientid;
 	struct nfs4_stateowner   *ld_sop;
 	u64             ld_start;
 	u64             ld_length;

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:55 UTC #14 lore raw

PATCH 1/1 nfsd4: move open_upgrade code into a separate function

In reply to NeilBrown

Move most of the code for the open_upgrade case into a separate nfs4_upgrade_open.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   58 +++++++++++++++++++++++++-------------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +30 ~~-28~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1131,20 +1131,6 @@ nfs4_share_conflict(struct svc_fh *curre
 	return nfs_ok;
 }
 
-static inline int
-nfs4_file_upgrade(struct file *filp, unsigned int share_access)
-{
-int status;
-
-	if (share_access & NFS4_SHARE_ACCESS_WRITE) {
-		status = get_write_access(filp->f_dentry->d_inode);
-		if (status)
-			return nfserrno(status);
-		filp->f_mode = (filp->f_mode | FMODE_WRITE) & ~FMODE_READ;
-	}
-	return nfs_ok;
-}
-
 static inline void
 nfs4_file_downgrade(struct file *filp, unsigned int share_access)
 {

@@ -1274,6 +1260,33 @@ out:
 	return status;
 }
 
+static int
+nfs4_upgrade_open(struct svc_rqst *rqstp, struct svc_fh *cur_fh, struct nfs4_stateid *stp, struct nfsd4_open *open)
+{
+	struct file *filp = stp->st_vfs_file;
+	struct inode *inode = filp->f_dentry->d_inode;
+	unsigned int share_access;
+	int status;
+
+	set_access(&share_access, stp->st_access_bmap);
+	share_access = ~share_access;
+	share_access &= open->op_share_access;
+
+	/* update the struct file */
+	if (share_access & NFS4_SHARE_ACCESS_WRITE) {
+		status = get_write_access(inode);
+		if (status)
+			return nfserrno(status);
+
+		/* remember the open */
+		filp->f_mode = (filp->f_mode | FMODE_WRITE) & ~FMODE_READ;
+		set_bit(open->op_share_access, &stp->st_access_bmap);
+		set_bit(open->op_share_deny, &stp->st_deny_bmap);
+	}
+	return nfs_ok;
+}
+
+
 /* decrement seqid on successful reclaim, it will be bumped in encode_open */
 static void
 nfs4_set_claim_prev(struct nfsd4_open *open, int *status)

@@ -1327,21 +1340,10 @@ nfsd4_process_open2(struct svc_rqst *rqs
 	}
 
 	if (stp) {
-		/* This is an upgrade of an existing OPEN.
-		 * OR the incoming share with the existing
-		 * nfs4_stateid share */
-		unsigned int share_access;
-
-		set_access(&share_access, stp->st_access_bmap);
-		share_access = ~share_access;
-		share_access &= open->op_share_access;
-
-		/* update the struct file */
-		if ((status = nfs4_file_upgrade(stp->st_vfs_file, share_access)))
+		/* Stateid was found, this is an OPEN upgrade */
+		status = nfs4_upgrade_open(rqstp, current_fh, stp, open);
+		if (status)
 			goto out;
-		/* remember the open */
-		set_bit(open->op_share_access, &stp->st_access_bmap);
-		set_bit(open->op_share_deny, &stp->st_deny_bmap);
 	} else {
 		int flags = 0;

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:56 UTC #15 lore raw

PATCH 1/1 Clean up nfsd4_process_open2

In reply to NeilBrown

Comments, dprintk cleanup

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +6 -1

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1363,6 +1363,10 @@ nfsd4_process_open2(struct svc_rqst *rqs
 			goto out;
 	}
 
+	/*
+	 * OPEN the file, or upgrade an existing OPEN.
+	 * If truncate fails, the OPEN fails.
+	 */
 	if (stp) {
 		/* Stateid was found, this is an OPEN upgrade */
 		status = nfs4_upgrade_open(rqstp, current_fh, stp, open);

@@ -1379,7 +1383,7 @@ nfsd4_process_open2(struct svc_rqst *rqs
 			goto out;
 		init_stateid(stp, fp, sop, open);
 	}
-	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n\n",
+	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n",
 	            stp->st_stateid.si_boot, stp->st_stateid.si_stateownerid,
 	            stp->st_stateid.si_fileid, stp->st_stateid.si_generation);

@@ -1395,6 +1399,7 @@ nfsd4_process_open2(struct svc_rqst *rqs
 	open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE;
 	status = nfs_ok;
 out:
+	/* take the opportunity to clean up unused state */
 	if (fp && list_empty(&fp->fi_perfile))
 		release_file(fp);

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:56 UTC #16 lore raw

PATCH 1/1 nfsd4: fix putrootfh return

In reply to NeilBrown

Thanks to Al Viro for noticing that putrootfh could return either a linux or an nfs error.

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4proc.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

fs/nfsd/nfs4proc.c renamed from fs/nfsd/nfs4proc.c~current~ +1 -1

diff ./fs/nfsd/nfs4proc.c~current~ ./fs/nfsd/nfs4proc.c
@@ -267,7 +267,7 @@ nfsd4_putrootfh(struct svc_rqst *rqstp, 
 	status = exp_pseudoroot(rqstp->rq_client, current_fh,
 			      &rqstp->rq_chandle);
 	if (!status)
-		status = nfsd_setuser(rqstp, current_fh->fh_export);
+		status = nfserrno(nfsd_setuser(rqstp, current_fh->fh_export));
 	return status;
 }

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:56 UTC #17 lore raw

PATCH 1/1 nfsd4: move some nfsd4_process_open2 code to nfs4_new_open

In reply to NeilBrown

Move most of the code in the new_open case of nfsd4_process_open2 to nfs4_new_open.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +27 ~~-17~~

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1261,6 +1261,30 @@ out:
 }
 
 static int
+nfs4_new_open(struct svc_rqst *rqstp, struct nfs4_stateid **stpp,
+		struct svc_fh *cur_fh, int flags)
+{
+	struct nfs4_stateid *stp;
+	int status;
+
+	stp = kmalloc(sizeof(struct nfs4_stateid), GFP_KERNEL);
+	if (stp == NULL)
+		return nfserr_resource;
+
+	status = nfsd_open(rqstp, cur_fh, S_IFREG, flags, &stp->st_vfs_file);
+	if (status) {
+		if (status == nfserr_dropit)
+			status = nfserr_jukebox;
+		kfree(stp);
+		return status;
+	}
+	vfsopen++;
+	stp->st_vfs_set = 1;
+	*stpp = stp;
+	return 0;
+}
+
+static int
 nfs4_upgrade_open(struct svc_rqst *rqstp, struct svc_fh *cur_fh, struct nfs4_stateid *stp, struct nfsd4_open *open)
 {
 	struct file *filp = stp->st_vfs_file;

@@ -1345,26 +1369,15 @@ nfsd4_process_open2(struct svc_rqst *rqs
 		if (status)
 			goto out;
 	} else {
+		/* Stateid was not found, this is a new OPEN */
 		int flags = 0;
-
-		status = nfserr_resource;
-		if ((stp = kmalloc(sizeof(struct nfs4_stateid),
-						GFP_KERNEL)) == NULL)
-			goto out;
-
 		if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
 			flags = MAY_WRITE;
 		else
 			flags = MAY_READ;
-		if ((status = nfsd_open(rqstp, current_fh,  S_IFREG,
-			                      flags,
-			                      &stp->st_vfs_file)) != 0)
-			goto out_free;
-
-		vfsopen++;
-
+		if ((status = nfs4_new_open(rqstp, &stp, current_fh, flags)))
+			goto out;
 		init_stateid(stp, fp, sop, open);
-		stp->st_vfs_set = 1;
 	}
 	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n\n",
 	            stp->st_stateid.si_boot, stp->st_stateid.si_stateownerid,

@@ -1395,9 +1408,6 @@ out:
 		open->op_rflags |= NFS4_OPEN_RESULT_CONFIRM;
 
 	return status;
-out_free:
-	kfree(stp);
-	goto out;
 }
 
 static struct work_struct laundromat_work;

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

NeilBrown

author

2004-09-24 04:56 UTC #18 lore raw

PATCH 1/1 nfsd4: move code to truncate on open to separate function

In reply to NeilBrown

for OPEN with O_TRUNC, if the truncate fails, the open fails. for nfs4_open_upgrade, this means undo the get_write_access. for new OPENs, this means release the newly created stateid.

Signed-off-by Andy Adamson <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./fs/nfsd/nfs4state.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

fs/nfsd/nfs4state.c renamed from fs/nfsd/nfs4state.c~current~ +24 -8

diff ./fs/nfsd/nfs4state.c~current~ ./fs/nfsd/nfs4state.c
@@ -1301,6 +1301,18 @@ nfs4_upgrade_open(struct svc_rqst *rqstp
 		status = get_write_access(inode);
 		if (status)
 			return nfserrno(status);
+		if (open->op_truncate) {
+			struct iattr iattr = {
+				.ia_valid = ATTR_SIZE,
+				.ia_size = 0,
+			};
+			status = nfsd_setattr(rqstp, cur_fh, &iattr, 0,
+					(time_t)0);
+			if (status) {
+				put_write_access(inode);
+				return status;
+			}
+		}
 
 		/* remember the open */
 		filp->f_mode = (filp->f_mode | FMODE_WRITE) & ~FMODE_READ;

@@ -1331,7 +1343,6 @@ nfs4_set_claim_prev(struct nfsd4_open *o
 int
 nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open)
 {
-	struct iattr iattr;
 	struct nfs4_stateowner *sop = open->op_stateowner;
 	struct nfs4_file *fp = NULL;
 	struct inode *ino;

@@ -1382,18 +1393,23 @@ nfsd4_process_open2(struct svc_rqst *rqs
 		if ((status = nfs4_new_open(rqstp, &stp, current_fh, flags)))
 			goto out;
 		init_stateid(stp, fp, sop, open);
+		if (open->op_truncate) {
+			struct iattr iattr = {
+				.ia_valid = ATTR_SIZE,
+				.ia_size = 0,
+			};
+			status = nfsd_setattr(rqstp, current_fh, &iattr, 0,
+					(time_t)0);
+			if (status) {
+				release_stateid(stp, OPEN_STATE);
+				goto out;
+			}
+		}
 	}
 	dprintk("nfs4_process_open2: stateid=(%08x/%08x/%08x/%08x)\n",
 	            stp->st_stateid.si_boot, stp->st_stateid.si_stateownerid,
 	            stp->st_stateid.si_fileid, stp->st_stateid.si_generation);
 
-	if (open->op_truncate) {
-		iattr.ia_valid = ATTR_SIZE;
-		iattr.ia_size = 0;
-		status = nfsd_setattr(rqstp, current_fh, &iattr, 0, (time_t)0);
-		if (status)
-			goto out;
-	}
 	memcpy(&open->op_stateid, &stp->st_stateid, sizeof(stateid_t));
 
 	open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE;

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

author

2004-09-24 05:40 UTC #20 lore raw

Quoted 5 lines · On Thursday September 23, [email protected] wrote:

Quoted 4 lines · NeilBrown
Subject: [PATCH kNFSd 1 of 18] nfsd4 fix: nfsd oopsed when encountering a conflict with a local lock
Subject: [PATCH kNFSd 2 of 18] separate a little of logic from fh_verify into new function
Subject: [PATCH kNFSd 3 of 18] nfsd4: don't take i_sem around call to ->getxattr

....

Quoted 2 lines

Which if any of these do you think are 2.6.9 material?

That depends on your criteria, but I suspect none of them are. While some of them do fix real bugs, NFSv4 is very much a work-in-progress and people should expect bugs. I don't feel that any are particularly more urgent than any others.

If we really are in the "release candidate" phase, then leave them until we enter the "pre" phase .... except that I'm not sure if "pre" exists any more.

Thanks,
NeilBrown

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs