Module Name: src
Committed By: hannken
Date: Sun Oct 2 13:00:07 UTC 2011
Modified Files:
src/sys/kern: vfs_vnode.c
Log Message:
The path getnewvnode()->getcleanvnode()->vclean()->VOP_LOCK() will panic
if the vnode we want to clean is a layered vnode and the caller already
locked its lower vnode.
Change getnewvnode() to always allocate a fresh vnode and add a helper
thread (vdrain) to keep the number of allocated vnodes within desiredvnodes.
Rename getcleanvnode() to cleanvnode() and let it take a vnode from the
lists, clean and free it.
Reviewed by: David Holland <dholl...@netbsd.org>
Should fix:
PR #19110 (nullfs mounts over NFS cause lock manager problems)
PR #34102 (ffs panic in NetBSD 3.0_STABLE)
PR #45115 (lock error panic when build.sh*3 and daily script is running)
PR #45355 (Reader/writer lock error: rw_vector_enter: locking against myself)
To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.12 src/sys/kern/vfs_vnode.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/vfs_vnode.c
diff -u src/sys/kern/vfs_vnode.c:1.11 src/sys/kern/vfs_vnode.c:1.12
--- src/sys/kern/vfs_vnode.c:1.11 Thu Sep 29 20:51:38 2011
+++ src/sys/kern/vfs_vnode.c Sun Oct 2 13:00:06 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_vnode.c,v 1.11 2011/09/29 20:51:38 christos Exp $ */
+/* $NetBSD: vfs_vnode.c,v 1.12 2011/10/02 13:00:06 hannken Exp $ */
/*-
* Copyright (c) 1997-2011 The NetBSD Foundation, Inc.
@@ -76,7 +76,6 @@
* starts in one of the following ways:
*
* - Allocation, via getnewvnode(9) and/or vnalloc(9).
- * - Recycle from a free list, via getnewvnode(9) -> getcleanvnode(9).
* - Reclamation of inactive vnode, via vget(9).
*
* The life-cycle ends when the last reference is dropped, usually
@@ -121,7 +120,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.11 2011/09/29 20:51:38 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.12 2011/10/02 13:00:06 hannken Exp $");
#include <sys/param.h>
#include <sys/kernel.h>
@@ -159,8 +158,10 @@ static kcondvar_t vrele_cv __cacheline_
static lwp_t * vrele_lwp __cacheline_aligned;
static int vrele_pending __cacheline_aligned;
static int vrele_gen __cacheline_aligned;
+static kcondvar_t vdrain_cv __cacheline_aligned;
-static vnode_t * getcleanvnode(void);
+static int cleanvnode(void);
+static void vdrain_thread(void *);
static void vrele_thread(void *);
static void vnpanic(vnode_t *, const char *, ...)
__attribute__((__format__(__printf__, 2, 3)));
@@ -183,7 +184,11 @@ vfs_vnode_sysinit(void)
TAILQ_INIT(&vrele_list);
mutex_init(&vrele_lock, MUTEX_DEFAULT, IPL_NONE);
+ cv_init(&vdrain_cv, "vdrain");
cv_init(&vrele_cv, "vrele");
+ error = kthread_create(PRI_VM, KTHREAD_MPSAFE, NULL, vdrain_thread,
+ NULL, NULL, "vdrain");
+ KASSERT(error == 0);
error = kthread_create(PRI_VM, KTHREAD_MPSAFE, NULL, vrele_thread,
NULL, &vrele_lwp, "vrele");
KASSERT(error == 0);
@@ -249,13 +254,12 @@ vnfree(vnode_t *vp)
}
/*
- * getcleanvnode: grab a vnode from freelist and clean it.
+ * cleanvnode: grab a vnode from freelist, clean and free it.
*
* => Releases vnode_free_list_lock.
- * => Returns referenced vnode on success.
*/
-static vnode_t *
-getcleanvnode(void)
+static int
+cleanvnode(void)
{
vnode_t *vp;
vnodelst_t *listhd;
@@ -288,7 +292,7 @@ try_nextlist:
goto try_nextlist;
}
mutex_exit(&vnode_free_list_lock);
- return NULL;
+ return EBUSY;
}
/* Remove it from the freelist. */
@@ -300,7 +304,7 @@ try_nextlist:
/*
* The vnode is still associated with a file system, so we must
- * clean it out before reusing it. We need to add a reference
+ * clean it out before freeing it. We need to add a reference
* before doing this. If the vnode gains another reference while
* being cleaned out then we lose - retry.
*/
@@ -308,15 +312,7 @@ try_nextlist:
vclean(vp, DOCLOSE);
KASSERT(vp->v_usecount >= 1 + VC_XLOCK);
atomic_add_int(&vp->v_usecount, -VC_XLOCK);
- if (vp->v_usecount == 1) {
- /* We're about to dirty it. */
- vp->v_iflag &= ~VI_CLEAN;
- mutex_exit(vp->v_interlock);
- if (vp->v_type == VBLK || vp->v_type == VCHR) {
- spec_node_destroy(vp);
- }
- vp->v_type = VNON;
- } else {
+ if (vp->v_usecount > 1) {
/*
* Don't return to freelist - the holder of the last
* reference will destroy it.
@@ -326,17 +322,26 @@ try_nextlist:
goto retry;
}
+ KASSERT((vp->v_iflag & VI_CLEAN) == VI_CLEAN);
+ mutex_exit(vp->v_interlock);
+ if (vp->v_type == VBLK || vp->v_type == VCHR) {
+ spec_node_destroy(vp);
+ }
+ vp->v_type = VNON;
+
KASSERT(vp->v_data == NULL);
KASSERT(vp->v_uobj.uo_npages == 0);
KASSERT(TAILQ_EMPTY(&vp->v_uobj.memq));
KASSERT(vp->v_numoutput == 0);
KASSERT((vp->v_iflag & VI_ONWORKLST) == 0);
- return vp;
+ vrele(vp);
+
+ return 0;
}
/*
- * getnewvnode: return the next vnode from the free list.
+ * getnewvnode: return a fresh vnode.
*
* => Returns referenced vnode, moved into the mount queue.
* => Shares the interlock specified by 'slock', if it is not NULL.
@@ -346,9 +351,8 @@ getnewvnode(enum vtagtype tag, struct mo
kmutex_t *slock, vnode_t **vpp)
{
struct uvm_object *uobj;
- static int toggle;
vnode_t *vp;
- int error = 0, tryalloc;
+ int error = 0;
try_again:
if (mp != NULL) {
@@ -361,80 +365,28 @@ try_again:
return error;
}
- /*
- * We must choose whether to allocate a new vnode or recycle an
- * existing one. The criterion for allocating a new one is that
- * the total number of vnodes is less than the number desired or
- * there are no vnodes on either free list. Generally we only
- * want to recycle vnodes that have no buffers associated with
- * them, so we look first on the vnode_free_list. If it is empty,
- * we next consider vnodes with referencing buffers on the
- * vnode_hold_list. The toggle ensures that half the time we
- * will use a buffer from the vnode_hold_list, and half the time
- * we will allocate a new one unless the list has grown to twice
- * the desired size. We are reticent to recycle vnodes from the
- * vnode_hold_list because we will lose the identity of all its
- * referencing buffers.
- */
-
vp = NULL;
+ /* Allocate a new vnode. */
mutex_enter(&vnode_free_list_lock);
-
- toggle ^= 1;
- if (numvnodes > 2 * desiredvnodes)
- toggle = 0;
-
- tryalloc = numvnodes < desiredvnodes ||
- (TAILQ_FIRST(&vnode_free_list) == NULL &&
- (TAILQ_FIRST(&vnode_hold_list) == NULL || toggle));
-
- if (tryalloc) {
- /* Allocate a new vnode. */
- numvnodes++;
+ numvnodes++;
+ if (numvnodes > desiredvnodes + desiredvnodes / 10)
+ cv_signal(&vdrain_cv);
+ mutex_exit(&vnode_free_list_lock);
+ if ((vp = vnalloc(NULL)) == NULL) {
+ mutex_enter(&vnode_free_list_lock);
+ numvnodes--;
mutex_exit(&vnode_free_list_lock);
- if ((vp = vnalloc(NULL)) == NULL) {
- mutex_enter(&vnode_free_list_lock);
- numvnodes--;
- } else
- vp->v_usecount = 1;
- }
-
- if (vp == NULL) {
- /* Recycle and get vnode clean. */
- vp = getcleanvnode();
- if (vp == NULL) {
- if (mp != NULL) {
- vfs_unbusy(mp, false, NULL);
- }
- if (tryalloc) {
- printf("WARNING: unable to allocate new "
- "vnode, retrying...\n");
- kpause("newvn", false, hz, NULL);
- goto try_again;
- }
- tablefull("vnode", "increase kern.maxvnodes or NVNODE");
- *vpp = 0;
- return ENFILE;
- }
- if ((vp->v_iflag & VI_LOCKSHARE) != 0 || slock) {
- /* We must remove vnode from the old mount point. */
- if (vp->v_mount) {
- vfs_insmntque(vp, NULL);
- }
- /* Allocate a new interlock, if it was shared. */
- if (vp->v_iflag & VI_LOCKSHARE) {
- uvm_obj_setlock(&vp->v_uobj, NULL);
- vp->v_iflag &= ~VI_LOCKSHARE;
- }
+ if (mp != NULL) {
+ vfs_unbusy(mp, false, NULL);
}
- vp->v_iflag = 0;
- vp->v_vflag = 0;
- vp->v_uflag = 0;
- vp->v_socket = NULL;
+ printf("WARNING: unable to allocate new vnode, retrying...\n");
+ kpause("newvn", false, hz, NULL);
+ goto try_again;
}
- KASSERT(vp->v_usecount == 1);
+ vp->v_usecount = 1;
+
KASSERT(vp->v_freelisthd == NULL);
KASSERT(LIST_EMPTY(&vp->v_nclist));
KASSERT(LIST_EMPTY(&vp->v_dnclist));
@@ -493,6 +445,29 @@ ungetnewvnode(vnode_t *vp)
}
/*
+ * Helper thread to keep the number of vnodes below desiredvnodes.
+ */
+static void
+vdrain_thread(void *cookie)
+{
+ int error;
+
+ mutex_enter(&vnode_free_list_lock);
+
+ for (;;) {
+ cv_timedwait(&vdrain_cv, &vnode_free_list_lock, hz);
+ while (numvnodes > desiredvnodes) {
+ error = cleanvnode();
+ if (error)
+ kpause("vndsbusy", false, hz, NULL);
+ mutex_enter(&vnode_free_list_lock);
+ if (error)
+ break;
+ }
+ }
+}
+
+/*
* Remove a vnode from its freelist.
*/
void
@@ -1204,17 +1179,19 @@ vwait(vnode_t *vp, int flags)
int
vfs_drainvnodes(long target)
{
+ int error;
- while (numvnodes > target) {
- vnode_t *vp;
+ mutex_enter(&vnode_free_list_lock);
+ while (numvnodes > target) {
+ error = cleanvnode();
+ if (error != 0)
+ return error;
mutex_enter(&vnode_free_list_lock);
- vp = getcleanvnode();
- if (vp == NULL) {
- return EBUSY;
- }
- ungetnewvnode(vp);
}
+
+ mutex_exit(&vnode_free_list_lock);
+
return 0;
}
