Date: Tue, 18 Apr 2000 14:18:24 -0400 From: Dave Chapeskie <dchapes@borderware.com> To: Adrian Chadd <adrian@FreeBSD.ORG> Cc: Matthew Dillon <dillon@apollo.backplane.com>, freebsd-fs@FreeBSD.ORG Subject: Re: vnode_free_list corruption Message-ID: <00Apr18.141542edt.117123@gateway.borderware.com> In-Reply-To: <20000418174608.C71428@ewok.creative.net.au>; from Adrian Chadd on Tue, Apr 18, 2000 at 05:46:11PM %2B0800 References: <00Apr14.141908edt.117140@gateway.borderware.com> <20000415023148.F34852@ewok.creative.net.au> <200004141835.LAA71253@apollo.backplane.com> <20000418042733.I59015@ewok.creative.net.au> <00Apr17.185117edt.117127@gateway.borderware.com> <20000418174608.C71428@ewok.creative.net.au>
next in thread | previous in thread | raw e-mail | index | archive | help
--BOKacYhQ+x31HxR3
Content-Type: text/plain; charset=us-ascii
On Tue, Apr 18, 2000 at 05:46:11PM +0800, Adrian Chadd wrote:
> Yes, but from my take of the code, if a vnode reaches VDOOMED, its been
> earmarked for recycling and is in the process of being flushed. If
> the vnode is being used in the FS code somewhere (or anywhere for that
> matter :) it shouldn't ever be considered for recycling as it should be
> vref()'ed or at the least vhold()'ed.
I see it the other way around, getnewvnode is perfectly within it's
rights to call vgonel() on a VOP_LOCKED vnode from what I can see (see
code comment below). I think the problem is that vhold and vbusy need
to be checking for VXLOCK and returning an error just the way vget does.
If that's the case it seems silly to have both vhold and vget.
The vclean() call (from vgonel called by getnewvnode) must be blocking.
It's the only place between getnewvnode()'s setting VDOOMED and it's
later clearing of the flags (assuming VXLOCK isn't already set) where it
can block. There is a comment in vclean() which says:
/*
* Even if the count is zero, the VOP_INACTIVE routine may still
* have the object locked while it cleans it out. The VOP_LOCK
* ensures that the VOP_INACTIVE routine is done with its work.
* For active vnodes, it ensures that no other activity can
* occur while the underlying object is being cleaned out.
*/
VOP_LOCK(vp, LK_DRAIN | LK_INTERLOCK, p);
Alternatively it may sometimes be blocking in the vinvalbuf() call.
I just repeated the problem again with some extra kernel debugging. One
of the 'head' processes calls getnewvnode and it blocks with a wait
message of "inode", I think that means its blocked in the VOP_LOCK call
waiting for the VOP_LOCK.
vprint() from the getnewvnode call looks like this:
getnewvnode: 0xcd2e7200: type VREG, usecount 0, writecount 0,
refcount 0, flags (VFREE)
tag VT_UFS, ino 27201, on dev 0x20015 (0, 131093)
lock type inode: EXCL (count 1) by pid 611
getnewvnode: pid 1455 recycling VOP_ISLOCKED vnode!
The vprint() from vbusy looks like this:
vbusy: 0xcd2e7200: type VREG, usecount 0, writecount 0,
refcount 1, flags (VXLOCK|VDOOMED|VFREE)
tag VT_UFS, ino 27201, on dev 0x20015 (0, 131093)
lock type inode: EXCL (count 1) by pid 611
panic: vbusy on VDOOMED vnode
pid 611 is 'rm', pid 1455 is 'head' with a wait message of "inode".
So it looks like my panic call in vbusy should be checking for VXLOCK
instead of VDOOMED (since the later is only set/checked from within
getnewvnode it's better not to make other parts of the system know about
it).
> Right, I'll drop MAXMEM down and try to starve the system further, and
> see what happens.
Also make sure softupdates is off since it could easily be changing the
timing (all my test were done with it off).
Also try the attached patch in order to make the problem easier to
replicate. It makes getnewvnode _try_ and find a VOP_LOCKED vnode to
recycle. It still only picks vnodes that might otherwise have been
selected (if they were closer to the front of the inactive list). It of
course slows things down since it often walks the complete free list but
that shouldn't matter for the purposes of this test.
With similar changes in my kernel it can still takes a couple of minutes
for the vbusy panic to occur (although I can do it with fewer instances
of my test scripts running). You'll probably notice that getnewvnode
does successfully recycle several VOP_LOCKED vnodes before vbusy() gets
called on one.
--
Dave Chapeskie
Senior Software Engineer
Borderware Technologies Inc.
Mississauga, Ontario, Canada
--BOKacYhQ+x31HxR3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="test.diff"
diff -u -r1.253 vfs_subr.c
--- kern/vfs_subr.c 2000/03/20 11:28:45 1.253
+++ kern/vfs_subr.c 2000/04/18 18:12:58
@@ -112,6 +112,14 @@
SYSCTL_INT(_vfs, OID_AUTO, reassignbufsortbad, CTLFLAG_RW, &reassignbufsortbad, 0, "");
static int reassignbufmethod = 1;
SYSCTL_INT(_vfs, OID_AUTO, reassignbufmethod, CTLFLAG_RW, &reassignbufmethod, 0, "");
+#ifdef DDB
+static int skip_vop_locked = 0;
+SYSCTL_INT(_debug, OID_AUTO, skip_vop_locked, CTLFLAG_RW, &skip_vop_locked, 0,
+ "move VOP_LOCKED vnodes to the back of the free list");
+static int find_vop_locked = 1;
+SYSCTL_INT(_debug, OID_AUTO, find_vop_locked, CTLFLAG_RW, &find_vop_locked, 0,
+ "try and cause problems by looking for VOP_LOCKED vnodes to recycle");
+#endif
#ifdef ENABLE_VFS_IOOPT
int vfs_ioopt = 0;
@@ -453,6 +461,9 @@
struct vnode *vp, *tvp, *nvp;
vm_object_t object;
TAILQ_HEAD(freelst, vnode) vnode_tmp_list;
+#ifdef DDB
+ struct vnode *non_locked = NULL;
+#endif
/*
* We take the least recently used vnode from the freelist
@@ -507,7 +518,35 @@
/* Don't recycle if active in the namecache */
simple_unlock(&vp->v_interlock);
continue;
+ } else if (VOP_ISLOCKED(vp)) {
+#ifdef DDB
+ vprint("getnewvnode", vp);
+ if (!skip_vop_locked) {
+ printf (getnewvnode: "pid %ld recycling"
+ " VOP_ISLOCKED vnode!\n",
+ curproc ? curproc->p_pid : 0);
+ break;
+ }
+ printf ("getnewvnode: pushing VOP_ISLOCKED"
+ " vnode to end of list\n");
+#endif
+ TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
+ TAILQ_INSERT_TAIL(&vnode_tmp_list, vp, v_freelist);
+ continue;
} else {
+#ifdef DDB
+ if (!skip_vop_locked && find_vop_locked) {
+ /*
+ * To illistrate a problem look for
+ * VOP_LOCKED vnodes to recycle,
+ * but remember the first non-locked
+ * vnode
+ */
+ if (non_locked == NULL)
+ non_locked = vp;
+ continue;
+ } else
+#endif
break;
}
}
@@ -520,6 +559,11 @@
simple_unlock(&tvp->v_interlock);
}
+#ifdef DDB
+ /* If there are no locked vnodes, use the first non-locked one */
+ if (vp == NULL && non_locked != NULL)
+ vp = non_locked;
+#endif
if (vp) {
vp->v_flag |= VDOOMED;
TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
@@ -2613,6 +2657,13 @@
int s;
s = splbio();
+ if (vp->v_flag & VDOOMED|VXLOCK) {
+#ifdef DIAGNOSTIC
+ vprint ("vbusy", vp);
+ printf ("vbusy by pid %ld\n", curproc ? curproc->p_pid : 0);
+#endif
+ panic ("vbusy on VDOOMED or VXLOCKed vnode");
+ }
simple_lock(&vnode_free_list_slock);
if (vp->v_flag & VTBFREE) {
TAILQ_REMOVE(&vnode_tobefree_list, vp, v_freelist);
--BOKacYhQ+x31HxR3--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-fs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00Apr18.141542edt.117123>