Module Name: src
Committed By: snj
Date: Tue Jul 21 00:29:33 UTC 2009
Modified Files:
src/sys/kern [netbsd-5-0]: vfs_subr.c
src/sys/sys [netbsd-5-0]: vnode.h
Log Message:
Pull up following revision(s) (requested by rmind in ticket #863):
sys/sys/vnode.h: revision 1.207
sys/kern/vfs_subr.c: revision 1.379
put a flag bit into v_usecount to prevent vtryget during getcleanvnode.
this fixes the following deadlock.
a thread doing getcleanvnode:
pick a vnode
acqure v_interlock
v_usecount++
call vclean
now, another thread doing cache_lookup:
picks the vnode
vtryget succeed
vn_lock succeed
now in vclean:
set VI_XLOCK (too late to be noticed by the competing thread)
wait on the vnode lock (this might violate locking order)
the use of a flag bit was suggested by Andrew Doran. PR/41374.
To generate a diff of this commit:
cvs rdiff -u -r1.357.4.4 -r1.357.4.4.2.1 src/sys/kern/vfs_subr.c
cvs rdiff -u -r1.197 -r1.197.10.1 src/sys/sys/vnode.h
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_subr.c
diff -u src/sys/kern/vfs_subr.c:1.357.4.4 src/sys/kern/vfs_subr.c:1.357.4.4.2.1
--- src/sys/kern/vfs_subr.c:1.357.4.4 Mon Feb 16 03:33:16 2009
+++ src/sys/kern/vfs_subr.c Tue Jul 21 00:29:32 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_subr.c,v 1.357.4.4 2009/02/16 03:33:16 snj Exp $ */
+/* $NetBSD: vfs_subr.c,v 1.357.4.4.2.1 2009/07/21 00:29:32 snj Exp $ */
/*-
* Copyright (c) 1997, 1998, 2004, 2005, 2007, 2008 The NetBSD Foundation, Inc.
@@ -76,12 +76,22 @@
* change from a non-zero value to zero, again the interlock must be
* held.
*
- * Changing the usecount from a non-zero value to a non-zero value can
- * safely be done using atomic operations, without the interlock held.
+ * There's a flag bit, VC_XLOCK, embedded in v_usecount.
+ * To raise v_usecount, if the VC_XLOCK bit is set in it, the interlock
+ * must be held.
+ * To modify the VC_XLOCK bit, the interlock must be held.
+ * We always keep the usecount (v_usecount & VC_MASK) non-zero while the
+ * VC_XLOCK bit is set.
+ *
+ * Unless the VC_XLOCK bit is set, changing the usecount from a non-zero
+ * value to a non-zero value can safely be done using atomic operations,
+ * without the interlock held.
+ * Even if the VC_XLOCK bit is set, decreasing the usecount to a non-zero
+ * value can be done using atomic operations, without the interlock held.
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.357.4.4 2009/02/16 03:33:16 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.357.4.4.2.1 2009/07/21 00:29:32 snj Exp $");
#include "opt_ddb.h"
#include "opt_compat_netbsd.h"
@@ -366,8 +376,10 @@
* before doing this. If the vnode gains another reference while
* being cleaned out then we lose - retry.
*/
- atomic_inc_uint(&vp->v_usecount);
+ atomic_add_int(&vp->v_usecount, 1 + VC_XLOCK);
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;
@@ -1221,7 +1233,7 @@
return false;
}
for (use = vp->v_usecount;; use = next) {
- if (use == 0) {
+ if (use == 0 || __predict_false((use & VC_XLOCK) != 0)) {
/* Need interlock held if first reference. */
return false;
}
@@ -1310,9 +1322,10 @@
u_int use, next;
for (use = vp->v_usecount;; use = next) {
- if (use == 1) {
+ if (use == 1) {
return false;
}
+ KASSERT((use & VC_MASK) > 1);
next = atomic_cas_uint(&vp->v_usecount, use, use - 1);
if (__predict_true(next == use)) {
return true;
Index: src/sys/sys/vnode.h
diff -u src/sys/sys/vnode.h:1.197 src/sys/sys/vnode.h:1.197.10.1
--- src/sys/sys/vnode.h:1.197 Thu Jul 31 05:38:06 2008
+++ src/sys/sys/vnode.h Tue Jul 21 00:29:32 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: vnode.h,v 1.197 2008/07/31 05:38:06 simonb Exp $ */
+/* $NetBSD: vnode.h,v 1.197.10.1 2009/07/21 00:29:32 snj Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -249,6 +249,12 @@
#define VSIZENOTSET ((voff_t)-1)
/*
+ * v_usecount; see the comment in vfs_subr.c
+ */
+#define VC_XLOCK 0x80000000
+#define VC_MASK 0x7fffffff
+
+/*
* Vnode attributes. A field value of VNOVAL represents a field whose value
* is unavailable (getattr) or which is not to be changed (setattr).
*/
