Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/fs/union Change union_allocvp() to take an unlocked uppe...



details:   https://anonhg.NetBSD.org/src/rev/f82843e96651
branches:  trunk
changeset: 326750:f82843e96651
user:      hannken <hannken%NetBSD.org@localhost>
date:      Sun Feb 16 09:50:25 2014 +0000

description:
Change union_allocvp() to take an unlocked uppervp and to return the
union node unlocked.  Another VI_XLOCK hack is gone.

diffstat:

 sys/fs/union/union_subr.c   |  86 +++++++++++++-------------------------------
 sys/fs/union/union_vfsops.c |  12 +++--
 sys/fs/union/union_vnops.c  |  26 +++----------
 3 files changed, 39 insertions(+), 85 deletions(-)

diffs (truncated from 323 to 300 lines):

diff -r d23d6898d46a -r f82843e96651 sys/fs/union/union_subr.c
--- a/sys/fs/union/union_subr.c Sun Feb 16 08:18:28 2014 +0000
+++ b/sys/fs/union/union_subr.c Sun Feb 16 09:50:25 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_subr.c,v 1.62 2014/02/14 08:50:27 hannken Exp $  */
+/*     $NetBSD: union_subr.c,v 1.63 2014/02/16 09:50:25 hannken Exp $  */
 
 /*
  * Copyright (c) 1994
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.62 2014/02/14 08:50:27 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.63 2014/02/16 09:50:25 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -291,7 +291,7 @@
 
 /*
  * allocate a union_node/vnode pair.  the vnode is
- * referenced and locked.  the new vnode is returned
+ * referenced and unlocked.  the new vnode is returned
  * via (vpp).  (mp) is the mountpoint of the union filesystem,
  * (dvp) is the parent directory where the upper layer object
  * should exist (but doesn't) and (cnp) is the componentname
@@ -299,7 +299,7 @@
  * layer object to be created at a later time.  (uppervp)
  * and (lowervp) reference the upper and lower layer objects
  * being mapped.  either, but not both, can be nil.
- * if supplied, (uppervp) is locked.
+ * both, if supplied, are unlocked.
  * the reference is either maintained in the new union_node
  * object which is allocated, or they are vrele'd.
  *
@@ -339,11 +339,11 @@
        voff_t uppersz, lowersz;
        dev_t rdev;
        u_long hash[3];
-       int vflag, iflag, lflag;
+       int vflag, iflag;
        int try;
+       bool is_dotdot;
 
-       if (uppervp)
-               KASSERT(VOP_ISLOCKED(uppervp) == LK_EXCLUSIVE);
+       is_dotdot = (dvp != NULL && cnp != NULL && (cnp->cn_flags & ISDOTDOT));
 
        if (uppervp == NULLVP && lowervp == NULLVP)
                panic("union: unidentifiable allocation");
@@ -396,28 +396,10 @@
                            UNIONTOV(un)->v_mount != mp)
                                continue;
 
-                       if (uppervp != NULL &&
-                           (uppervp == dvp || uppervp == un->un_uppervp))
-                               /* "." or already locked. */
-                               lflag = 0;
-                       else
-                               lflag = LK_EXCLUSIVE;
                        vp = UNIONTOV(un);
                        mutex_enter(vp->v_interlock);
-                       /*
-                        * If this node being cleaned out and our caller
-                        * holds a lock, then ignore it and continue.  To
-                        * allow the cleaning to succeed the current thread
-                        * must make progress.  For a brief time the cache
-                        * may contain more than one vnode referring to
-                        * a lower node.
-                        */
-                       if ((vp->v_iflag & VI_XLOCK) != 0 && lflag == 0) {
-                               mutex_exit(vp->v_interlock);
-                               continue;
-                       }
                        mutex_exit(&uhash_lock);
-                       if (vget(vp, lflag))
+                       if (vget(vp, 0))
                                goto loop;
                        goto found;
                }
@@ -427,9 +409,13 @@
 
 found:
        if (un) {
-               KASSERT(VOP_ISLOCKED(UNIONTOV(un)) == LK_EXCLUSIVE);
-               KASSERT(uppervp == NULL ||
-                   VOP_ISLOCKED(uppervp) == LK_EXCLUSIVE);
+               if (uppervp != dvp) {
+                       if (is_dotdot)
+                               VOP_UNLOCK(dvp);
+                       vn_lock(UNIONTOV(un), LK_EXCLUSIVE | LK_RETRY);
+                       if (is_dotdot)
+                               vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
+               }
                /*
                 * Save information about the upper layer.
                 */
@@ -460,19 +446,23 @@
                        vrele(lowervp);
                }
                *vpp = UNIONTOV(un);
+               if (uppervp != dvp)
+                       VOP_UNLOCK(*vpp);
                return (0);
        }
 
        uppersz = lowersz = VNOVAL;
-       if (uppervp != NULLVP)
+       if (uppervp != NULLVP) {
+               vn_lock(uppervp, LK_SHARED | LK_RETRY);
                if (VOP_GETATTR(uppervp, &va, FSCRED) == 0)
                        uppersz = va.va_size;
+               VOP_UNLOCK(uppervp);
+       }
        if (lowervp != NULLVP) {
                vn_lock(lowervp, LK_SHARED | LK_RETRY);
-               error = VOP_GETATTR(lowervp, &va, FSCRED);
+               if (VOP_GETATTR(lowervp, &va, FSCRED) == 0)
+                       lowersz = va.va_size;
                VOP_UNLOCK(lowervp);
-               if (error == 0)
-                       lowersz = va.va_size;
        }
 
        /*
@@ -483,12 +473,8 @@
        error = getnewvnode(VT_UNION, mp, union_vnodeop_p,
            svp->v_interlock, vpp);
        if (error) {
-               if (uppervp) {
-                       if (dvp == uppervp)
-                               vrele(uppervp);
-                       else
-                               vput(uppervp);
-               }
+               if (uppervp)
+                       vrele(uppervp);
                if (lowervp)
                        vrele(lowervp);
 
@@ -501,17 +487,6 @@
                        if (un1->un_lowervp == lowervp &&
                            un1->un_uppervp == uppervp &&
                            UNIONTOV(un1)->v_mount == mp) {
-                               vp = UNIONTOV(un1);
-                               mutex_enter(vp->v_interlock);
-                               /*
-                                * Ignore nodes being cleaned out.
-                                * See the cache lookup above.
-                                */
-                               if ((vp->v_iflag & VI_XLOCK) != 0) {
-                                       mutex_exit(vp->v_interlock);
-                                       continue;
-                               }
-                               mutex_exit(vp->v_interlock);
                                /*
                                 * Another thread beat us, push back freshly
                                 * allocated vnode and retry.
@@ -552,15 +527,6 @@
        un->un_openl = 0;
        un->un_cflags = 0;
 
-       if (uppervp == NULL) {
-               struct vop_lock_args ap;
-
-               ap.a_vp = UNIONTOV(un);
-               ap.a_flags = LK_EXCLUSIVE;
-               error = genfs_lock(&ap);
-               KASSERT(error == 0);
-       }
-
        mutex_enter(&un->un_lock);
        un->un_uppersz = VNOVAL;
        un->un_lowersz = VNOVAL;
@@ -1076,10 +1042,10 @@
        if (*vpp == NULLVP)
                goto out;
 
-       vn_lock(*vpp, LK_EXCLUSIVE | LK_RETRY);
        vref(*vpp);
        error = union_allocvp(&nvp, vp->v_mount, NULLVP, NULLVP, 0, *vpp, NULLVP, 0);
        if (!error) {
+               vn_lock(nvp, LK_EXCLUSIVE | LK_RETRY);
                VTOUNION(vp)->un_dircache = 0;
                VTOUNION(nvp)->un_dircache = dircache;
        }
diff -r d23d6898d46a -r f82843e96651 sys/fs/union/union_vfsops.c
--- a/sys/fs/union/union_vfsops.c       Sun Feb 16 08:18:28 2014 +0000
+++ b/sys/fs/union/union_vfsops.c       Sun Feb 16 09:50:25 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_vfsops.c,v 1.68 2012/04/30 22:51:27 rmind Exp $  */
+/*     $NetBSD: union_vfsops.c,v 1.69 2014/02/16 09:50:25 hannken Exp $        */
 
 /*
  * Copyright (c) 1994 The Regents of the University of California.
@@ -77,7 +77,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_vfsops.c,v 1.68 2012/04/30 22:51:27 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_vfsops.c,v 1.69 2014/02/16 09:50:25 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -386,19 +386,21 @@
         * Return locked reference to root.
         */
        vref(um->um_uppervp);
-       vn_lock(um->um_uppervp, LK_EXCLUSIVE | LK_RETRY);
        if (um->um_lowervp)
                vref(um->um_lowervp);
        error = union_allocvp(vpp, mp, NULL, NULL, NULL,
                              um->um_uppervp, um->um_lowervp, 1);
 
        if (error) {
-               vput(um->um_uppervp);
+               vrele(um->um_uppervp);
                if (um->um_lowervp)
                        vrele(um->um_lowervp);
+               return error;
        }
 
-       return (error);
+       vn_lock(*vpp, LK_EXCLUSIVE | LK_RETRY);
+
+       return 0;
 }
 
 int
diff -r d23d6898d46a -r f82843e96651 sys/fs/union/union_vnops.c
--- a/sys/fs/union/union_vnops.c        Sun Feb 16 08:18:28 2014 +0000
+++ b/sys/fs/union/union_vnops.c        Sun Feb 16 09:50:25 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_vnops.c,v 1.55 2014/02/13 21:05:26 martin Exp $  */
+/*     $NetBSD: union_vnops.c,v 1.56 2014/02/16 09:50:25 hannken Exp $ */
 
 /*
  * Copyright (c) 1992, 1993, 1994, 1995
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.55 2014/02/13 21:05:26 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.56 2014/02/16 09:50:25 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -467,11 +467,6 @@
                                                vput(lowervp);
                                        goto start;
                                }
-                               /*
-                                * XXX: lock upper node until lookup returns
-                                * unlocked nodes.
-                                */
-                               vn_lock(uppervp, LK_EXCLUSIVE | LK_RETRY);
                        }
                        if (uerror) {
                                if (lowervp != NULLVP) {
@@ -481,6 +476,9 @@
                                return (uerror);
                        }
                }
+       } else { /* uerror == 0 */
+               if (uppervp != upperdvp)
+                       VOP_UNLOCK(uppervp);
        }
 
        if (lowervp != NULLVP)
@@ -491,15 +489,12 @@
 
        if (error) {
                if (uppervp != NULLVP)
-                       vput(uppervp);
+                       vrele(uppervp);
                if (lowervp != NULLVP)
                        vrele(lowervp);
                return error;
        }
 
-       if (*ap->a_vpp != dvp)
-               VOP_UNLOCK(*ap->a_vpp);
-
        return 0;
 }
 
@@ -526,11 +521,8 @@
                if (error)
                        return (error);
 
-               /* XXX: lock upper node until lookup returns unlocked nodes. */
-               vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
                error = union_allocvp(ap->a_vpp, mp, NULLVP, NULLVP, cnp, vp,
                                NULLVP, 1);
-               VOP_UNLOCK(vp);
                if (error)
                        vrele(vp);
                return (error);
@@ -579,11 +571,8 @@



Home | Main Index | Thread Index | Old Index