[src/trunk]: src/sys/fs/union Make union_newlower() ans union_newupper() loca...

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

details:   https://anonhg.NetBSD.org/src/rev/b97c73cbc9ea
branches:  trunk
changeset: 825481:b97c73cbc9ea
user:      hannken <hannken%NetBSD.org@localhost>
date:      Mon Jul 17 09:22:36 2017 +0000

description:
Make union_newlower() ans union_newupper() local to union_subr.c,
expand and remove union_updatevp() and take care to transfer the
vnode lock from the union vnode to its new upper vnode without
breaking the fstrans state.

Add assertions that un_lowervp and un_uppervp never change from
non-NULL to non-NULL.

diffstat:

 sys/fs/union/union.h      |    4 +-
 sys/fs/union/union_subr.c |  136 ++++++++++++++++++++++-----------------------
 2 files changed, 68 insertions(+), 72 deletions(-)

diffs (214 lines):

diff -r 974d95b3dd58 -r b97c73cbc9ea sys/fs/union/union.h
--- a/sys/fs/union/union.h      Sun Jul 16 21:40:28 2017 +0000
+++ b/sys/fs/union/union.h      Mon Jul 17 09:22:36 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union.h,v 1.28 2015/02/16 10:22:00 hannken Exp $       */
+/*     $NetBSD: union.h,v 1.29 2017/07/17 09:22:36 hannken Exp $       */
 
 /*
  * Copyright (c) 1994 The Regents of the University of California.
@@ -158,8 +158,6 @@
     struct lwp *);
 extern void union_removed_upper(struct union_node *un);
 extern struct vnode *union_lowervp(struct vnode *);
-extern void union_newlower(struct union_node *, struct vnode *);
-extern void union_newupper(struct union_node *, struct vnode *);
 extern void union_newsize(struct vnode *, off_t, off_t);
 int union_readdirhook(struct vnode **, struct file *, struct lwp *);
 
diff -r 974d95b3dd58 -r b97c73cbc9ea sys/fs/union/union_subr.c
--- a/sys/fs/union/union_subr.c Sun Jul 16 21:40:28 2017 +0000
+++ b/sys/fs/union/union_subr.c Mon Jul 17 09:22:36 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_subr.c,v 1.75 2017/06/01 02:45:13 chs Exp $      */
+/*     $NetBSD: union_subr.c,v 1.76 2017/07/17 09:22:36 hannken Exp $  */
 
 /*
  * Copyright (c) 1994
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.75 2017/06/01 02:45:13 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.76 2017/07/17 09:22:36 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -104,7 +104,8 @@
 
 static kmutex_t uhash_lock;
 
-void union_updatevp(struct union_node *, struct vnode *, struct vnode *);
+static void union_newupper(struct union_node *, struct vnode *);
+static void union_newlower(struct union_node *, struct vnode *);
 static void union_ref(struct union_node *);
 static void union_rele(struct union_node *);
 static int union_do_lookup(struct vnode *, struct componentname *, kauth_cred_t,    const char *);
@@ -160,71 +161,28 @@
 }
 
 void
-union_updatevp(struct union_node *un, struct vnode *uppervp,
-       struct vnode *lowervp)
+union_newlower(struct union_node *un, struct vnode *lowervp)
 {
        int ohash = UNION_HASH(un->un_uppervp, un->un_lowervp);
-       int nhash = UNION_HASH(uppervp, lowervp);
-       int docache = (lowervp != NULLVP || uppervp != NULLVP);
-       bool un_unlock;
+       int nhash = UNION_HASH(un->un_uppervp, lowervp);
+
+       if (un->un_lowervp == lowervp)
+               return;
 
        KASSERT(VOP_ISLOCKED(UNIONTOV(un)) == LK_EXCLUSIVE);
+       KASSERT(un->un_lowervp == NULL);
 
        mutex_enter(&uhash_lock);
 
-       if (!docache || ohash != nhash) {
-               if (un->un_cflags & UN_CACHED) {
-                       un->un_cflags &= ~UN_CACHED;
-                       LIST_REMOVE(un, un_cache);
-               }
-       }
-
-       if (un->un_lowervp != lowervp) {
-               if (un->un_lowervp) {
-                       vrele(un->un_lowervp);
-                       if (un->un_path) {
-                               free(un->un_path, M_TEMP);
-                               un->un_path = 0;
-                       }
-                       if (un->un_dirvp) {
-                               vrele(un->un_dirvp);
-                               un->un_dirvp = NULLVP;
-                       }
-               }
-               un->un_lowervp = lowervp;
-               mutex_enter(&un->un_lock);
-               un->un_lowersz = VNOVAL;
-               mutex_exit(&un->un_lock);
+       if (ohash != nhash && (un->un_cflags & UN_CACHED)) {
+               un->un_cflags &= ~UN_CACHED;
+               LIST_REMOVE(un, un_cache);
        }
-
-       if (un->un_uppervp != uppervp) {
-               if (un->un_uppervp) {
-                       un_unlock = false;
-                       vrele(un->un_uppervp);
-               } else
-                       un_unlock = true;
-
-               mutex_enter(&un->un_lock);
-               un->un_uppervp = uppervp;
-               mutex_exit(&un->un_lock);
-               if (un_unlock) {
-                       struct vop_unlock_args ap;
-
-                       ap.a_vp = UNIONTOV(un);
-                       genfs_unlock(&ap);
-               }
-               mutex_enter(&un->un_lock);
-               un->un_uppersz = VNOVAL;
-               mutex_exit(&un->un_lock);
-               /* Update union vnode interlock. */
-               if (uppervp != NULL) {
-                       mutex_obj_hold(uppervp->v_interlock);
-                       uvm_obj_setlock(&UNIONTOV(un)->v_uobj,
-                           uppervp->v_interlock);
-               }
-       }
-
-       if (docache && (ohash != nhash)) {
+       mutex_enter(&un->un_lock);
+       un->un_lowervp = lowervp;
+       un->un_lowersz = VNOVAL;
+       mutex_exit(&un->un_lock);
+       if (ohash != nhash) {
                LIST_INSERT_HEAD(&uhashtbl[nhash], un, un_cache);
                un->un_cflags |= UN_CACHED;
        }
@@ -233,17 +191,57 @@
 }
 
 void
-union_newlower(struct union_node *un, struct vnode *lowervp)
-{
-
-       union_updatevp(un, un->un_uppervp, lowervp);
-}
-
-void
 union_newupper(struct union_node *un, struct vnode *uppervp)
 {
+       int ohash = UNION_HASH(un->un_uppervp, un->un_lowervp);
+       int nhash = UNION_HASH(uppervp, un->un_lowervp);
+       struct vop_lock_args lock_ap;
+       struct vop_unlock_args unlock_ap;
+       int error __diagused;
 
-       union_updatevp(un, uppervp, un->un_lowervp);
+       if (un->un_uppervp == uppervp)
+               return;
+
+       KASSERT(VOP_ISLOCKED(UNIONTOV(un)) == LK_EXCLUSIVE);
+       KASSERT(un->un_uppervp == NULL);
+
+       /*
+        * We have to transfer the vnode lock from the union vnode to
+        * the upper vnode.  Lock the upper vnode first.  We cannot use
+        * VOP_LOCK() here as it would break the fstrans state.
+        */
+       lock_ap.a_desc = VDESC(vop_lock);
+       lock_ap.a_vp = uppervp;
+       lock_ap.a_flags = LK_EXCLUSIVE;
+       error = VCALL(lock_ap.a_vp,  VOFFSET(vop_lock), &lock_ap);
+       KASSERT(error == 0);
+
+       mutex_enter(&uhash_lock);
+
+       if (ohash != nhash && (un->un_cflags & UN_CACHED)) {
+               un->un_cflags &= ~UN_CACHED;
+               LIST_REMOVE(un, un_cache);
+       }
+       mutex_enter(&un->un_lock);
+       un->un_uppervp = uppervp;
+       un->un_uppersz = VNOVAL;
+       /*
+        * With the upper vnode in place unlock the union vnode to
+        * finalize the lock transfer.
+        */
+       unlock_ap.a_desc = VDESC(vop_unlock);
+       unlock_ap.a_vp = UNIONTOV(un);
+       genfs_unlock(&unlock_ap);
+       /* Update union vnode interlock. */
+       mutex_obj_hold(uppervp->v_interlock);
+       uvm_obj_setlock(&UNIONTOV(un)->v_uobj, uppervp->v_interlock);
+       mutex_exit(&un->un_lock);
+       if (ohash != nhash) {
+               LIST_INSERT_HEAD(&uhashtbl[nhash], un, un_cache);
+               un->un_cflags |= UN_CACHED;
+       }
+
+       mutex_exit(&uhash_lock);
 }
 
 /*
@@ -694,7 +692,6 @@
        if (error)
                return (error);
 
-       KASSERT(VOP_ISLOCKED(uvp) == LK_EXCLUSIVE);
        union_newupper(un, uvp);
 
        lvp = un->un_lowervp;
@@ -934,6 +931,7 @@
        }
 
        vp->v_writecount++;
+       VOP_UNLOCK(vp);
        *vpp = vp;
        return 0;
 }