From 016344807a236cf2c4f3bae50a5385ef7aa7552b Mon Sep 17 00:00:00 2001
From: Brian Feldman <green@FreeBSD.org>
Date: Fri, 16 Jan 2004 16:31:01 +0000
Subject: [PATCH] Fix an upper-vnode leak created in revision 1.52.  When an
 upper-layer file has been removed, it should be purged from the cache, but it
 need not be removed from the directory stack causing corruption; instead, it
 will simply be removed once the last references and holds on it are dropped
 at the end of the unlink/rmdir system calls, and the normal !UN_CACHED
 VOP_INACTIVE() handler for unionfs finishes it off.

This is easily reproduced by repeated "echo >file; rm file" on a
unionfs mount.  Strangely, "echo -n >file; rm file" didn't make
it happen.
---
 sys/fs/unionfs/union_subr.c  | 27 +++++++++++----------------
 sys/fs/unionfs/union_vnops.c |  5 ++---
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 896a5b9b5b94..fab061a12f5f 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -1155,38 +1155,33 @@ union_vn_close(vp, fmode, cred, td)
 	return (VOP_CLOSE(vp, fmode, cred, td));
 }
 
-#if 0
-
 /*
  *	union_removed_upper:
  *
- *	called with union_node unlocked. XXX
+ *	An upper-only file/directory has been removed; un-cache it so
+ *	that unionfs vnode gets reclaimed and the last uppervp reference
+ *	disappears.
+ *
+ *	Called with union_node unlocked.
  */
 
 void
 union_removed_upper(un)
 	struct union_node *un;
 {
-	struct thread *td = curthread;	/* XXX */
-	struct vnode **vpp;
-
-	/*
-	 * Do not set the uppervp to NULLVP.  If lowervp is NULLVP,
-	 * union node will have neither uppervp nor lowervp.  We remove
-	 * the union node from cache, so that it will not be referrenced.
-	 */
-	union_newupper(un, NULLVP);
-	if (un->un_dircache != NULL)
-		union_dircache_free(un);
+	struct thread *td = curthread;
 
 	if (un->un_flags & UN_CACHED) {
+		int hash = UNION_HASH(un->un_uppervp, un->un_lowervp);
+
+		while (union_list_lock(hash))
+			continue;
 		un->un_flags &= ~UN_CACHED;
 		LIST_REMOVE(un, un_cache);
+		union_list_unlock(hash);
 	}
 }
 
-#endif
-
 /*
  * Determine whether a whiteout is needed
  * during a remove/rmdir operation.
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index be7b28c8eee4..603acd1f7386 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1225,11 +1225,8 @@ union_remove(ap)
 		if (union_dowhiteout(un, cnp->cn_cred, td))
 			cnp->cn_flags |= DOWHITEOUT;
 		error = VOP_REMOVE(upperdvp, uppervp, cnp);
-#if 0
-		/* XXX */
 		if (!error)
 			union_removed_upper(un);
-#endif
 		union_unlock_upper(uppervp, td);
 	} else {
 		error = union_mkwhiteout(
@@ -1542,6 +1539,8 @@ union_rmdir(ap)
 		if (union_dowhiteout(un, cnp->cn_cred, td))
 			cnp->cn_flags |= DOWHITEOUT;
 		error = VOP_RMDIR(upperdvp, uppervp, ap->a_cnp);
+		if (!error)
+			union_removed_upper(un);
 		union_unlock_upper(uppervp, td);
 	} else {
 		error = union_mkwhiteout(