CVS commit: src/sys/kern

[YAMAMOTO Takashi]

Module Name:    src
Committed By:   yamt
Date:           Fri Dec 17 22:34:04 UTC 2010

Modified Files:
        src/sys/kern: vfs_lookup.c

Log Message:
- lookup_once: when crossing a mount point, don't keep the parent vnode locked.
  ie. don't lock a vnode while holding another vnode which belongs to a
  different filesystem.  otherwise we propagate slowness (or deadness) of a
  filesystem to another via vnode lock chain.
- lookup_parsepath: don't alter vnode states.  let the caller do it instead.
- add comments and assertions.


To generate a diff of this commit:
cvs rdiff -u -r1.125 -r1.126 src/sys/kern/vfs_lookup.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_lookup.c
diff -u src/sys/kern/vfs_lookup.c:1.125 src/sys/kern/vfs_lookup.c:1.126
--- src/sys/kern/vfs_lookup.c:1.125	Tue Nov 30 10:43:05 2010
+++ src/sys/kern/vfs_lookup.c	Fri Dec 17 22:34:04 2010
@@ -1,4 +1,4 @@
-/*	$NetBSD: vfs_lookup.c,v 1.125 2010/11/30 10:43:05 dholland Exp $	*/
+/*	$NetBSD: vfs_lookup.c,v 1.126 2010/12/17 22:34:04 yamt Exp $	*/
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.125 2010/11/30 10:43:05 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.126 2010/12/17 22:34:04 yamt Exp $");
 
 #include "opt_magiclinks.h"
 
@@ -696,6 +696,7 @@
 		}
 		return error;
 	}
+	KASSERT(VOP_ISLOCKED(state->namei_startdir) == LK_EXCLUSIVE);
 
 	/* Loop through symbolic links */
 	for (;;) {
@@ -904,6 +905,11 @@
 	return 0;
 }
 
+/*
+ * lookup_parsepath: consume a component name from state->ndp and prepare
+ * state->cnp for it.
+ */
+
 static int
 lookup_parsepath(struct namei_state *state)
 {
@@ -922,16 +928,12 @@
 	 * cnp->cn_nameptr for callers that need the name. Callers needing
 	 * the name set the SAVENAME flag. When done, they assume
 	 * responsibility for freeing the pathname buffer.
-	 *
-	 * At this point, our only vnode state is that "dp" is held and locked.
 	 */
 	cnp->cn_consume = 0;
 	cp = NULL;
 	cnp->cn_hash = namei_hash(cnp->cn_nameptr, &cp);
 	cnp->cn_namelen = cp - cnp->cn_nameptr;
 	if (cnp->cn_namelen > NAME_MAX) {
-		vput(state->dp);
-		ndp->ni_dvp = NULL;
 		return ENAMETOOLONG;
 	}
 #ifdef NAMEI_DIAGNOSTIC
@@ -982,6 +984,21 @@
 	return 0;
 }
 
+/*
+ * lookup_once: look up the vnode for the next component name (state->cnp).
+ *
+ * takes care of dot-dot, cross-mount, and MNT_UNION.
+ *
+ * inputs:
+ *	state->dp	the parent vnode
+ *	state->cnp	the componentname to lookup
+ *
+ * outputs:
+ *	state->dp	the result vnode
+ *	ndp->ni_vp	updated to state->dp
+ *	ndp->ni_dvp	updated to the parent directory vnode
+ */
+
 static int
 lookup_once(struct namei_state *state)
 {
@@ -1136,6 +1153,8 @@
 	 * "state->dp" and "ndp->ni_dvp" are both locked and held,
 	 * and may be the same vnode.
 	 */
+	KASSERT(VOP_ISLOCKED(state->dp) == LK_EXCLUSIVE);
+	KASSERT(VOP_ISLOCKED(ndp->ni_dvp) == LK_EXCLUSIVE);
 
 	/*
 	 * Check to see if the vnode has been mounted on;
@@ -1157,10 +1176,9 @@
 			vn_lock(ndp->ni_dvp, LK_EXCLUSIVE | LK_RETRY);
 			return error;
 		}
-		VOP_UNLOCK(tdp);
-		ndp->ni_vp = state->dp = tdp;
-		vn_lock(ndp->ni_dvp, LK_EXCLUSIVE | LK_RETRY);
-		vn_lock(ndp->ni_vp, LK_EXCLUSIVE | LK_RETRY);
+		vrele(ndp->ni_dvp);
+		vref(tdp);
+		ndp->ni_dvp = ndp->ni_vp = state->dp = tdp;
 	}
 
 	return 0;
@@ -1187,8 +1205,14 @@
 	}
 
 dirloop:
+	/*
+	 * At this point, our only vnode state is that "dp" is held and locked.
+	 */
+	KASSERT(VOP_ISLOCKED(state->dp) == LK_EXCLUSIVE);
+	KASSERT(ndp->ni_dvp == NULL);
 	error = lookup_parsepath(state);
 	if (error) {
+		vput(state->dp);
 		goto bad;
 	}
 
@@ -1235,6 +1259,7 @@
 		} else {
 			vput(ndp->ni_dvp);
 		}
+		ndp->ni_dvp = NULL;
 		goto dirloop;
 	}