Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-2-0]: src/sys/kern Pull up following revision(s) (requested by ch...
details: https://anonhg.NetBSD.org/src/rev/eafda4041bb7
branches: netbsd-2-0
changeset: 564933:eafda4041bb7
user: riz <riz%NetBSD.org@localhost>
date: Thu Dec 29 01:37:32 2005 +0000
description:
Pull up following revision(s) (requested by chs in ticket #10207):
sys/kern/vfs_subr.c: revision 1.231
There is an annoying deadlock that goes like this:
* Process A is closing one file descriptor belonging to a device. In doing so,
ffs_update() is called and starts writing a block synchronously. (Note: This
leaves the vnode locked. It also has other instances -- stdin, et al -- of
the same device open, so v_usecount is definitely non-zero.)
* Process B does a revoke() on the device. The revoke() has to wait for the
vnode to be unlocked because ffs_update() is still in progress.
* Process C tries to open() the device. It wedges in checkalias() repeatedly
calling vget() because it returns EBUSY immediately.
To fix, this:
* checkalias() now uses LK_SLEEPFAIL rather than LK_NOWAIT. Therefore it will
wait for the vnode to become unlocked, but it will recheck that it is on the
hash list, in case it was in the process of being revoke()d or was revoke()d
again before we were woken up.
* Since we're relying on the vnode lock to tell us that the vnode hasn't been
removed from the hash list *anyway*, I have moved the code to remove it into
the DOCLOSE section of vclean(), inside the vnode lock.
In the example at hand, process A was sh(1), process B was a child of init(8),
and process C was syslogd(8).
diffstat:
sys/kern/vfs_subr.c | 113 +++++++++++++++++++++++++++------------------------
1 files changed, 59 insertions(+), 54 deletions(-)
diffs (177 lines):
diff -r b9d7b1b385c3 -r eafda4041bb7 sys/kern/vfs_subr.c
--- a/sys/kern/vfs_subr.c Thu Dec 29 01:36:09 2005 +0000
+++ b/sys/kern/vfs_subr.c Thu Dec 29 01:37:32 2005 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_subr.c,v 1.218.2.4 2004/06/21 10:20:07 tron Exp $ */
+/* $NetBSD: vfs_subr.c,v 1.218.2.5 2005/12/29 01:37:32 riz Exp $ */
/*-
* Copyright (c) 1997, 1998 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.218.2.4 2004/06/21 10:20:07 tron Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.218.2.5 2005/12/29 01:37:32 riz Exp $");
#include "opt_inet.h"
#include "opt_ddb.h"
@@ -1142,15 +1142,21 @@
* Alias, but not in use, so flush it out.
*/
simple_lock(&vp->v_interlock);
+ simple_unlock(&spechash_slock);
if (vp->v_usecount == 0) {
- simple_unlock(&spechash_slock);
vgonel(vp, p);
goto loop;
}
- if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK | LK_NOWAIT)) {
- simple_unlock(&spechash_slock);
+ /*
+ * What we're interested to know here is if someone else has
+ * removed this vnode from the device hash list while we were
+ * waiting. This can only happen if vclean() did it, and
+ * this requires the vnode to be locked. Therefore, we use
+ * LK_SLEEPFAIL and retry.
+ */
+ if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK | LK_SLEEPFAIL))
goto loop;
- }
+ simple_lock(&spechash_slock);
break;
}
if (vp == NULL || vp->v_tag != VT_NON || vp->v_type != VBLK) {
@@ -1606,9 +1612,13 @@
/*
* Clean out any cached data associated with the vnode.
+ * If special device, remove it from special device alias list.
+ * if it is on one.
*/
if (flags & DOCLOSE) {
int error;
+ struct vnode *vq, *vx;
+
vn_start_write(vp, &mp, V_WAIT | V_LOWER);
error = vinvalbuf(vp, V_SAVE, NOCRED, p, 0, 0);
vn_finished_write(mp, V_LOWER);
@@ -1616,6 +1626,49 @@
error = vinvalbuf(vp, 0, NOCRED, p, 0, 0);
KASSERT(error == 0);
KASSERT((vp->v_flag & VONWORKLST) == 0);
+
+ if (active)
+ VOP_CLOSE(vp, FNONBLOCK, NOCRED, NULL);
+
+ if ((vp->v_type == VBLK || vp->v_type == VCHR) &&
+ vp->v_specinfo != 0) {
+ simple_lock(&spechash_slock);
+ if (vp->v_hashchain != NULL) {
+ if (*vp->v_hashchain == vp) {
+ *vp->v_hashchain = vp->v_specnext;
+ } else {
+ for (vq = *vp->v_hashchain; vq;
+ vq = vq->v_specnext) {
+ if (vq->v_specnext != vp)
+ continue;
+ vq->v_specnext = vp->v_specnext;
+ break;
+ }
+ if (vq == NULL)
+ panic("missing bdev");
+ }
+ if (vp->v_flag & VALIASED) {
+ vx = NULL;
+ for (vq = *vp->v_hashchain; vq;
+ vq = vq->v_specnext) {
+ if (vq->v_rdev != vp->v_rdev ||
+ vq->v_type != vp->v_type)
+ continue;
+ if (vx)
+ break;
+ vx = vq;
+ }
+ if (vx == NULL)
+ panic("missing alias");
+ if (vq == NULL)
+ vx->v_flag &= ~VALIASED;
+ vp->v_flag &= ~VALIASED;
+ }
+ }
+ simple_unlock(&spechash_slock);
+ FREE(vp->v_specinfo, M_VNODE);
+ vp->v_specinfo = NULL;
+ }
}
LOCK_ASSERT(!simple_lock_held(&vp->v_interlock));
@@ -1625,8 +1678,6 @@
* VOP_INACTIVE will unlock the vnode.
*/
if (active) {
- if (flags & DOCLOSE)
- VOP_CLOSE(vp, FNONBLOCK, NOCRED, NULL);
VOP_INACTIVE(vp, p);
} else {
/*
@@ -1732,8 +1783,6 @@
struct vnode *vp;
struct proc *p;
{
- struct vnode *vq;
- struct vnode *vx;
LOCK_ASSERT(simple_lock_held(&vp->v_interlock));
@@ -1763,50 +1812,6 @@
insmntque(vp, (struct mount *)0);
/*
- * If special device, remove it from special device alias list.
- * if it is on one.
- */
-
- if ((vp->v_type == VBLK || vp->v_type == VCHR) && vp->v_specinfo != 0) {
- simple_lock(&spechash_slock);
- if (vp->v_hashchain != NULL) {
- if (*vp->v_hashchain == vp) {
- *vp->v_hashchain = vp->v_specnext;
- } else {
- for (vq = *vp->v_hashchain; vq;
- vq = vq->v_specnext) {
- if (vq->v_specnext != vp)
- continue;
- vq->v_specnext = vp->v_specnext;
- break;
- }
- if (vq == NULL)
- panic("missing bdev");
- }
- if (vp->v_flag & VALIASED) {
- vx = NULL;
- for (vq = *vp->v_hashchain; vq;
- vq = vq->v_specnext) {
- if (vq->v_rdev != vp->v_rdev ||
- vq->v_type != vp->v_type)
- continue;
- if (vx)
- break;
- vx = vq;
- }
- if (vx == NULL)
- panic("missing alias");
- if (vq == NULL)
- vx->v_flag &= ~VALIASED;
- vp->v_flag &= ~VALIASED;
- }
- }
- simple_unlock(&spechash_slock);
- FREE(vp->v_specinfo, M_VNODE);
- vp->v_specinfo = NULL;
- }
-
- /*
* The test of the back pointer and the reference count of
* zero is because it will be removed from the free list by
* getcleanvnode, but will not have its reference count
Home |
Main Index |
Thread Index |
Old Index