[src/trunk]: src/sys/kern - lookup_once: when crossing a mount point, don't k...

[yamt <yamt%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/8bd4cc377455
branches:  trunk
changeset: 759842:8bd4cc377455
user:      yamt <yamt%NetBSD.org@localhost>
date:      Fri Dec 17 22:34:04 2010 +0000

description:
- 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.

diffstat:

 sys/kern/vfs_lookup.c |  45 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 35 insertions(+), 10 deletions(-)

diffs (123 lines):

diff -r 450cee8cb4d6 -r 8bd4cc377455 sys/kern/vfs_lookup.c
--- a/sys/kern/vfs_lookup.c     Fri Dec 17 22:24:11 2010 +0000
+++ b/sys/kern/vfs_lookup.c     Fri Dec 17 22:34:04 2010 +0000
@@ -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;
        }