- cache_lookup() now locks the new vnode for us to prevent some races.

   Remove redundant code.

Sponsored by:	Isilon Systems, Inc.

3 files changed, 73 insertions, 134 deletions

diff --git a/sys/fs/smbfs/smbfs_vnops.c b/sys/fs/smbfs/smbfs_vnops.c
index 94f3a0c..3bdcfdb 100644
--- a/sys/fs/smbfs/smbfs_vnops.c
+++ b/sys/fs/smbfs/smbfs_vnops.c
@@ -1120,64 +1120,44 @@ smbfs_lookup(ap)
 	if (error) {		/* name was found */
 		struct vattr vattr;
 
-		vhold(*vpp);
-		vp = *vpp;
-		if (dvp == vp) {	/* lookup on current */
-			vref(vp);
-			error = 0;
-			SMBVDEBUG("cached '.'\n");
-		} else if (flags & ISDOTDOT) {
-			VOP_UNLOCK(dvp, 0, td);	/* unlock parent */
-			error = vget(vp, LK_EXCLUSIVE, td);
-			if (error)
-				if (vn_lock(dvp, LK_EXCLUSIVE, td))
-					panic("smbfs_lookup: Can't " 
-					    "relock directory.");
-		} else
-			error = vget(vp, LK_EXCLUSIVE, td);
-		if (!error) {
-			killit = 0;
-			error = VOP_GETATTR(vp, &vattr, cnp->cn_cred, td);
-			/*
-			 * If the file type on the server is inconsistent
-			 * with what it was when we created the vnode,
-			 * kill the bogus vnode now and fall through to
-			 * the code below to create a new one with the
-			 * right type.
-			 */
-			if (error == 0 &&
-			   ((vp->v_type == VDIR &&
-			   (VTOSMB(vp)->n_dosattr & SMB_FA_DIR) == 0) ||
-			   (vp->v_type == VREG &&
-			   (VTOSMB(vp)->n_dosattr & SMB_FA_DIR) != 0)))
-			   killit = 1;
-			else if (error == 0
-		     /*    && vattr.va_ctime.tv_sec == VTOSMB(vp)->n_ctime*/) {
-			     if (nameiop != LOOKUP && islastcn)
-				     cnp->cn_flags |= SAVENAME;
-			     SMBVDEBUG("use cached vnode\n");
-			     vdrop(vp);
-			     return (0);
-			}
-			cache_purge(vp);
-			/*
-			 * 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);
-			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.");
+		killit = 0;
+		error = VOP_GETATTR(vp, &vattr, cnp->cn_cred, td);
+		/*
+		 * If the file type on the server is inconsistent
+		 * with what it was when we created the vnode,
+		 * kill the bogus vnode now and fall through to
+		 * the code below to create a new one with the
+		 * right type.
+		 */
+		if (error == 0 &&
+		   ((vp->v_type == VDIR &&
+		   (VTOSMB(vp)->n_dosattr & SMB_FA_DIR) == 0) ||
+		   (vp->v_type == VREG &&
+		   (VTOSMB(vp)->n_dosattr & SMB_FA_DIR) != 0)))
+		   killit = 1;
+		else if (error == 0
+	     /*    && vattr.va_ctime.tv_sec == VTOSMB(vp)->n_ctime*/) {
+		     if (nameiop != LOOKUP && islastcn)
+			     cnp->cn_flags |= SAVENAME;
+		     SMBVDEBUG("use cached vnode\n");
+		     return (0);
 		}
-		vdrop(vp);
+		cache_purge(vp);
+		/*
+		 * 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);
+		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.");
 		*vpp = NULLVP;
 	}
 	/* 
diff --git a/sys/nfs4client/nfs4_vnops.c b/sys/nfs4client/nfs4_vnops.c
index be6bd85..8877a95 100644
--- a/sys/nfs4client/nfs4_vnops.c
+++ b/sys/nfs4client/nfs4_vnops.c
@@ -956,50 +956,29 @@ nfs4_lookup(struct vop_lookup_args *ap)
 
 	isdot = cnp->cn_namelen == 1 && cnp->cn_nameptr[0] == '.';
 
+	if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0) {
+		*vpp = NULLVP;
+		return (error);
+	}
 	if ((error = cache_lookup(dvp, vpp, cnp)) && error != ENOENT) {
 		struct vattr vattr;
 
-		if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0) {
-			*vpp = NULLVP;
-			return (error);
-		}
-
-		vhold(*vpp);
 		newvp = *vpp;
-		/*
-		 * See the comment starting `Step through' in ufs/ufs_lookup.c
-		 * for an explanation of the locking protocol
-		 */
-		if (dvp == newvp) {
-			VREF(newvp);
-			error = 0;
-		} else if (flags & ISDOTDOT) {
-			VOP_UNLOCK(dvp, 0, td);
-			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (error)
-				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
-		} else
-			error = vget(newvp, LK_EXCLUSIVE, td);
-		if (!error) {
-			if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
-			 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
-			     nfsstats.lookupcache_hits++;
-			     if (cnp->cn_nameiop != LOOKUP &&
-				 (flags & ISLASTCN))
-				     cnp->cn_flags |= SAVENAME;
-			     vdrop(newvp);
-			     return (0);
-			}
-			cache_purge(newvp);
-			if (newvp != dvp)
-				vput(newvp);
-			else
-				vrele(newvp);
-			if (flags & ISDOTDOT)
-				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
+		if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
+		 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
+		     nfsstats.lookupcache_hits++;
+		     if (cnp->cn_nameiop != LOOKUP &&
+			 (flags & ISLASTCN))
+			     cnp->cn_flags |= SAVENAME;
+		     return (0);
 		}
-		vdrop(newvp);
-		*vpp = NULLVP;
+		cache_purge(newvp);
+		if (newvp != dvp)
+			vput(newvp);
+		else
+			vrele(newvp);
+		if (flags & ISDOTDOT)
+			vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
 	}
 
 	error = 0;
diff --git a/sys/nfsclient/nfs_vnops.c b/sys/nfsclient/nfs_vnops.c
index 2dfb345..b07a358 100644
--- a/sys/nfsclient/nfs_vnops.c
+++ b/sys/nfsclient/nfs_vnops.c
@@ -789,49 +789,29 @@ nfs_lookup(struct vop_lookup_args *ap)
 	wantparent = flags & (LOCKPARENT|WANTPARENT);
 	nmp = VFSTONFS(dvp->v_mount);
 	np = VTONFS(dvp);
+	if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0) {
+		*vpp = NULLVP;
+		return (error);
+	}
 	if ((error = cache_lookup(dvp, vpp, cnp)) && error != ENOENT) {
 		struct vattr vattr;
 
-		if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0) {
-			*vpp = NULLVP;
-			return (error);
-		}
-
-		vhold(*vpp);
 		newvp = *vpp;
-		/*
-		 * See the comment starting `Step through' in ufs/ufs_lookup.c
-		 * for an explanation of the locking protocol
-		 */
-		if (dvp == newvp) {
-			VREF(newvp);
-			error = 0;
-		} else if (flags & ISDOTDOT) {
-			VOP_UNLOCK(dvp, 0, td);
-			error = vget(newvp, LK_EXCLUSIVE, td);
-			if (error)
-				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
-		} else
-			error = vget(newvp, LK_EXCLUSIVE, td);
-		if (!error) {
-			if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
-			 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
-			     nfsstats.lookupcache_hits++;
-			     if (cnp->cn_nameiop != LOOKUP &&
-				 (flags & ISLASTCN))
-				     cnp->cn_flags |= SAVENAME;
-			     vdrop(newvp);
-			     return (0);
-			}
-			cache_purge(newvp);
-			if (dvp != newvp)
-				vput(newvp);
-			else 
-				vrele(newvp);
-			if (flags & ISDOTDOT)
-				vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
+		if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td)
+		 && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) {
+		     nfsstats.lookupcache_hits++;
+		     if (cnp->cn_nameiop != LOOKUP &&
+			 (flags & ISLASTCN))
+			     cnp->cn_flags |= SAVENAME;
+		     return (0);
 		}
-		vdrop(newvp);
+		cache_purge(newvp);
+		if (dvp != newvp)
+			vput(newvp);
+		else 
+			vrele(newvp);
+		if (flags & ISDOTDOT)
+			vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td);
 		*vpp = NULLVP;
 	}
 	error = 0;