Subject: Re: vop_symlink and unused vpp?
To: Bill Studenmund <wrstuden@zembu.com>
From: Assar Westerlund <assar@netbsd.org>
List: tech-kern
Date: 07/17/2001 11:14:06
--=-=-=

Bill Studenmund <wrstuden@zembu.com> writes:
> > a) document that VOP_SYMLINK always gets *a_vpp = NULL in and can
> > choose to set it or not, but that it should be vrefed if != NULL.  ufs
> > can always set it, thereby making lfs happy
> 
> Sounds the best to me.

These patches follow that line.  I'll commit them if there are no
objections.

/assar


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=netbsd-sys2.diff

Index: coda/coda_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/coda/coda_vnops.c,v
retrieving revision 1.25
diff -u -w -r1.25 coda_vnops.c
--- coda/coda_vnops.c	2001/07/03 06:46:52	1.25
+++ coda/coda_vnops.c	2001/07/17 09:08:43
@@ -1655,19 +1655,9 @@
     }
 
     /* 
-     * Okay, now we have to drop locks on dvp.  vpp is unlocked, but
-     * ref'd.  It doesn't matter what happens in either symlink or
-     * lookup.  Furthermore, there isn't any way for (dvp == *vpp), so
-     * we don't bother checking.  
+     * Okay, now we have to drop locks on dvp.
      */
 /*  vput(ap->a_dvp);		released earlier */
-    if (*ap->a_vpp) {
-    	VOP_UNLOCK(*ap->a_vpp, 0);	/* this line is new!! It is necessary because lookup() calls
-				   VOP_LOOKUP (coda_lookup) which returns vpp locked.  cfs_nb_lookup
-				   merged with coda_lookup() to become coda_lookup so UNLOCK is
-				   necessary */
-    	vrele(*ap->a_vpp);
-    }
 
     /* 
      * Free the name buffer 
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vfs_syscalls.c,v
retrieving revision 1.167
diff -u -w -r1.167 vfs_syscalls.c
--- kern/vfs_syscalls.c	2001/06/28 08:04:18	1.167
+++ kern/vfs_syscalls.c	2001/07/17 09:08:43
@@ -1514,6 +1514,8 @@
 	vattr.va_mode = ACCESSPERMS &~ p->p_cwdi->cwdi_cmask;
 	VOP_LEASE(nd.ni_dvp, p, p->p_ucred, LEASE_WRITE);
 	error = VOP_SYMLINK(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &vattr, path);
+	if (error == 0 && nd.ni_vp != NULL)
+		vput(nd.ni_vp);
 out:
 	PNBUF_PUT(path);
 	return (error);
Index: kern/vnode_if.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vnode_if.c,v
retrieving revision 1.36
diff -u -w -r1.36 vnode_if.c
--- kern/vnode_if.c	2001/05/26 21:34:04	1.36
+++ kern/vnode_if.c	2001/07/17 09:08:43
@@ -868,7 +868,7 @@
 const struct vnodeop_desc vop_symlink_desc = {
 	25,
 	"vop_symlink",
-	0 | VDESC_VP0_WILLPUT | VDESC_VPP_WILLRELE,
+	0 | VDESC_VP0_WILLPUT,
 	vop_symlink_vp_offsets,
 	VOPARG_OFFSETOF(struct vop_symlink_args, a_vpp),
 	VDESC_NO_OFFSET,
Index: kern/vnode_if.src
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vnode_if.src,v
retrieving revision 1.28
diff -u -w -r1.28 vnode_if.src
--- kern/vnode_if.src	2001/05/26 21:33:11	1.28
+++ kern/vnode_if.src	2001/07/17 09:08:44
@@ -338,17 +338,13 @@
 
 #
 #% symlink    dvp     L U U
-#% symlink    vpp     - U -
+#% symlink    vpp     - L -
 #
 #! symlink cnp	CREATE, LOCKPARENT
 #
-# XXX - note that the return vnode has already been VRELE'ed
-#     by the filesystem layer.  To use it you must use vget,
-#     possibly with a further namei.
-#
 vop_symlink {
 	IN WILLPUT struct vnode *dvp;
-	OUT WILLRELE struct vnode **vpp;
+	OUT struct vnode **vpp;
 	IN struct componentname *cnp;
 	IN struct vattr *vap;
 	IN char *target;
Index: miscfs/union/union_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/miscfs/union/union_vnops.c,v
retrieving revision 1.53
diff -u -w -r1.53 union_vnops.c
--- miscfs/union/union_vnops.c	2001/07/04 21:38:00	1.53
+++ miscfs/union/union_vnops.c	2001/07/17 09:08:44
@@ -1534,7 +1534,7 @@
 		un->un_flags |= UN_KLOCK;
 		vput(ap->a_dvp);
 		error = VOP_SYMLINK(dvp, &vp, cnp, ap->a_vap, ap->a_target);
-		*ap->a_vpp = NULLVP;
+		*ap->a_vpp = vp;
 		return (error);
 	}
 
Index: nfs/nfs_serv.c
===================================================================
RCS file: /cvsroot/syssrc/sys/nfs/nfs_serv.c,v
retrieving revision 1.59
diff -u -w -r1.59 nfs_serv.c
--- nfs/nfs_serv.c	2000/11/27 08:39:49	1.59
+++ nfs/nfs_serv.c	2001/07/17 09:08:44
@@ -2146,12 +2146,14 @@
 		vrele(nd.ni_startdir);
 	else {
 	    if (v3) {
+		if (nd.ni_vp == NULL) {
 		nd.ni_cnd.cn_nameiop = LOOKUP;
 		nd.ni_cnd.cn_flags &= ~(LOCKPARENT | SAVESTART | FOLLOW);
 		nd.ni_cnd.cn_flags |= (NOFOLLOW | LOCKLEAF);
 		nd.ni_cnd.cn_proc = procp;
 		nd.ni_cnd.cn_cred = cred;
 		error = lookup(&nd);
+		}
 		if (!error) {
 			memset((caddr_t)fhp, 0, sizeof(nfh));
 			fhp->fh_fsid = nd.ni_vp->v_mount->mnt_stat.f_fsid;
@@ -2161,8 +2163,11 @@
 					procp);
 			vput(nd.ni_vp);
 		}
-	    } else
+	    } else {
 		vrele(nd.ni_startdir);
+		if (nd.ni_vp != NULL)
+		    vput(nd.ni_vp);
+	    }
 	    PNBUF_PUT(nd.ni_cnd.cn_pnbuf);
 	}
 out:
Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/nfs/nfs_vnops.c,v
retrieving revision 1.134
diff -u -w -r1.134 nfs_vnops.c
--- nfs/nfs_vnops.c	2001/06/07 01:04:40	1.134
+++ nfs/nfs_vnops.c	2001/07/17 09:08:45
@@ -1869,6 +1869,7 @@
 	struct vnode *newvp = (struct vnode *)0;
 	const int v3 = NFS_ISV3(dvp);
 
+	*ap->a_vpp = NULL;
 	nfsstats.rpccnt[NFSPROC_SYMLINK]++;
 	slen = strlen(ap->a_target);
 	nfsm_reqhead(dvp, NFSPROC_SYMLINK, NFSX_FH(v3) + 2*NFSX_UNSIGNED +
Index: ufs/ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.33
diff -u -w -r1.33 ext2fs_vnops.c
--- ufs/ext2fs/ext2fs_vnops.c	2001/03/23 21:11:08	1.33
+++ ufs/ext2fs/ext2fs_vnops.c	2001/07/17 09:08:45
@@ -1206,6 +1206,7 @@
 		error = vn_rdwr(UIO_WRITE, vp, ap->a_target, len, (off_t)0,
 		    UIO_SYSSPACE, IO_NODELOCKED, ap->a_cnp->cn_cred,
 		    (size_t *)0, (struct proc *)0);
+	if (error)
 	vput(vp);
 	return (error);
 }
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.78
diff -u -w -r1.78 ufs_vnops.c
--- ufs/ufs/ufs_vnops.c	2001/05/28 02:50:53	1.78
+++ ufs/ufs/ufs_vnops.c	2001/07/17 09:08:45
@@ -1450,6 +1450,7 @@
 		error = vn_rdwr(UIO_WRITE, vp, ap->a_target, len, (off_t)0,
 		    UIO_SYSSPACE, IO_NODELOCKED, ap->a_cnp->cn_cred, NULL,
 		    (struct proc *)0);
+	if (error)
 	vput(vp);
 	return (error);
 }

--=-=-=--