Modify the experimental NFS server so that it uses LK_SHARED

for RPC operations when it can. Since VFS_FHTOVP() currently
always gets an exclusively locked vnode and is usually called
at the beginning of each RPC, the RPCs for a given vnode will
still be serialized. As such, passing a lock type argument to
VFS_FHTOVP() would be preferable to doing the vn_lock() with
LK_DOWNGRADE after the VFS_FHTOVP() call.

Reviewed by:	kib
MFC after:	2 weeks

6 files changed, 102 insertions, 78 deletions

diff --git a/sys/fs/nfs/nfs.h b/sys/fs/nfs/nfs.h
index 0325d3d..656c6c2 100644
--- a/sys/fs/nfs/nfs.h
+++ b/sys/fs/nfs/nfs.h
@@ -568,6 +568,7 @@ struct nfsv4_opflag {
 	int	needscfh;
 	int	savereply;
 	int	modifyfs;
+	int	lktype;
 };
 
 /*
diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c
index 6585fce..5f554d5 100644
--- a/sys/fs/nfs/nfs_commonsubs.c
+++ b/sys/fs/nfs/nfs_commonsubs.c
@@ -84,46 +84,46 @@ NFSSOCKMUTEX;
  * Define it here, since it is used by both the client and server.
  */
 struct nfsv4_opflag nfsv4_opflag[NFSV4OP_NOPS] = {
-	{ 0, 0, 0, 0 },		/* undef */
-	{ 0, 0, 0, 0 },		/* undef */
-	{ 0, 0, 0, 0 },		/* undef */
-	{ 0, 1, 0, 0 },		/* Access */
-	{ 0, 1, 0, 0 },		/* Close */
-	{ 0, 2, 0, 1 },		/* Commit */
-	{ 1, 2, 1, 1 },		/* Create */
-	{ 0, 0, 0, 0 },		/* Delegpurge */
-	{ 0, 1, 0, 0 },		/* Delegreturn */
-	{ 0, 1, 0, 0 },		/* Getattr */
-	{ 0, 1, 0, 0 },		/* GetFH */
-	{ 2, 1, 1, 1 },		/* Link */
-	{ 0, 1, 0, 0 },		/* Lock */
-	{ 0, 1, 0, 0 },		/* LockT */
-	{ 0, 1, 0, 0 },		/* LockU */
-	{ 1, 1, 0, 0 },		/* Lookup */
-	{ 1, 1, 0, 0 },		/* Lookupp */
-	{ 0, 1, 0, 0 },		/* NVerify */
-	{ 1, 1, 0, 1 },		/* Open */
-	{ 1, 1, 0, 0 },		/* OpenAttr */
-	{ 0, 1, 0, 0 },		/* OpenConfirm */
-	{ 0, 1, 0, 0 },		/* OpenDowngrade */
-	{ 1, 0, 0, 0 },		/* PutFH */
-	{ 1, 0, 0, 0 },		/* PutPubFH */
-	{ 1, 0, 0, 0 },		/* PutRootFH */
-	{ 0, 1, 0, 0 },		/* Read */
-	{ 0, 1, 0, 0 },		/* Readdir */
-	{ 0, 1, 0, 0 },		/* ReadLink */
-	{ 0, 2, 1, 1 },		/* Remove */
-	{ 2, 1, 1, 1 },		/* Rename */
-	{ 0, 0, 0, 0 },		/* Renew */
-	{ 0, 0, 0, 0 },		/* RestoreFH */
-	{ 0, 1, 0, 0 },		/* SaveFH */
-	{ 0, 1, 0, 0 },		/* SecInfo */
-	{ 0, 2, 1, 1 },		/* Setattr */
-	{ 0, 0, 0, 0 },		/* SetClientID */
-	{ 0, 0, 0, 0 },		/* SetClientIDConfirm */
-	{ 0, 1, 0, 0 },		/* Verify */
-	{ 0, 2, 1, 1 },		/* Write */
-	{ 0, 0, 0, 0 },		/* ReleaseLockOwner */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* undef */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* undef */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* undef */
+	{ 0, 1, 0, 0, LK_SHARED },		/* Access */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* Close */
+	{ 0, 2, 0, 1, LK_EXCLUSIVE },		/* Commit */
+	{ 1, 2, 1, 1, LK_EXCLUSIVE },		/* Create */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* Delegpurge */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* Delegreturn */
+	{ 0, 1, 0, 0, LK_SHARED },		/* Getattr */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* GetFH */
+	{ 2, 1, 1, 1, LK_EXCLUSIVE },		/* Link */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* Lock */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* LockT */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* LockU */
+	{ 1, 1, 0, 0, LK_EXCLUSIVE },		/* Lookup */
+	{ 1, 1, 0, 0, LK_EXCLUSIVE },		/* Lookupp */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* NVerify */
+	{ 1, 1, 0, 1, LK_EXCLUSIVE },		/* Open */
+	{ 1, 1, 0, 0, LK_EXCLUSIVE },		/* OpenAttr */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* OpenConfirm */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* OpenDowngrade */
+	{ 1, 0, 0, 0, LK_EXCLUSIVE },		/* PutFH */
+	{ 1, 0, 0, 0, LK_EXCLUSIVE },		/* PutPubFH */
+	{ 1, 0, 0, 0, LK_EXCLUSIVE },		/* PutRootFH */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* Read */
+	{ 0, 1, 0, 0, LK_SHARED },		/* Readdir */
+	{ 0, 1, 0, 0, LK_SHARED },		/* ReadLink */
+	{ 0, 2, 1, 1, LK_EXCLUSIVE },		/* Remove */
+	{ 2, 1, 1, 1, LK_EXCLUSIVE },		/* Rename */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* Renew */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* RestoreFH */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* SaveFH */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* SecInfo */
+	{ 0, 2, 1, 1, LK_EXCLUSIVE },		/* Setattr */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* SetClientID */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* SetClientIDConfirm */
+	{ 0, 1, 0, 0, LK_EXCLUSIVE },		/* Verify */
+	{ 0, 2, 1, 1, LK_EXCLUSIVE },		/* Write */
+	{ 0, 0, 0, 0, LK_EXCLUSIVE },		/* ReleaseLockOwner */
 };
 #endif	/* !APPLEKEXT */
 
@@ -1982,12 +1982,14 @@ nfsv4_fillattr(struct nfsrv_descript *nd, vnode_t vp, NFSACL_T *saclp,
 		    !NFSHASNFS4ACL(vnode_mount(vp)))) {
 			NFSCLRBIT_ATTRBIT(retbitp, NFSATTRBIT_ACL);
 		} else if (naclp != NULL) {
-			NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p);
-			error = VOP_ACCESS(vp, VREAD_ACL, cred, p);
-			if (error == 0)
-				error = VOP_GETACL(vp, ACL_TYPE_NFS4, naclp,
-				    cred, p);
-			NFSVOPUNLOCK(vp, 0, p);
+			if (vn_lock(vp, LK_SHARED) == 0) {
+				error = VOP_ACCESS(vp, VREAD_ACL, cred, p);
+				if (error == 0)
+					error = VOP_GETACL(vp, ACL_TYPE_NFS4,
+					    naclp, cred, p);
+				VOP_UNLOCK(vp, 0);
+			} else
+				error = NFSERR_PERM;
 			if (error != 0) {
 				if (reterr) {
 					nd->nd_repstat = NFSERR_ACCES;
diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h
index 244daf3..6113fb5 100644
--- a/sys/fs/nfs/nfs_var.h
+++ b/sys/fs/nfs/nfs_var.h
@@ -279,7 +279,7 @@ int nfscl_request(struct nfsrv_descript *, vnode_t,
 void nfsm_stateidtom(struct nfsrv_descript *, nfsv4stateid_t *, int);
 
 /* nfs_nfsdsubs.c */
-void nfsd_fhtovp(struct nfsrv_descript *, struct nfsrvfh *,
+void nfsd_fhtovp(struct nfsrv_descript *, struct nfsrvfh *, int,
     vnode_t *, struct nfsexstuff *,
     mount_t *, int, NFSPROC_T *);
 int nfsd_excred(struct nfsrv_descript *, struct nfsexstuff *, struct ucred *);
@@ -564,7 +564,7 @@ int nfsv4_sattr(struct nfsrv_descript *, struct nfsvattr *, nfsattrbit_t *,
     NFSACL_T *, NFSPROC_T *);
 int nfsvno_checkexp(mount_t, NFSSOCKADDR_T, struct nfsexstuff *,
     struct ucred **);
-int nfsvno_fhtovp(mount_t, fhandle_t *, NFSSOCKADDR_T,
+int nfsvno_fhtovp(mount_t, fhandle_t *, NFSSOCKADDR_T, int,
     vnode_t *, struct nfsexstuff *, struct ucred **);
 int nfsvno_pathconf(vnode_t, int, register_t *, struct ucred *,
     NFSPROC_T *);
diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c
index 4b18cbb..beedfef 100644
--- a/sys/fs/nfsserver/nfs_nfsdport.c
+++ b/sys/fs/nfsserver/nfs_nfsdport.c
@@ -180,7 +180,7 @@ nfsvno_accchk(struct vnode *vp, accmode_t accmode, struct ucred *cred,
 			return (ETXTBSY);
 	}
 	if (vpislocked == 0)
-		NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p);
+		vn_lock(vp, LK_SHARED | LK_RETRY);
 
 	/*
 	 * Should the override still be applied when ACLs are enabled?
@@ -218,7 +218,7 @@ nfsvno_accchk(struct vnode *vp, accmode_t accmode, struct ucred *cred,
 		}
 	}
 	if (vpislocked == 0)
-		NFSVOPUNLOCK(vp, 0, p);
+		VOP_UNLOCK(vp, 0);
 	return (error);
 }
 
@@ -1916,7 +1916,7 @@ again:
 				if (refp == NULL) {
 					if (usevget)
 						r = VFS_VGET(vp->v_mount,
-						    dp->d_fileno, LK_EXCLUSIVE,
+						    dp->d_fileno, LK_SHARED,
 						    &nvp);
 					else
 						r = EOPNOTSUPP;
@@ -1925,7 +1925,7 @@ again:
 							usevget = 0;
 							cn.cn_nameiop = LOOKUP;
 							cn.cn_lkflags =
-							    LK_EXCLUSIVE |
+							    LK_SHARED |
 							    LK_RETRY;
 							cn.cn_cred =
 							    nd->nd_cred;
@@ -1941,7 +1941,7 @@ again:
 						    dp->d_name[1] == '.')
 							cn.cn_flags |=
 							    ISDOTDOT;
-						if (vn_lock(vp, LK_EXCLUSIVE)
+						if (vn_lock(vp, LK_SHARED)
 						    != 0) {
 							nd->nd_repstat = EPERM;
 							break;
@@ -2454,7 +2454,8 @@ nfsvno_checkexp(struct mount *mp, struct sockaddr *nam, struct nfsexstuff *exp,
  */
 int
 nfsvno_fhtovp(struct mount *mp, fhandle_t *fhp, struct sockaddr *nam,
-    struct vnode **vpp, struct nfsexstuff *exp, struct ucred **credp)
+    int lktype, struct vnode **vpp, struct nfsexstuff *exp,
+    struct ucred **credp)
 {
 	int i, error, *secflavors;
 
@@ -2481,6 +2482,13 @@ nfsvno_fhtovp(struct mount *mp, fhandle_t *fhp, struct sockaddr *nam,
 				exp->nes_secflavors[i] = secflavors[i];
 		}
 	}
+	if (error == 0 && lktype == LK_SHARED)
+		/*
+		 * It would be much better to pass lktype to VFS_FHTOVP(),
+		 * but this will have to do until VFS_FHTOVP() has a lock
+		 * type argument like VFS_VGET().
+		 */
+		vn_lock(*vpp, LK_DOWNGRADE | LK_RETRY);
 	return (error);
 }
 
@@ -2518,7 +2526,7 @@ nfsvno_pathconf(struct vnode *vp, int flag, register_t *retf,
  *       call VFS_LOCK_GIANT()
  */
 void
-nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp,
+nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
     struct vnode **vpp, struct nfsexstuff *exp,
     struct mount **mpp, int startwrite, struct thread *p)
 {
@@ -2555,7 +2563,7 @@ nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp,
 	if (startwrite)
 		vn_start_write(NULL, mpp, V_WAIT);
 
-	nd->nd_repstat = nfsvno_fhtovp(mp, fhp, nd->nd_nam, vpp, exp,
+	nd->nd_repstat = nfsvno_fhtovp(mp, fhp, nd->nd_nam, lktype, vpp, exp,
 	    &credanon);
 
 	/*
diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c
index e4976f5..b4a4126 100644
--- a/sys/fs/nfsserver/nfs_nfsdserv.c
+++ b/sys/fs/nfsserver/nfs_nfsdserv.c
@@ -1392,7 +1392,7 @@ nfsrvd_rename(struct nfsrv_descript *nd, int isdgram,
 		nd->nd_cred->cr_uid = nd->nd_saveduid;
 		/* Won't lock vfs if already locked, mp == NULL */
 		tnes.nes_vfslocked = exp->nes_vfslocked;
-		nfsd_fhtovp(nd, &tfh, &tdp, &tnes, &mp, 0, p);
+		nfsd_fhtovp(nd, &tfh, LK_EXCLUSIVE, &tdp, &tnes, &mp, 0, p);
 		if (tdp) {
 			tdirfor_ret = nfsvno_getattr(tdp, &tdirfor, nd->nd_cred,
 			    p, 1);
@@ -1546,7 +1546,8 @@ nfsrvd_link(struct nfsrv_descript *nd, int isdgram,
 			}
 			/* Won't lock vfs if already locked, mp == NULL */
 			tnes.nes_vfslocked = exp->nes_vfslocked;
-			nfsd_fhtovp(nd, &dfh, &dp, &tnes, &mp, 0, p);
+			nfsd_fhtovp(nd, &dfh, LK_EXCLUSIVE, &dp, &tnes, &mp, 0,
+			    p);
 			if (dp)
 				NFSVOPUNLOCK(dp, 0, p);
 		}
@@ -3141,7 +3142,7 @@ nfsrvd_secinfo(struct nfsrv_descript *nd, int isdgram,
 	vput(vp);
 	savflag = nd->nd_flag;
 	if (!nd->nd_repstat) {
-		nfsd_fhtovp(nd, &fh, &vp, &retnes, &mp, 0, p);
+		nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, &mp, 0, p);
 		if (vp)
 			vput(vp);
 	}
diff --git a/sys/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsdsocket.c
index af1a3fa..8917e92 100644
--- a/sys/fs/nfsserver/nfs_nfsdsocket.c
+++ b/sys/fs/nfsserver/nfs_nfsdsocket.c
@@ -354,7 +354,7 @@ APPLESTATIC void
 nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram,
     NFSPROC_T *p)
 {
-	int error = 0;
+	int error = 0, lktype;
 	vnode_t vp;
 	mount_t mp = NULL;
 	struct nfsrvfh fh;
@@ -380,12 +380,20 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram,
 				nd->nd_repstat = NFSERR_GARBAGE;
 				return;
 			}
+			if (nd->nd_procnum == NFSPROC_READ ||
+			    nd->nd_procnum == NFSPROC_READDIR ||
+			    nd->nd_procnum == NFSPROC_READLINK ||
+			    nd->nd_procnum == NFSPROC_GETATTR ||
+			    nd->nd_procnum == NFSPROC_ACCESS)
+				lktype = LK_SHARED;
+			else
+				lktype = LK_EXCLUSIVE;
 			nes.nes_vfslocked = 0;
 			if (nd->nd_flag & ND_PUBLOOKUP)
-				nfsd_fhtovp(nd, &nfs_pubfh, &vp, &nes,
+				nfsd_fhtovp(nd, &nfs_pubfh, lktype, &vp, &nes,
 				    &mp, nfs_writerpc[nd->nd_procnum], p);
 			else
-				nfsd_fhtovp(nd, &fh, &vp, &nes,
+				nfsd_fhtovp(nd, &fh, lktype, &vp, &nes,
 				    &mp, nfs_writerpc[nd->nd_procnum], p);
 			if (nd->nd_repstat == NFSERR_PROGNOTV4)
 				return;
@@ -700,7 +708,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
 				goto nfsmout;
 			if (!nd->nd_repstat) {
 				nes.nes_vfslocked = vpnes.nes_vfslocked;
-				nfsd_fhtovp(nd, &fh, &nvp, &nes, &mp,
+				nfsd_fhtovp(nd, &fh, LK_SHARED, &nvp, &nes, &mp,
 				    0, p);
 			}
 			/* For now, allow this for non-export FHs */
@@ -715,7 +723,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
 		case NFSV4OP_PUTPUBFH:
 			if (nfs_pubfhset) {
 			    nes.nes_vfslocked = vpnes.nes_vfslocked;
-			    nfsd_fhtovp(nd, &nfs_pubfh, &nvp,
+			    nfsd_fhtovp(nd, &nfs_pubfh, LK_SHARED, &nvp,
 				&nes, &mp, 0, p);
 			} else {
 			    nd->nd_repstat = NFSERR_NOFILEHANDLE;
@@ -731,7 +739,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
 		case NFSV4OP_PUTROOTFH:
 			if (nfs_rootfhset) {
 				nes.nes_vfslocked = vpnes.nes_vfslocked;
-				nfsd_fhtovp(nd, &nfs_rootfh, &nvp,
+				nfsd_fhtovp(nd, &nfs_rootfh, LK_SHARED, &nvp,
 				    &nes, &mp, 0, p);
 				if (!nd->nd_repstat) {
 					if (vp)
@@ -907,24 +915,28 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
 			if (nfsv4_opflag[op].retfh != 0)
 				panic("nfsrvd_compound");
 			if (nfsv4_opflag[op].needscfh) {
-				if (vp) {
-					VREF(vp);
-					if (nfsv4_opflag[op].modifyfs)
-						NFS_STARTWRITE(NULL, &mp);
-					NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p);
-				} else {
+				if (vp != NULL) {
+					if (vn_lock(vp, nfsv4_opflag[op].lktype)
+					    != 0)
+						nd->nd_repstat = NFSERR_PERM;
+				} else
 					nd->nd_repstat = NFSERR_NOFILEHANDLE;
+				if (nd->nd_repstat != 0) {
 					if (op == NFSV4OP_SETATTR) {
-					    /*
-					     * Setattr reply requires a bitmap
-					     * even for errors like these.
-					     */
-					    NFSM_BUILD(tl, u_int32_t *,
-						NFSX_UNSIGNED);
-					    *tl = 0;
+						/*
+						 * Setattr reply requires a
+						 * bitmap even for errors like
+						 * these.
+						 */
+						NFSM_BUILD(tl, u_int32_t *,
+						    NFSX_UNSIGNED);
+						*tl = 0;
 					}
 					break;
 				}
+				VREF(vp);
+				if (nfsv4_opflag[op].modifyfs)
+					NFS_STARTWRITE(NULL, &mp);
 				error = (*(nfsrv4_ops0[op]))(nd, isdgram, vp,
 				    p, &vpnes);
 				if (nfsv4_opflag[op].modifyfs)