--y0ulUmNC+osPPQO6
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Thu Jul 05 2007 at 01:37:23 +0300, Antti Kantee wrote:
> On Wed Jul 04 2007 at 20:43:30 +0200, Juan RP wrote:
> > On Wed, 4 Jul 2007 19:34:07 +0200
> > "Zafer Aydogan" <zafer@aydogan.de> wrote:
> > 
> > > I did. Now I can't boot. System panics right before going into multiuser.
> > > Screenshots are available at http://aydogan.org/reboot/
> > 
> > Build it with LOCKDEBUG, take pictures of the panic message/backtrace and
> > send a PR.
> > 
> > There seems to be a locking error in layerfs or something.
> 
> The problem is that nullfs passes the VOP_REVOKE operation to the
> lower vnode.  However, the upper nullfs vnode remains entirely intact.
> Then when vrele() is called from sys_revoke(), the upper layer vnode tries
> to use the lock of the now-revoked lower layer vnode and goes kabloom.
> I think the correct fix is to supply a revoke operation for nullfs &
> layerfs, but I'm not intimate enough with them to be entirely sure that's
> the correct fix.  At least the problem goes away using the attached patch.
> 
> Bill?

Ok, so Juan pointed me to one more problem which happens only when
revoking device nodes (which is what the live cd seems to do).  This is
kind of a "fixing a hole where the rain gets in" -type of solution.
Although my mind is certainly wandering in the layerfs call paths ...

-- 
Antti Kantee <pooka@iki.fi>                     Of course he runs NetBSD
http://www.iki.fi/pooka/                          http://www.NetBSD.org/
    "la qualité la plus indispensable du cuisinier est l'exactitude"

--y0ulUmNC+osPPQO6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="layer_revoke2.patch"

Index: genfs/layer_extern.h
===================================================================
RCS file: /cvsroot/src/sys/miscfs/genfs/layer_extern.h,v
retrieving revision 1.22
diff -u -r1.22 layer_extern.h
--- genfs/layer_extern.h	13 Jul 2006 12:00:25 -0000	1.22
+++ genfs/layer_extern.h	5 Jul 2007 09:02:37 -0000
@@ -99,6 +99,7 @@
 int	layer_getattr(void *);
 int	layer_inactive(void *);
 int	layer_reclaim(void *);
+int	layer_revoke(void *);
 int	layer_print(void *);
 int	layer_bwrite(void *);
 int	layer_bmap(void *);
Index: genfs/layer_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/genfs/layer_vnops.c,v
retrieving revision 1.31
diff -u -r1.31 layer_vnops.c
--- genfs/layer_vnops.c	16 Apr 2007 08:10:58 -0000	1.31
+++ genfs/layer_vnops.c	5 Jul 2007 09:02:38 -0000
@@ -741,6 +741,7 @@
 		struct lwp *a_l;
 	} */ *ap = v;
 	struct vnode *vp = ap->a_vp;
+	int vxlock;
 
 	/*
 	 * Do nothing (and _don't_ bypass).
@@ -756,12 +757,19 @@
 	 */
 	VOP_UNLOCK(vp, 0);
 
+	simple_lock(&vp->v_interlock);
+	vxlock = vp->v_flag & VXLOCK;
+	simple_unlock(&vp->v_interlock);
+
 	/*
 	 * ..., but don't cache the device node. Also, if we did a
-	 * remove, don't cache the node.
+	 * remove, don't cache the node.  Finally, if the node is
+	 * already being nuked, don't do this, as we would re-enter
+	 * the same path and deadlock.
 	 */
-	if (vp->v_type == VBLK || vp->v_type == VCHR
+	if ((vp->v_type == VBLK || vp->v_type == VCHR
 	    || (VTOLAYER(vp)->layer_flags & LAYERFS_REMOVED))
+	  && (vxlock == 0))
 		vgone(vp);
 	return (0);
 }
@@ -883,6 +891,24 @@
 	return (0);
 }
 
+int
+layer_revoke(v)
+	void *v;
+{
+	struct vop_revoke_args /* {
+		struct vnode *a_vp;
+		int a_flags;
+	} */ *ap = v;
+	struct vnode *vp = ap->a_vp;
+	struct layer_node *xp = VTOLAYER(vp);
+	struct vnode *lowervp = xp->layer_lowervp;
+
+	genfs_revoke(v);
+	VOP_REVOKE(lowervp, ap->a_flags);
+
+	return 0;
+}
+
 /*
  * We just feed the returned vnode up to the caller - there's no need
  * to build a layer node on top of the node on which we're going to do
Index: nullfs/null_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/nullfs/null_vnops.c,v
retrieving revision 1.34
diff -u -r1.34 null_vnops.c
--- nullfs/null_vnops.c	11 Dec 2005 12:24:51 -0000	1.34
+++ nullfs/null_vnops.c	5 Jul 2007 09:02:38 -0000
@@ -235,6 +235,7 @@
 	{ &vop_fsync_desc,    layer_fsync },
 	{ &vop_inactive_desc, layer_inactive },
 	{ &vop_reclaim_desc,  layer_reclaim },
+	{ &vop_revoke_desc,   layer_revoke },
 	{ &vop_print_desc,    layer_print },
 	{ &vop_remove_desc,   layer_remove },
 	{ &vop_rename_desc,   layer_rename },
Index: overlay/overlay_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/overlay/overlay_vnops.c,v
retrieving revision 1.16
diff -u -r1.16 overlay_vnops.c
--- overlay/overlay_vnops.c	11 Dec 2005 12:24:51 -0000	1.16
+++ overlay/overlay_vnops.c	5 Jul 2007 09:02:39 -0000
@@ -158,6 +158,7 @@
 	{ &vop_fsync_desc,    layer_fsync },
 	{ &vop_inactive_desc, layer_inactive },
 	{ &vop_reclaim_desc,  layer_reclaim },
+	{ &vop_reclaim_desc,  layer_revoke },
 	{ &vop_print_desc,    layer_print },
 	{ &vop_remove_desc,   layer_remove },
 	{ &vop_rename_desc,   layer_rename },
Index: umapfs/umap_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/umapfs/umap_vnops.c,v
retrieving revision 1.43
diff -u -r1.43 umap_vnops.c
--- umapfs/umap_vnops.c	9 Dec 2006 16:11:52 -0000	1.43
+++ umapfs/umap_vnops.c	5 Jul 2007 09:02:39 -0000
@@ -83,6 +83,7 @@
 	{ &vop_fsync_desc,	layer_fsync },
 	{ &vop_inactive_desc,	layer_inactive },
 	{ &vop_reclaim_desc,	layer_reclaim },
+	{ &vop_reclaim_desc,	layer_revoke },
 	{ &vop_open_desc,	layer_open },
 	{ &vop_setattr_desc,	layer_setattr },
 	{ &vop_access_desc,	layer_access },

--y0ulUmNC+osPPQO6--