MFp4:

 - Respect cnflag and don't lock vnode always as LK_EXCLUSIVE [1]
 - Properly lock around tn_vnode to avoid NULL deference
 - Be more careful handling vnodes (*)

(*) This is a WIP
[1] by pjd via howardsu

Thanks kib@ for his valuable VFS related comments.

Tested with:	fsx, fstest, tmpfs regression test set
Found by:	pho's stress2 suite
Approved by:	re (tmpfs blanket)

4 files changed, 68 insertions, 35 deletions

diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h
index f681e3247ddb..c57eb25f73d9 100644
--- a/sys/fs/tmpfs/tmpfs.h
+++ b/sys/fs/tmpfs/tmpfs.h
@@ -353,8 +353,8 @@ int	tmpfs_alloc_dirent(struct tmpfs_mount *, struct tmpfs_node *,
 	    const char *, uint16_t, struct tmpfs_dirent **);
 void	tmpfs_free_dirent(struct tmpfs_mount *, struct tmpfs_dirent *,
 	    boolean_t);
-int	tmpfs_alloc_vp(struct mount *, struct tmpfs_node *, struct vnode **,
-	    struct thread *td);
+int	tmpfs_alloc_vp(struct mount *, struct tmpfs_node *, int,
+	    struct vnode **, struct thread *);
 void	tmpfs_free_vp(struct vnode *);
 int	tmpfs_alloc_file(struct vnode *, struct vnode **, struct vattr *,
 	    struct componentname *, char *);
diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c
index 4a2ae2da2e29..08127fd43d49 100644
--- a/sys/fs/tmpfs/tmpfs_subr.c
+++ b/sys/fs/tmpfs/tmpfs_subr.c
@@ -188,6 +188,12 @@ tmpfs_free_node(struct tmpfs_mount *tmp, struct tmpfs_node *node)
 {
 	size_t pages = 0;
 
+#ifdef INVARIANTS
+	TMPFS_NODE_LOCK(node);
+	MPASS(node->tn_vnode == NULL);
+	TMPFS_NODE_UNLOCK(node);
+#endif
+
 	TMPFS_LOCK(tmp);
 	LIST_REMOVE(node, tn_entries);
 	tmp->tm_nodes_inuse--;
@@ -302,15 +308,20 @@ tmpfs_free_dirent(struct tmpfs_mount *tmp, struct tmpfs_dirent *de,
  * Returns zero on success or an appropriate error code on failure.
  */
 int
-tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, struct vnode **vpp,
-    struct thread *td)
+tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, int lkflag,
+    struct vnode **vpp, struct thread *td)
 {
 	int error;
 	struct vnode *vp;
 
 loop:
+	TMPFS_NODE_LOCK(node);
 	if ((vp = node->tn_vnode) != NULL) {
-		error = vget(vp, LK_EXCLUSIVE | LK_RETRY, td);
+		VI_LOCK(vp);
+		TMPFS_NODE_UNLOCK(node);
+		vholdl(vp);
+		error = vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, td);
+		vdrop(vp);
 		if (error)
 			return error;
 
@@ -330,12 +341,11 @@ loop:
 	 * otherwise lock the vp list while we call getnewvnode
 	 * since that can block.
 	 */
-	TMPFS_NODE_LOCK(node);
 	if (node->tn_vpstate & TMPFS_VNODE_ALLOCATING) {
 		node->tn_vpstate |= TMPFS_VNODE_WANT;
 		error = msleep((caddr_t) &node->tn_vpstate,
 		    TMPFS_NODE_MTX(node), PDROP | PCATCH,
-		    "tmpfs_vplock", 0);
+		    "tmpfs_alloc_vp", 0);
 		if (error)
 			return error;
 
@@ -351,7 +361,7 @@ loop:
 		goto unlock;
 	MPASS(vp != NULL);
 
-	error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
+	error = vn_lock(vp, lkflag | LK_RETRY, td);
 	if (error != 0) {
 		vp->v_data = NULL;
 		vput(vp);
@@ -386,23 +396,15 @@ loop:
 
 	vnode_pager_setsize(vp, node->tn_size);
 	error = insmntque(vp, mp);
-	if (error) {
-		node->tn_vnode = NULL;
-		TMPFS_NODE_LOCK(node);
-		if (node->tn_vpstate & TMPFS_VNODE_WANT) {
-			node->tn_vpstate &= ~TMPFS_VNODE_WANT;
-			TMPFS_NODE_UNLOCK(node);
-			wakeup((caddr_t) &node->tn_vpstate);
-		} else
-			TMPFS_NODE_UNLOCK(node);
-		return error;
-	}
-	node->tn_vnode = vp;
+	if (error)
+		vp = NULL;
 
 unlock:
 	TMPFS_NODE_LOCK(node);
+
 	MPASS(node->tn_vpstate & TMPFS_VNODE_ALLOCATING);
 	node->tn_vpstate &= ~TMPFS_VNODE_ALLOCATING;
+	node->tn_vnode = vp;
 
 	if (node->tn_vpstate & TMPFS_VNODE_WANT) {
 		node->tn_vpstate &= ~TMPFS_VNODE_WANT;
@@ -415,7 +417,11 @@ out:
 	*vpp = vp;
 
 	MPASS(IFF(error == 0, *vpp != NULL && VOP_ISLOCKED(*vpp, td)));
+#ifdef INVARIANTS
+	TMPFS_NODE_LOCK(node);
 	MPASS(*vpp == node->tn_vnode);
+	TMPFS_NODE_UNLOCK(node);
+#endif
 
 	return error;
 }
@@ -433,8 +439,10 @@ tmpfs_free_vp(struct vnode *vp)
 
 	node = VP_TO_TMPFS_NODE(vp);
 
+	TMPFS_NODE_LOCK(node);
 	node->tn_vnode = NULL;
 	vp->v_data = NULL;
+	TMPFS_NODE_UNLOCK(node);
 }
 
 /* --------------------------------------------------------------------- */
@@ -499,7 +507,8 @@ tmpfs_alloc_file(struct vnode *dvp, struct vnode **vpp, struct vattr *vap,
 	}
 
 	/* Allocate a vnode for the new file. */
-	error = tmpfs_alloc_vp(dvp->v_mount, node, vpp, cnp->cn_thread);
+	error = tmpfs_alloc_vp(dvp->v_mount, node, LK_EXCLUSIVE, vpp,
+	    cnp->cn_thread);
 	if (error != 0) {
 		tmpfs_free_dirent(tmp, de, TRUE);
 		tmpfs_free_node(tmp, node);
diff --git a/sys/fs/tmpfs/tmpfs_vfsops.c b/sys/fs/tmpfs/tmpfs_vfsops.c
index e860e163d81c..a2ebf09a0c0f 100644
--- a/sys/fs/tmpfs/tmpfs_vfsops.c
+++ b/sys/fs/tmpfs/tmpfs_vfsops.c
@@ -390,7 +390,7 @@ static int
 tmpfs_root(struct mount *mp, int flags, struct vnode **vpp, struct thread *td)
 {
 	int error;
-	error = tmpfs_alloc_vp(mp, VFS_TO_TMPFS(mp)->tm_root, vpp, td);
+	error = tmpfs_alloc_vp(mp, VFS_TO_TMPFS(mp)->tm_root, flags, vpp, td);
 
 	if (!error)
 		(*vpp)->v_vflag |= VV_ROOT;
@@ -429,7 +429,10 @@ tmpfs_fhtovp(struct mount *mp, struct fid *fhp, struct vnode **vpp)
 	}
 	TMPFS_UNLOCK(tmp);
 
-	return found ? tmpfs_alloc_vp(mp, node, vpp, curthread) : EINVAL;
+	if (found)
+		return (tmpfs_alloc_vp(mp, node, LK_EXCLUSIVE, vpp, curthread));
+
+	return (EINVAL);
 }
 
 /* --------------------------------------------------------------------- */
diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c
index 514aeb75738e..d43e26741039 100644
--- a/sys/fs/tmpfs/tmpfs_vnops.c
+++ b/sys/fs/tmpfs/tmpfs_vnops.c
@@ -95,12 +95,20 @@ tmpfs_lookup(struct vop_cachedlookup_args *v)
 	    !(cnp->cn_flags & ISDOTDOT)));
 
 	if (cnp->cn_flags & ISDOTDOT) {
-		VOP_UNLOCK(dvp, 0, td);
+		int ltype = 0;
 
+		ltype = VOP_ISLOCKED(dvp, td);
+		vholdl(dvp);
+		VOP_UNLOCK(dvp, 0, td);
 		/* Allocate a new vnode on the matching entry. */
-		error = tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent, vpp, td);
+		error = tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent,
+		    cnp->cn_lkflags, vpp, td);
 
-		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
+		vn_lock(dvp, ltype | LK_RETRY, td);
+		if (ltype & LK_INTERLOCK)
+			vdropl(dvp);
+		else
+			vdrop(dvp);
 
 		dnode->tn_dir.tn_parent->tn_lookup_dirent = NULL;
 	} else if (cnp->cn_namelen == 1 && cnp->cn_nameptr[0] == '.') {
@@ -160,7 +168,8 @@ tmpfs_lookup(struct vop_cachedlookup_args *v)
 					goto out;
 
 				/* Allocate a new vnode on the matching entry. */
-				error = tmpfs_alloc_vp(dvp->v_mount, tnode, vpp, td);
+				error = tmpfs_alloc_vp(dvp->v_mount, tnode,
+						cnp->cn_lkflags, vpp, td);
 				if (error != 0)
 					goto out;
 
@@ -174,10 +183,10 @@ tmpfs_lookup(struct vop_cachedlookup_args *v)
 				}
 				tnode->tn_lookup_dirent = de;
 				cnp->cn_flags |= SAVENAME;
+			} else {
+				error = tmpfs_alloc_vp(dvp->v_mount, tnode,
+						cnp->cn_lkflags, vpp, td);
 			}
-			else
-				error = tmpfs_alloc_vp(dvp->v_mount, tnode, vpp, td);
-
 		}
 	}
 
@@ -478,7 +487,7 @@ lookupvpg:
 		vm_page_wakeup(m);
 		VM_OBJECT_UNLOCK(vobj);
 		return	(error);
-	} 
+	}
 	VM_OBJECT_UNLOCK(vobj);
 nocache:
 	VM_OBJECT_LOCK(tobj);
@@ -886,13 +895,13 @@ tmpfs_rename(struct vop_rename_args *v)
 	struct vnode *tdvp = v->a_tdvp;
 	struct vnode *tvp = v->a_tvp;
 	struct componentname *tcnp = v->a_tcnp;
-	struct tmpfs_node *tnode = 0; /* pacify gcc */
 
 	char *newname;
 	int error;
 	struct tmpfs_dirent *de;
 	struct tmpfs_node *fdnode;
 	struct tmpfs_node *fnode;
+	struct tmpfs_node *tnode;
 	struct tmpfs_node *tdnode;
 
 	MPASS(VOP_ISLOCKED(tdvp, tcnp->cn_thread));
@@ -902,6 +911,7 @@ tmpfs_rename(struct vop_rename_args *v)
 
 	fdnode = VP_TO_TMPFS_DIR(fdvp);
 	fnode = VP_TO_TMPFS_NODE(fvp);
+  	tnode = (tvp == NULL) ? NULL : VP_TO_TMPFS_NODE(tvp);
 	de = fnode->tn_lookup_dirent;
 
 	/* Disallow cross-device renames.
@@ -934,7 +944,7 @@ tmpfs_rename(struct vop_rename_args *v)
 	 * Kern_rename gurantees the destination to be a directory
 	 * if the source is one. */
 	if (tvp != NULL) {
-		tnode = VP_TO_TMPFS_NODE(tvp);
+		MPASS(tnode != NULL);
 
 		if ((tnode->tn_flags & (NOUNLINK | IMMUTABLE | APPEND)) ||
 		    (tdnode->tn_flags & (APPEND | IMMUTABLE))) {
@@ -942,9 +952,20 @@ tmpfs_rename(struct vop_rename_args *v)
 			goto out;
 		}
 
-		if ((de->td_node->tn_type == VDIR) && (tnode->tn_size > 0)) {
-			error = ENOTEMPTY;
+		if (fnode->tn_type == VDIR && tnode->tn_type == VDIR) {
+			if (tnode->tn_size > 0) {
+				error = ENOTEMPTY;
+				goto out;
+			}
+		} else if (fnode->tn_type == VDIR && tnode->tn_type != VDIR) {
+			error = ENOTDIR;
 			goto out;
+		} else if (fnode->tn_type != VDIR && tnode->tn_type == VDIR) {
+			error = EISDIR;
+			goto out;
+		} else {
+			MPASS(fnode->tn_type != VDIR &&
+				tnode->tn_type != VDIR);
 		}
 	}