[src/ad-namecache]: src/sys/kern Fast-forward through the namecache was stopp...

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

details:   https://anonhg.NetBSD.org/src/rev/7a9acf46e914
branches:  ad-namecache
changeset: 850540:7a9acf46e914
user:      ad <ad%NetBSD.org@localhost>
date:      Wed Jan 22 12:10:46 2020 +0000

description:
Fast-forward through the namecache was stopping one component too soon when
there was an obstacle, e.g. a mountpoint.  The obstacle should be returned
not the parent directory.

diffstat:

 sys/kern/vfs_lookup.c |  53 +++++++++++++++++++++++++-------------------------
 1 files changed, 27 insertions(+), 26 deletions(-)

diffs (130 lines):

diff -r c9830b30ecdf -r 7a9acf46e914 sys/kern/vfs_lookup.c
--- a/sys/kern/vfs_lookup.c     Wed Jan 22 12:04:36 2020 +0000
+++ b/sys/kern/vfs_lookup.c     Wed Jan 22 12:10:46 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_lookup.c,v 1.212.4.4 2020/01/19 21:19:25 ad Exp $  */
+/*     $NetBSD: vfs_lookup.c,v 1.212.4.5 2020/01/22 12:10:46 ad Exp $  */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.212.4.4 2020/01/19 21:19:25 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.212.4.5 2020/01/22 12:10:46 ad Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_magiclinks.h"
@@ -954,7 +954,11 @@
            (mp = foundobj->v_mountedhere) != NULL &&
            (cnp->cn_flags & NOCROSSMOUNT) == 0) {
                KASSERTMSG(searchdir != foundobj, "same vn %p", searchdir);
-               /* First get the vnode stable. */
+               /*
+                * First get the vnode stable.  LK_SHARED works brilliantly
+                * here because almost nothing else wants to lock the
+                * covered vnode.
+                */
                error = vn_lock(foundobj, LK_SHARED);
                if (error != 0) {
                        vrele(foundobj);
@@ -962,7 +966,7 @@
                        break;
                }
 
-               /* Then check to see if something is still mounted there. */
+               /* Then check to see if something is still mounted on it. */
                if ((mp = foundobj->v_mountedhere) == NULL) {
                        VOP_UNLOCK(foundobj);
                        break;
@@ -1109,7 +1113,9 @@
        /*
         * If the file system supports VOP_LOOKUP() with a shared lock, and
         * we are not making any modifications (nameiop LOOKUP) or this is
-        * not the last component then get a shared lock LK_SHARED.
+        * not the last component then get a shared lock LK_SHARED.  Where
+        * we can't do fast-forwarded lookups (for example with layered file
+        * systems) then this is the fallback for reducing lock contention.
         */
        if ((searchdir->v_mount->mnt_iflag & IMNT_SHRLOOKUP) != 0 &&
            (cnp->cn_nameiop == LOOKUP || (cnp->cn_flags & ISLASTCN) == 0)) {
@@ -1237,8 +1243,8 @@
 /*
  * Parse out the first path name component that we need to to consider. 
  *
- * While doing this, attempt to use the name cache to fastforward through as
- * many "easy" to find components of the path as possible.
+ * While doing this, attempt to use the name cache to fast-forward through
+ * as many "easy" to find components of the path as possible.
  *
  * We use the namecache's node locks to form a chain, and avoid as many
  * vnode references and locks as possible.  In the ideal case, only the
@@ -1280,8 +1286,7 @@
                /*
                 * Can't deal with dotdot lookups, because it means lock
                 * order reversal, and there are checks in lookup_once()
-                * that need to be made.  Also check for missing mountpoints
-                * (XXX racy).
+                * that need to be made.  Also check for missing mountpoints.
                 */
                if ((cnp->cn_flags & ISDOTDOT) != 0 ||
                    (*searchdir)->v_mount == NULL) {
@@ -1322,8 +1327,16 @@
                        break;
                }
 
-               /* Stop if we've reached the last component: get vnode. */
-               if (cnp->cn_flags & ISLASTCN) {
+               /*
+                * Stop and get a hold on the vnode if there's something
+                * that can't be handled here:
+                *
+                * - we've reached the last component.
+                * - or encountered a mount point that needs to be crossed.
+                * - or encountered something other than a directory.
+                */
+               if ((cnp->cn_flags & ISLASTCN) != 0 || vp->v_type != VDIR ||
+                   (vp->v_type == VDIR && vp->v_mountedhere != NULL)) {
                        mutex_enter(vp->v_interlock);
                        error = vcache_tryvget(vp);
                        /* v_interlock now released */
@@ -1335,17 +1348,6 @@
                }
 
                /*
-                * Not the last component.  If we found something other than
-                * a directory, or it's a directory with a filesystem
-                * mounted on it, bail out.
-                */
-               if (vp->v_type != VDIR || vp->v_mountedhere != NULL) {
-                       error = EOPNOTSUPP;
-                       vp = NULL;
-                       break;
-               }
-
-               /*
                 * Otherwise, we're still in business.  Set the found VDIR
                 * vnode as the search dir for the next component and
                 * continue on to it.
@@ -1365,9 +1367,8 @@
                mutex_enter((*searchdir)->v_interlock);
                error2 = vcache_tryvget(*searchdir);
                /* v_interlock now unheld */
-               if (plock != NULL) {
-                       rw_exit(plock);
-               }
+               KASSERT(plock != NULL);
+               rw_exit(plock);
                if (__predict_true(error2 == 0)) {
                        vrele(origsearchdir);
                } else {
@@ -1447,7 +1448,7 @@
                /*
                 * Parse out the first path name component that we need to
                 * to consider.  While doing this, attempt to use the name
-                * cache to fastforward through as many "easy" to find
+                * cache to fast-forward through as many "easy" to find
                 * components of the path as possible.
                 */
                error = lookup_fastforward(state, &searchdir, &foundobj);