Subject: kern/1083: union FS leaks directory vnodes
To: None <gnats-admin@sun-lamp.cs.berkeley.edu>
From: John Kohl <jtk@kolvir.blrc.ma.us>
List: netbsd-bugs
Date: 05/29/1995 23:35:05
>Number:         1083
>Category:       kern
>Synopsis:       getdirentries() on a directory with no lower layer leaks vnodes
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon May 29 23:35:03 1995
>Originator:     John Kohl
>Organization:
NetBSD Kernel Hackers `R` Us
>Release:        -current as of about May 20th.
>Environment:
	
System: NetBSD kolvir 1.0A NetBSD 1.0A (KOLVIR) #404: Tue May 30 00:14:18 EDT 1995 jtk@kolvir:/u1/NetBSD-current/src/sys/arch/i386/compile/KOLVIR i386


>Description:

The union FS code will leak directory vnodes if you call getdirentries()
on a directory which exists only in the upper layer and not in the lower
layer.

>How-To-Repeat:
Create such a directory.  read it from a union FS.  unmount the union
FS, then try to unmount the upper FS, and you'll get EBUSY.  Look at the
vnode table with 'pstat -v' and see the stranded directory.  You get an
extra ref count with each call to getdirentries().

Also, inspect the hash chain maintenance code, and notice its
bogosities.

>Fix:

These patches are cumulative with my earlier union FS patches.  With
these patches and the UFS patch I just submitted, the union FS finally
appears to be ready for prime time (at least for my source trees!)

===================================================================
RCS file: RCS/union_vnops.c,v
retrieving revision 1.5
diff -u -r1.5 union_vnops.c
--- union_vnops.c	1995/04/21 00:54:35	1.5
+++ union_vnops.c	1995/05/30 04:12:54
@@ -1361,8 +1368,15 @@
 #endif
 
 	if (un->un_dircache != 0) {
-		for (vpp = un->un_dircache; *vpp != NULLVP; vpp++)
+#ifdef UNION_DIAGNOSTIC
+	    vprint("inactive dircache", ap->a_vp);
+#endif
+	    for (vpp = un->un_dircache; *vpp != NULLVP; vpp++) {
+#ifdef UNION_DIAGNOSTIC
+			vprint("inactive dircache rel", *vpp);
+#endif
 			vrele(*vpp);
+		}
 		free(un->un_dircache, M_TEMP);
 		un->un_dircache = 0;
 	}
@@ -1527,6 +1541,11 @@
 	    vprint("union_upper", UPPERVP(vp));
 	if (LOWERVP(vp))
 	    vprint("union_lower", LOWERVP(vp));
+	if (VTOUNION(vp)->un_dircache) {
+	    struct vnode **vpp;
+	    for (vpp = VTOUNION(vp)->un_dircache; *vpp != NULLVP; vpp++)
+		vprint("dircache:", *vpp);
+	}
 	return (0);
 }
 
===================================================================
RCS file: RCS/union_subr.c,v
retrieving revision 1.5
diff -u -r1.5 union_subr.c
--- union_subr.c	1995/05/21 23:25:41	1.5
+++ union_subr.c	1995/05/30 04:12:37
@@ -123,17 +123,19 @@
 	 * to avoid deadlocks.
 	 */
 	if (nhash < ohash) {
-		int t = ohash;
-		ohash = nhash;
-		nhash = t;
-	}
-
-	if (ohash != nhash)
+		while (union_list_lock(nhash))
+			continue;
 		while (union_list_lock(ohash))
 			continue;
-
-	while (union_list_lock(nhash))
-		continue;
+	} else if (nhash == ohash) {
+		while (union_list_lock(ohash))
+			continue;
+	} else {
+		while (union_list_lock(ohash))
+			continue;
+		while (union_list_lock(nhash))
+			continue;
+	}
 
 	if (ohash != nhash || !docache) {
 		if (un->un_flags & UN_CACHED) {
@@ -1010,6 +1012,9 @@
 	if (vp->v_op != union_vnodeop_p) {
 		if (vppp) {
 			VREF(vp);
+#ifdef DIAGNOSTIC
+			vprint("dircache ref", vp);
+#endif
 			*(*vppp)++ = vp;
 			if (--(*cntp) == 0)
 				panic("union: dircache table too small");
@@ -1057,17 +1062,32 @@
 		} while (*vpp != NULLVP);
 	}
 
-	if (*vpp == NULLVP)
+	if (*vpp == NULLVP) {
+		/* hold onto dircache:  we may hit this if there's no lowervp
+		   (e.g. a new directory in the top layer only). */
+		if (!VTOUNION(vp)->un_dircache) {
+			VTOUNION(vp)->un_dircache = dircache;
+#ifdef UNION_DIAGNOSTIC
+			vprint("union_dircache nulllower", vp);
+#endif
+		}
 		return (NULLVP);
+	}
 
 	VOP_LOCK(*vpp);
 	VREF(*vpp);
 	error = union_allocvp(&nvp, vp->v_mount, NULLVP, NULLVP, 0, *vpp, NULLVP, 0);
 	if (error)
 		return (NULLVP);
+#ifdef UNION_DIAGNOSTIC
+	vprint("union_drop_dircache", vp);
+#endif
 	VTOUNION(vp)->un_dircache = 0;
 	un = VTOUNION(nvp);
 	un->un_dircache = dircache;
 
+#ifdef UNION_DIAGNOSTIC
+	vprint("union_dircache", nvp);
+#endif
 	return (nvp);
 }
>Audit-Trail:
>Unformatted: