In the words of the submitter:

---------
Make callers of namei() responsible for releasing references or locks
instead of having the underlying filesystems do it.  This eliminates
redundancy in all terminal filesystems and makes it possible for stacked
transport layers such as umapfs or nullfs to operate correctly.

Quality testing was done with testvn, and lat_fs from the lmbench suite.

Some NFS client testing courtesy of Patrik Kudo.

vop_mknod and vop_symlink still release the returned vpp.  vop_rename
still releases 4 vnode arguments before it returns.  These remaining cases
will be corrected in the next set of patches.
---------

Submitted by:	Michael Hancock <michaelh@cet.co.jp>

3 files changed, 91 insertions, 101 deletions

diff --git a/sys/fs/msdosfs/msdosfs_vnops.c b/sys/fs/msdosfs/msdosfs_vnops.c
index 4185114..555b271 100644
--- a/sys/fs/msdosfs/msdosfs_vnops.c
+++ b/sys/fs/msdosfs/msdosfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$Id: msdosfs_vnops.c,v 1.66 1998/03/20 02:33:42 kato Exp $ */
+/*	$Id: msdosfs_vnops.c,v 1.67 1998/03/26 20:52:58 phk Exp $ */
 /*	$NetBSD: msdosfs_vnops.c,v 1.68 1998/02/10 14:10:04 mrg Exp $	*/
 
 /*-
@@ -187,13 +187,11 @@ msdosfs_create(ap)
 		goto bad;
 	if ((cnp->cn_flags & SAVESTART) == 0)
 		zfree(namei_zone, cnp->cn_pnbuf);
-	vput(ap->a_dvp);
 	*ap->a_vpp = DETOV(dep);
 	return (0);
 
 bad:
 	zfree(namei_zone, cnp->cn_pnbuf);
-	vput(ap->a_dvp);
 	return (error);
 }
 
@@ -218,7 +216,6 @@ msdosfs_mknod(ap)
 
 	default:
 		zfree(namei_zone, ap->a_cnp->cn_pnbuf);
-		vput(ap->a_dvp);
 		return (EINVAL);
 	}
 	/* NOTREACHED */
@@ -872,9 +869,9 @@ msdosfs_remove(ap)
 		struct componentname *a_cnp;
 	} */ *ap;
 {
-	int error;
 	struct denode *dep = VTODE(ap->a_vp);
 	struct denode *ddep = VTODE(ap->a_dvp);
+	int error;
 
 	if (ap->a_vp->v_type == VDIR)
 		error = EPERM;
@@ -883,12 +880,6 @@ msdosfs_remove(ap)
 #ifdef MSDOSFS_DEBUG
 	printf("msdosfs_remove(), dep %p, v_usecount %d\n", dep, ap->a_vp->v_usecount);
 #endif
-	if (ddep == dep)
-		vrele(ap->a_vp);
-	else
-		vput(ap->a_vp);	/* causes msdosfs_inactive() to be called
-				 * via vrele() */
-	vput(ap->a_dvp);
 	return (error);
 }
 
@@ -906,8 +897,7 @@ msdosfs_link(ap)
 	} */ *ap;
 {
 	VOP_ABORTOP(ap->a_tdvp, ap->a_cnp);
-	vput(ap->a_tdvp);
-	return EOPNOTSUPP;
+	return (EOPNOTSUPP);
 }
 
 /*
@@ -1324,15 +1314,15 @@ msdosfs_mkdir(ap)
 	} */ *ap;
 {
 	struct componentname *cnp = ap->a_cnp;
-	struct denode ndirent;
 	struct denode *dep;
 	struct denode *pdep = VTODE(ap->a_dvp);
-	int error;
-	int bn;
-	u_long newcluster, pcl;
 	struct direntry *denp;
 	struct msdosfsmount *pmp = pdep->de_pmp;
 	struct buf *bp;
+	u_long newcluster, pcl;
+	int bn;
+	int error;
+	struct denode ndirent;
 	struct timespec ts;
 
 	/*
@@ -1419,7 +1409,6 @@ msdosfs_mkdir(ap)
 		goto bad;
 	if ((cnp->cn_flags & SAVESTART) == 0)
 		zfree(namei_zone, cnp->cn_pnbuf);
-	vput(ap->a_dvp);
 	*ap->a_vpp = DETOV(dep);
 	return (0);
 
@@ -1427,7 +1416,6 @@ bad:
 	clusterfree(pmp, newcluster, NULL);
 bad2:
 	zfree(namei_zone, cnp->cn_pnbuf);
-	vput(ap->a_dvp);
 	return (error);
 }
 
@@ -1443,6 +1431,7 @@ msdosfs_rmdir(ap)
 	register struct vnode *dvp = ap->a_dvp;
 	register struct componentname *cnp = ap->a_cnp;
 	register struct denode *ip, *dp;
+	struct proc *p = cnp->cn_proc;
 	int error;
 	
 	ip = VTODE(vp);
@@ -1474,20 +1463,18 @@ msdosfs_rmdir(ap)
 	/*
 	 * This is where we decrement the link count in the parent
 	 * directory.  Since dos filesystems don't do this we just purge
-	 * the name cache and let go of the parent directory denode.
+	 * the name cache.
 	 */
 	cache_purge(dvp);
-	vput(dvp);
-	dvp = NULL;
+	VOP_UNLOCK(dvp, 0, p);
 	/*
 	 * Truncate the directory that is being deleted.
 	 */
-	error = detrunc(ip, (u_long)0, IO_SYNC, cnp->cn_cred, cnp->cn_proc);
+	error = detrunc(ip, (u_long)0, IO_SYNC, cnp->cn_cred, p);
 	cache_purge(vp);
+
+	vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, p);
 out:
-	if (dvp)
-		vput(dvp);
-	vput(vp);
 	return (error);
 }
 
@@ -1506,7 +1493,6 @@ msdosfs_symlink(ap)
 {
 	zfree(namei_zone, ap->a_cnp->cn_pnbuf);
 	/* VOP_ABORTOP(ap->a_dvp, ap->a_cnp); ??? */
-	vput(ap->a_dvp);
 	return (EOPNOTSUPP);
 }
 
diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 879b0d0..7173734 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -35,7 +35,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)union_subr.c	8.20 (Berkeley) 5/20/95
- * $Id: union_subr.c,v 1.28 1998/02/10 03:32:05 kato Exp $
+ * $Id: union_subr.c,v 1.29 1998/02/26 03:23:54 kato Exp $
  */
 
 #include <sys/param.h>
@@ -830,6 +830,7 @@ union_mkshadow(um, dvp, cnp, vpp)
 	VOP_LEASE(dvp, p, cn.cn_cred, LEASE_WRITE);
 
 	error = VOP_MKDIR(dvp, vpp, &cn, &va);
+	vput(dvp);
 	return (error);
 }
 
@@ -955,7 +956,9 @@ union_vn_create(vpp, un, p)
 	vap->va_type = VREG;
 	vap->va_mode = cmode;
 	VOP_LEASE(un->un_dirvp, p, cred, LEASE_WRITE);
-	if (error = VOP_CREATE(un->un_dirvp, &vp, &cn, vap))
+	error = VOP_CREATE(un->un_dirvp, &vp, &cn, vap);
+	vput(un->un_dirvp);
+	if (error)
 		return (error);
 
 	error = VOP_OPEN(vp, fmode, cred, p);
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index e9a7a76..f1c6d78 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -35,7 +35,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)union_vnops.c	8.32 (Berkeley) 6/23/95
- * $Id: union_vnops.c,v 1.55 1998/02/26 03:23:56 kato Exp $
+ * $Id: union_vnops.c,v 1.56 1998/03/17 08:47:50 kato Exp $
  */
 
 #include <sys/param.h>
@@ -490,34 +490,36 @@ union_create(ap)
 		struct vattr *a_vap;
 	} */ *ap;
 {
-	struct union_node *un = VTOUNION(ap->a_dvp);
-	struct vnode *dvp = un->un_uppervp;
+	struct union_node *dun = VTOUNION(ap->a_dvp);
+	struct vnode *dvp = dun->un_uppervp;
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
 
 	if (dvp != NULLVP) {
-		int error;
 		struct vnode *vp;
 		struct mount *mp;
+		int error;
 
-		FIXUP(un, p);
+		FIXUP(dun, p);
 
-		VREF(dvp);
-		un->un_flags |= UN_KLOCK;
-		mp = ap->a_dvp->v_mount;
-		vput(ap->a_dvp);
+		dun->un_flags |= UN_KLOCK;
+		VOP_UNLOCK(ap->a_dvp, 0, p);
 		error = VOP_CREATE(dvp, &vp, cnp, ap->a_vap);
-		if (error)
+		if (error) {
+			dun->un_flags |= UN_ULOCK;
 			return (error);
+		}
 
+		mp = ap->a_dvp->v_mount;
+		VOP_UNLOCK(dvp, 0, p);
 		error = union_allocvp(ap->a_vpp, mp, NULLVP, NULLVP, cnp, vp,
 				NULLVP, 1);
 		if (error)
 			vput(vp);
+		vn_lock(ap->a_dvp, LK_EXCLUSIVE| LK_RETRY, p);
 		return (error);
 	}
 
-	vput(ap->a_dvp);
 	return (EROFS);
 }
 
@@ -549,36 +551,40 @@ union_mknod(ap)
 		struct vattr *a_vap;
 	} */ *ap;
 {
-	struct union_node *un = VTOUNION(ap->a_dvp);
-	struct vnode *dvp = un->un_uppervp;
+	struct union_node *dun = VTOUNION(ap->a_dvp);
+	struct vnode *dvp = dun->un_uppervp;
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
 
 	if (dvp != NULLVP) {
-		int error;
 		struct vnode *vp;
 		struct mount *mp;
+		int error;
 
-		FIXUP(un, p);
+		FIXUP(dun, p);
 
-		VREF(dvp);
-		un->un_flags |= UN_KLOCK;
-		mp = ap->a_dvp->v_mount;
-		vput(ap->a_dvp);
+		dun->un_flags |= UN_KLOCK;
+		VOP_UNLOCK(ap->a_dvp, 0, p);
 		error = VOP_MKNOD(dvp, &vp, cnp, ap->a_vap);
-		if (error)
+		if (error) {
+			dun->un_flags |= UN_ULOCK;
 			return (error);
+		}
 
 		if (vp != NULLVP) {
+			mp = ap->a_dvp->v_mount;
+			VOP_UNLOCK(dvp, 0, p);
 			error = union_allocvp(ap->a_vpp, mp, NULLVP, NULLVP,
 					cnp, vp, NULLVP, 1);
 			if (error)
 				vput(vp);
+			vn_lock(ap->a_dvp, LK_EXCLUSIVE| LK_RETRY, p);
+		} else {
+			dun->un_flags |= UN_ULOCK;
 		}
 		return (error);
 	}
 
-	vput(ap->a_dvp);
 	return (EROFS);
 }
 
@@ -1055,11 +1061,11 @@ union_remove(ap)
 		struct componentname *a_cnp;
 	} */ *ap;
 {
-	int error;
 	struct union_node *dun = VTOUNION(ap->a_dvp);
 	struct union_node *un = VTOUNION(ap->a_vp);
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
+	int error;
 
 	if (dun->un_uppervp == NULLVP)
 		panic("union remove: null upper vnode");
@@ -1069,15 +1075,13 @@ union_remove(ap)
 		struct vnode *vp = un->un_uppervp;
 
 		FIXUP(dun, p);
-		VREF(dvp);
 		dun->un_flags |= UN_KLOCK;
-		vput(ap->a_dvp);
+		VOP_UNLOCK(ap->a_dvp, 0, p);
 		FIXUP(un, p);
-		VREF(vp);
 		un->un_flags |= UN_KLOCK;
-		vput(ap->a_vp);
+		VOP_UNLOCK(ap->a_vp, 0, p);
 
-		if (union_dowhiteout(un, cnp->cn_cred, cnp->cn_proc))
+		if (union_dowhiteout(un, cnp->cn_cred, p))
 			cnp->cn_flags |= DOWHITEOUT;
 		error = VOP_REMOVE(dvp, vp, cnp);
 #if 0
@@ -1085,13 +1089,13 @@ union_remove(ap)
 		if (!error)
 			union_removed_upper(un);
 #endif
+		dun->un_flags |= UN_ULOCK;
+		un->un_flags |= UN_ULOCK;
 	} else {
 		FIXUP(dun, p);
 		error = union_mkwhiteout(
 			MOUNTTOUNIONMOUNT(UNIONTOV(dun)->v_mount),
 			dun->un_uppervp, ap->a_cnp, un->un_path);
-		vput(ap->a_dvp);
-		vput(ap->a_vp);
 	}
 
 	return (error);
@@ -1105,14 +1109,13 @@ union_link(ap)
 		struct componentname *a_cnp;
 	} */ *ap;
 {
-	int error = 0;
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
-	struct union_node *un;
+	struct union_node *dun = VTOUNION(ap->a_tdvp);
 	struct vnode *vp;
 	struct vnode *tdvp;
+	int error = 0;
 
-	un = VTOUNION(ap->a_tdvp);
 
 	if (ap->a_tdvp->v_op != ap->a_vp->v_op) {
 		vp = ap->a_vp;
@@ -1120,36 +1123,37 @@ union_link(ap)
 		struct union_node *tun = VTOUNION(ap->a_vp);
 		if (tun->un_uppervp == NULLVP) {
 			vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY, p);
-			if (un->un_uppervp == tun->un_dirvp) {
-				un->un_flags &= ~UN_ULOCK;
-				VOP_UNLOCK(un->un_uppervp, 0, p);
+			if (dun->un_uppervp == tun->un_dirvp) {
+				dun->un_flags &= ~UN_ULOCK;
+				VOP_UNLOCK(dun->un_uppervp, 0, p);
 			}
 			error = union_copyup(tun, 1, cnp->cn_cred, p);
-			if (un->un_uppervp == tun->un_dirvp) {
-				vn_lock(un->un_uppervp,
+			if (dun->un_uppervp == tun->un_dirvp) {
+				vn_lock(dun->un_uppervp,
 						LK_EXCLUSIVE | LK_RETRY, p);
-				un->un_flags |= UN_ULOCK;
+				dun->un_flags |= UN_ULOCK;
 			}
 			VOP_UNLOCK(ap->a_vp, 0, p);
 		}
 		vp = tun->un_uppervp;
 	}
 
-	tdvp = un->un_uppervp;
+	tdvp = dun->un_uppervp;
 	if (tdvp == NULLVP)
 		error = EROFS;
 
-	if (error) {
-		vput(ap->a_tdvp);
+	if (error)
 		return (error);
-	}
 
-	FIXUP(un, p);
-	VREF(tdvp);
-	un->un_flags |= UN_KLOCK;
-	vput(ap->a_tdvp);
+	FIXUP(dun, p);
+	dun->un_flags |= UN_KLOCK;
+	VOP_UNLOCK(ap->a_tdvp, 0, p);
+
+	error = VOP_LINK(tdvp, vp, cnp);
 
-	return (VOP_LINK(tdvp, vp, cnp));
+	dun->un_flags |= UN_ULOCK;
+
+	return (error);
 }
 
 static int
@@ -1255,34 +1259,34 @@ union_mkdir(ap)
 		struct vattr *a_vap;
 	} */ *ap;
 {
-	struct union_node *un = VTOUNION(ap->a_dvp);
-	struct vnode *dvp = un->un_uppervp;
+	struct union_node *dun = VTOUNION(ap->a_dvp);
+	struct vnode *dvp = dun->un_uppervp;
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
 
 	if (dvp != NULLVP) {
-		int error;
 		struct vnode *vp;
+		int error;
 
-		FIXUP(un, p);
-		VREF(dvp);
-		un->un_flags |= UN_KLOCK;
+		FIXUP(dun, p);
+		dun->un_flags |= UN_KLOCK;
 		VOP_UNLOCK(ap->a_dvp, 0, p);
 		error = VOP_MKDIR(dvp, &vp, cnp, ap->a_vap);
 		if (error) {
-			vrele(ap->a_dvp);
+			dun->un_flags |= UN_ULOCK;
 			return (error);
 		}
 
+		VOP_UNLOCK(dvp, 0, p);
 		error = union_allocvp(ap->a_vpp, ap->a_dvp->v_mount, ap->a_dvp,
 				NULLVP, cnp, vp, NULLVP, 1);
-		vrele(ap->a_dvp);
 		if (error)
 			vput(vp);
+		vn_lock(ap->a_dvp, LK_EXCLUSIVE| LK_RETRY, p);
+
 		return (error);
 	}
 
-	vput(ap->a_dvp);
 	return (EROFS);
 }
 
@@ -1294,11 +1298,11 @@ union_rmdir(ap)
 		struct componentname *a_cnp;
 	} */ *ap;
 {
-	int error;
 	struct union_node *dun = VTOUNION(ap->a_dvp);
 	struct union_node *un = VTOUNION(ap->a_vp);
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
+	int error;
 
 	if (dun->un_uppervp == NULLVP)
 		panic("union rmdir: null upper vnode");
@@ -1308,15 +1312,13 @@ union_rmdir(ap)
 		struct vnode *vp = un->un_uppervp;
 
 		FIXUP(dun, p);
-		VREF(dvp);
 		dun->un_flags |= UN_KLOCK;
-		vput(ap->a_dvp);
+		VOP_UNLOCK(ap->a_dvp, 0, p);
 		FIXUP(un, p);
-		VREF(vp);
 		un->un_flags |= UN_KLOCK;
-		vput(ap->a_vp);
+		VOP_UNLOCK(ap->a_vp, 0, p);
 
-		if (union_dowhiteout(un, cnp->cn_cred, cnp->cn_proc))
+		if (union_dowhiteout(un, cnp->cn_cred, p))
 			cnp->cn_flags |= DOWHITEOUT;
 		error = VOP_RMDIR(dvp, vp, ap->a_cnp);
 #if 0
@@ -1324,13 +1326,13 @@ union_rmdir(ap)
 		if (!error)
 			union_removed_upper(un);
 #endif
+		dun->un_flags |= UN_ULOCK;
+		un->un_flags |= UN_ULOCK;
 	} else {
 		FIXUP(dun, p);
 		error = union_mkwhiteout(
 			MOUNTTOUNIONMOUNT(UNIONTOV(dun)->v_mount),
 			dun->un_uppervp, ap->a_cnp, un->un_path);
-		vput(ap->a_dvp);
-		vput(ap->a_vp);
 	}
 
 	return (error);
@@ -1346,25 +1348,24 @@ union_symlink(ap)
 		char *a_target;
 	} */ *ap;
 {
-	struct union_node *un = VTOUNION(ap->a_dvp);
-	struct vnode *dvp = un->un_uppervp;
+	struct union_node *dun = VTOUNION(ap->a_dvp);
+	struct vnode *dvp = dun->un_uppervp;
 	struct componentname *cnp = ap->a_cnp;
 	struct proc *p = cnp->cn_proc;
 
 	if (dvp != NULLVP) {
-		int error;
 		struct vnode *vp;
+		int error;
 
-		FIXUP(un, p);
-		VREF(dvp);
-		un->un_flags |= UN_KLOCK;
-		vput(ap->a_dvp);
+		FIXUP(dun, p);
+		dun->un_flags |= UN_KLOCK;
+		VOP_UNLOCK(ap->a_dvp, 0, p);
 		error = VOP_SYMLINK(dvp, &vp, cnp, ap->a_vap, ap->a_target);
+		dun->un_flags |= UN_ULOCK;
 		*ap->a_vpp = NULLVP;
 		return (error);
 	}
 
-	vput(ap->a_dvp);
 	return (EROFS);
 }