- We no longer have to bother with PDIRUNLOCK, lookup() handles it for us.

 - Network filesystems are written with a special idiom that checks the
   cache first, and may even unlock dvp before discovering that a network
   round-trip is required to resolve the name.  I believe dvp is prevented
   from being recycled even in the forced unmount case by the shared lock
   on the mount point.  If not, this code should grow checks for VI_DOOMED
   after it relocks dvp or it will access NULL v_data fields.

Sponsored by:	Isilon Systems, Inc.

3 files changed, 41 insertions, 146 deletions

diff --git a/sys/fs/smbfs/smbfs_vnops.c b/sys/fs/smbfs/smbfs_vnops.c
index 9b27a527bcf1..94f3a0c5cf60 100644
--- a/sys/fs/smbfs/smbfs_vnops.c
+++ b/sys/fs/smbfs/smbfs_vnops.c
@@ -1050,10 +1050,6 @@ smbfs_pathcheck(struct smbmount *smp, const char *name, int nmlen, int nameiop)
 	return 0;
 }
 
-#ifndef PDIRUNLOCK
-#define	PDIRUNLOCK	0
-#endif
-
 /*
  * Things go even weird without fixed inode numbers...
  */
@@ -1080,11 +1076,10 @@ smbfs_lookup(ap)
 	int flags = cnp->cn_flags;
 	int nameiop = cnp->cn_nameiop;
 	int nmlen = cnp->cn_namelen;
-	int lockparent, wantparent, error, islastcn, isdot;
+	int wantparent, error, islastcn, isdot;
 	int killit;
 	
 	SMBVDEBUG("\n");
-	cnp->cn_flags &= ~PDIRUNLOCK;
 	if (dvp->v_type != VDIR)
 		return ENOTDIR;
 	if ((flags & ISDOTDOT) && (dvp->v_vflag & VV_ROOT)) {
@@ -1108,7 +1103,6 @@ smbfs_lookup(ap)
 		return EROFS;
 	if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0)
 		return error;
-	lockparent = flags & LOCKPARENT;
 	wantparent = flags & (LOCKPARENT|WANTPARENT);
 	smp = VFSTOSMBFS(mp);
 	dnp = VTOSMB(dvp);
@@ -1128,27 +1122,19 @@ smbfs_lookup(ap)
 
 		vhold(*vpp);
 		vp = *vpp;
-		mp_fixme("Unlocked v_id access.");
 		if (dvp == vp) {	/* lookup on current */
 			vref(vp);
 			error = 0;
 			SMBVDEBUG("cached '.'\n");
 		} else if (flags & ISDOTDOT) {
 			VOP_UNLOCK(dvp, 0, td);	/* unlock parent */
-			cnp->cn_flags |= PDIRUNLOCK;
 			error = vget(vp, LK_EXCLUSIVE, td);
-			if (!error && lockparent && islastcn) {
-				error = vn_lock(dvp, LK_EXCLUSIVE, td);
-				if (error == 0)
-					cnp->cn_flags &= ~PDIRUNLOCK;
-			}
-		} else {
+			if (error)
+				if (vn_lock(dvp, LK_EXCLUSIVE, td))
+					panic("smbfs_lookup: Can't " 
+					    "relock directory.");
+		} else
 			error = vget(vp, LK_EXCLUSIVE, td);
-			if (!lockparent || error || !islastcn) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
-		}
 		if (!error) {
 			killit = 0;
 			error = VOP_GETATTR(vp, &vattr, cnp->cn_cred, td);
@@ -1174,20 +1160,25 @@ smbfs_lookup(ap)
 			     return (0);
 			}
 			cache_purge(vp);
-			if (killit)
+			/*
+			 * XXX This is not quite right, if '.' is
+			 * inconsistent, we really need to start the lookup
+			 * all over again.  Hopefully there is some other
+			 * guarantee that prevents this case from happening.
+			 */
+			if (killit && vp != dvp)
 				vgone(vp);
-			vput(vp);
-			if (lockparent && dvp != vp && islastcn)
-				VOP_UNLOCK(dvp, 0, td);
+			if (vp != dvp)
+				vput(vp);
+			else
+				vrele(vp);
+			if (flags & ISDOTDOT)
+				if (vn_lock(dvp, LK_EXCLUSIVE, td))
+					panic("smbfs_lookup: Can't " 
+					    "relock directory.");
 		}
 		vdrop(vp);
-		error = vn_lock(dvp, LK_EXCLUSIVE, td);
 		*vpp = NULLVP;
-		if (error) {
-			cnp->cn_flags |= PDIRUNLOCK;
-			return (error);
-		}
-		cnp->cn_flags &= ~PDIRUNLOCK;
 	}
 	/* 
 	 * entry is not in the cache or has been expired
@@ -1217,10 +1208,6 @@ smbfs_lookup(ap)
 			if (error)
 				return error;
 			cnp->cn_flags |= SAVENAME;
-			if (!lockparent) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
 			return (EJUSTRETURN);
 		}
 		return ENOENT;
@@ -1244,10 +1231,6 @@ smbfs_lookup(ap)
 			return error;
 		*vpp = vp;
 		cnp->cn_flags |= SAVENAME;
-		if (!lockparent) {
-			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
-		}
 		return 0;
 	}
 	if (nameiop == RENAME && islastcn && wantparent) {
@@ -1261,10 +1244,6 @@ smbfs_lookup(ap)
 			return error;
 		*vpp = vp;
 		cnp->cn_flags |= SAVENAME;
-		if (!lockparent) {
-			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
-		}
 		return 0;
 	}
 	if (flags & ISDOTDOT) {
@@ -1274,14 +1253,6 @@ smbfs_lookup(ap)
 			vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
 			return error;
 		}
-		if (lockparent && islastcn) {
-			error = vn_lock(dvp, LK_EXCLUSIVE, td);
-			if (error) {
-				cnp->cn_flags |= PDIRUNLOCK;
-				vput(vp);
-				return error;
-			}
-		}
 		*vpp = vp;
 	} else if (isdot) {
 		vref(dvp);
@@ -1292,10 +1263,6 @@ smbfs_lookup(ap)
 			return error;
 		*vpp = vp;
 		SMBVDEBUG("lookup: getnewvp!\n");
-		if (!lockparent || !islastcn) {
-			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
-		}
 	}
 	if ((cnp->cn_flags & MAKEENTRY)/* && !islastcn*/) {
 /*		VTOSMB(*vpp)->n_ctime = VTOSMB(*vpp)->n_vattr.va_ctime.tv_sec;*/
diff --git a/sys/nfs4client/nfs4_vnops.c b/sys/nfs4client/nfs4_vnops.c
index ca1f3032dd69..be6bd858d38c 100644
--- a/sys/nfs4client/nfs4_vnops.c
+++ b/sys/nfs4client/nfs4_vnops.c
@@ -937,7 +937,7 @@ nfs4_lookup(struct vop_lookup_args *ap)
 	long len;
 	nfsfh_t *fhp;
 	struct nfsnode *np;
-	int lockparent, wantparent, error = 0, fhsize;
+	int wantparent, error = 0, fhsize;
 	struct thread *td = cnp->cn_thread;
 	struct nfs4_compound cp;
 	struct nfs4_oparg_getattr ga, dga;
@@ -945,13 +945,11 @@ nfs4_lookup(struct vop_lookup_args *ap)
 	struct nfs4_oparg_getfh gfh;
 
 	*vpp = NULLVP;
-	cnp->cn_flags &= ~PDIRUNLOCK;
 	if ((flags & ISLASTCN) && (dvp->v_mount->mnt_flag & MNT_RDONLY) &&
 	    (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
 		return (EROFS);
 	if (dvp->v_type != VDIR)
 		return (ENOTDIR);
-	lockparent = flags & LOCKPARENT;
 	wantparent = flags & (LOCKPARENT|WANTPARENT);
 	nmp = VFSTONFS(dvp->v_mount);
 	np = VTONFS(dvp);
@@ -977,20 +975,11 @@ nfs4_lookup(struct vop_lookup_args *ap)
 			error = 0;
 		} else if (flags & ISDOTDOT) {
 			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
 			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (!error && lockparent && (flags & ISLASTCN)) {
-				error = vn_lock(dvp, LK_EXCLUSIVE, td);
-				if (error == 0)
-					cnp->cn_flags &= ~PDIRUNLOCK;
-			}
-		} else {
+			if (error)
+				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
+		} else
 			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (!lockparent || error || !(flags & ISLASTCN)) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
-		}
 		if (!error) {
 			if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
 			 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
@@ -1002,18 +991,15 @@ nfs4_lookup(struct vop_lookup_args *ap)
 			     return (0);
 			}
 			cache_purge(newvp);
-			vput(newvp);
-			if (lockparent && dvp != newvp && (flags & ISLASTCN))
-				VOP_UNLOCK(dvp, 0, td);
+			if (newvp != dvp)
+				vput(newvp);
+			else
+				vrele(newvp);
+			if (flags & ISDOTDOT)
+				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
 		}
 		vdrop(newvp);
-		error = vn_lock(dvp, LK_EXCLUSIVE, td);
 		*vpp = NULLVP;
-		if (error) {
-			cnp->cn_flags |= PDIRUNLOCK;
-			return (error);
-		}
-		cnp->cn_flags &= ~PDIRUNLOCK;
 	}
 
 	error = 0;
@@ -1079,10 +1065,6 @@ nfs4_lookup(struct vop_lookup_args *ap)
 
 		*vpp = newvp;
 		cnp->cn_flags |= SAVENAME;
-		if (!lockparent) {
-			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
-		}
 		return (0);
 	}
 
@@ -1097,16 +1079,6 @@ nfs4_lookup(struct vop_lookup_args *ap)
 		newvp = NFSTOV(np);
 
 		nfs4_vnop_loadattrcache(newvp, &ga.fa, NULL);
-
-		if (lockparent && (flags & ISLASTCN)) {
-			error = vn_lock(dvp, LK_EXCLUSIVE, td);
-			if (error) {
-				cnp->cn_flags |= PDIRUNLOCK;
-		    		vput(newvp);
-				return (error);
-			}
-		} else
-			cnp->cn_flags |= PDIRUNLOCK;
 	} else if (NFS_CMPFH(np, fhp, fhsize)) {
 		VREF(dvp);
 		newvp = dvp;
@@ -1114,11 +1086,6 @@ nfs4_lookup(struct vop_lookup_args *ap)
 		error = nfs_nget(dvp->v_mount, fhp, fhsize, &np);
 		if (error)
 			return (error);
-
-		if (!lockparent || !(flags & ISLASTCN)) {
-			cnp->cn_flags |= PDIRUNLOCK;
-			VOP_UNLOCK(dvp, 0, td);
-		}
 		newvp = NFSTOV(np);
 
 		/* Fill in np used by open. */
@@ -1152,10 +1119,6 @@ nfsmout:
 		}
 		if ((cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME) &&
 		    (flags & ISLASTCN) && error == ENOENT) {
-			if (!lockparent) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
 			if (dvp->v_mount->mnt_flag & MNT_RDONLY)
 				error = EROFS;
 			else
diff --git a/sys/nfsclient/nfs_vnops.c b/sys/nfsclient/nfs_vnops.c
index d3059a26f7bf..2dfb3456bdb8 100644
--- a/sys/nfsclient/nfs_vnops.c
+++ b/sys/nfsclient/nfs_vnops.c
@@ -776,18 +776,16 @@ nfs_lookup(struct vop_lookup_args *ap)
 	long len;
 	nfsfh_t *fhp;
 	struct nfsnode *np;
-	int lockparent, wantparent, error = 0, attrflag, fhsize;
+	int wantparent, error = 0, attrflag, fhsize;
 	int v3 = NFS_ISV3(dvp);
 	struct thread *td = cnp->cn_thread;
 
 	*vpp = NULLVP;
-	cnp->cn_flags &= ~PDIRUNLOCK;
 	if ((flags & ISLASTCN) && (dvp->v_mount->mnt_flag & MNT_RDONLY) &&
 	    (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
 		return (EROFS);
 	if (dvp->v_type != VDIR)
 		return (ENOTDIR);
-	lockparent = flags & LOCKPARENT;
 	wantparent = flags & (LOCKPARENT|WANTPARENT);
 	nmp = VFSTONFS(dvp->v_mount);
 	np = VTONFS(dvp);
@@ -810,20 +808,11 @@ nfs_lookup(struct vop_lookup_args *ap)
 			error = 0;
 		} else if (flags & ISDOTDOT) {
 			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
 			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (!error && lockparent && (flags & ISLASTCN)) {
-				error = vn_lock(dvp, LK_EXCLUSIVE, td);
-				if (error == 0)
-					cnp->cn_flags &= ~PDIRUNLOCK;
-			}
-		} else {
+			if (error)
+				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
+		} else
 			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (!lockparent || error || !(flags & ISLASTCN)) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
-		}
 		if (!error) {
 			if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
 			 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
@@ -835,18 +824,15 @@ nfs_lookup(struct vop_lookup_args *ap)
 			     return (0);
 			}
 			cache_purge(newvp);
-			vput(newvp);
-			if (lockparent && dvp != newvp && (flags & ISLASTCN))
-				VOP_UNLOCK(dvp, 0, td);
+			if (dvp != newvp)
+				vput(newvp);
+			else 
+				vrele(newvp);
+			if (flags & ISDOTDOT)
+				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
 		}
 		vdrop(newvp);
-		error = vn_lock(dvp, LK_EXCLUSIVE, td);
 		*vpp = NULLVP;
-		if (error) {
-			cnp->cn_flags |= PDIRUNLOCK;
-			return (error);
-		}
-		cnp->cn_flags &= ~PDIRUNLOCK;
 	}
 	error = 0;
 	newvp = NULLVP;
@@ -889,10 +875,6 @@ nfs_lookup(struct vop_lookup_args *ap)
 		*vpp = newvp;
 		m_freem(mrep);
 		cnp->cn_flags |= SAVENAME;
-		if (!lockparent) {
-			VOP_UNLOCK(dvp, 0, td);
-			cnp->cn_flags |= PDIRUNLOCK;
-		}
 		return (0);
 	}
 
@@ -904,15 +886,6 @@ nfs_lookup(struct vop_lookup_args *ap)
 			return (error);
 		}
 		newvp = NFSTOV(np);
-		if (lockparent && (flags & ISLASTCN)) {
-			error = vn_lock(dvp, LK_EXCLUSIVE, td);
-			if (error) {
-				cnp->cn_flags |= PDIRUNLOCK;
-		    		vput(newvp);
-				return (error);
-			}
-		} else
-			cnp->cn_flags |= PDIRUNLOCK;
 	} else if (NFS_CMPFH(np, fhp, fhsize)) {
 		VREF(dvp);
 		newvp = dvp;
@@ -922,10 +895,6 @@ nfs_lookup(struct vop_lookup_args *ap)
 			m_freem(mrep);
 			return (error);
 		}
-		if (!lockparent || !(flags & ISLASTCN)) {
-			cnp->cn_flags |= PDIRUNLOCK;
-			VOP_UNLOCK(dvp, 0, td);
-		}
 		newvp = NFSTOV(np);
 	}
 	if (v3) {
@@ -950,10 +919,6 @@ nfsmout:
 		}
 		if ((cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME) &&
 		    (flags & ISLASTCN) && error == ENOENT) {
-			if (!lockparent) {
-				VOP_UNLOCK(dvp, 0, td);
-				cnp->cn_flags |= PDIRUNLOCK;
-			}
 			if (dvp->v_mount->mnt_flag & MNT_RDONLY)
 				error = EROFS;
 			else