make sure open(..., O_TRUNC) works right.
[unix-history] / usr / src / sys / miscfs / union / union_vnops.c
index 469537e..1f55175 100644 (file)
@@ -8,14 +8,13 @@
  *
  * %sccs.include.redist.c%
  *
  *
  * %sccs.include.redist.c%
  *
- *     @(#)union_vnops.c       1.5 (Berkeley) %G%
+ *     @(#)union_vnops.c       8.3 (Berkeley) %G%
  */
 
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/proc.h>
 #include <sys/file.h>
  */
 
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/proc.h>
 #include <sys/file.h>
-#include <sys/filedesc.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/vnode.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/vnode.h>
 #include <sys/namei.h>
 #include <sys/malloc.h>
 #include <sys/buf.h>
 #include <sys/namei.h>
 #include <sys/malloc.h>
 #include <sys/buf.h>
-#include "union.h"
-
-/*
- * Create a shadow directory in the upper layer.
- * The new vnode is returned locked.
- */
-static int
-union_mkshadow(dvp, cnp, vpp)
-       struct vnode *dvp;
-       struct componentname *cnp;
-       struct vnode *vpp;
-{
-       int error;
-       struct vattr va;
-       struct proc *p = cnp->cn_proc;
-       struct componentname cn;
-
-       /*
-        * policy: when creating the shadow directory in the
-        * upper layer, create it owned by the current user,
-        * group from parent directory, and mode 777 modified
-        * by umask (ie mostly identical to the mkdir syscall).
-        * (jsp, kb)
-        * TODO: create the directory owned by the user who
-        * did the mount (um->um_cred).
-        */
-
-       /*
-        * A new componentname structure must be faked up because
-        * there is no way to know where the upper level cnp came
-        * from or what it is being used for.  This must duplicate
-        * some of the work done by NDINIT, some of the work done
-        * by namei, some of the work done by lookup and some of
-        * the work done by VOP_LOOKUP when given a CREATE flag.
-        * Conclusion: Horrible.
-        *
-        * The pathname buffer will be FREEed by VOP_MKDIR.
-        */
-       cn.cn_pnbuf = malloc(cnp->cn_namelen+1, M_NAMEI, M_WAITOK);
-       bcopy(cnp->cn_nameptr, cn.cn_pnbuf, cnp->cn_namelen+1);
-
-       cn.cn_nameiop = CREATE;
-       cn.cn_flags = HASBUF | SAVENAME | ISLASTCN;
-       cn.cn_proc = cnp->cn_proc;
-       cn.cn_cred = cnp->cn_cred;
-       cn.cn_nameptr = cn.cn_pnbuf;
-       cn.cn_namelen = cnp->cn_namelen;
-       cn.cn_hash = cnp->cn_hash;
-       cn.cn_consume = cnp->cn_consume;
-
-       VATTR_NULL(&va);
-       va.va_type = VDIR;
-       va.va_mode = UN_DIRMODE &~ p->p_fd->fd_cmask;
-
-       /* LEASE_CHECK: dvp is locked */
-       LEASE_CHECK(dvp, p, p->p_ucred, LEASE_WRITE);
-
-       VREF(dvp);
-       error = VOP_MKDIR(dvp, vpp, &cn, &va);
-       VOP_LOCK(dvp);
-       return (error);
-}
+#include <sys/queue.h>
+#include <miscfs/union/union.h>
 
 static int
 union_lookup1(udvp, dvp, vpp, cnp)
 
 static int
 union_lookup1(udvp, dvp, vpp, cnp)
@@ -105,8 +44,13 @@ union_lookup1(udvp, dvp, vpp, cnp)
         */
        if (cnp->cn_flags & ISDOTDOT) {
                for (;;) {
         */
        if (cnp->cn_flags & ISDOTDOT) {
                for (;;) {
-                       if ((dvp->v_flag & VROOT) == 0 ||
-                           (cnp->cn_flags & NOCROSSMOUNT))
+                       /*
+                        * Don't do the NOCROSSMOUNT check
+                        * at this level.  By definition,
+                        * union fs deals with namespaces, not
+                        * filesystems.
+                        */
+                       if ((dvp->v_flag & VROOT) == 0)
                                break;
 
                        tdvp = dvp;
                                break;
 
                        tdvp = dvp;
@@ -116,30 +60,28 @@ union_lookup1(udvp, dvp, vpp, cnp)
                        VOP_LOCK(dvp);
                }
        }
                        VOP_LOCK(dvp);
                }
        }
-       
+
         error = VOP_LOOKUP(dvp, &tdvp, cnp);
        if (error)
                return (error);
 
        /*
         error = VOP_LOOKUP(dvp, &tdvp, cnp);
        if (error)
                return (error);
 
        /*
-        * If going back up the directory tree, then the parent directory
-        * will have been unlocked, unless lookup found the last
-        * component.  In which case, re-lock the node here to allow
-        * it to be unlocked again (phew) in union_lookup.
+        * The parent directory will have been unlocked, unless lookup
+        * found the last component.  In which case, re-lock the node
+        * here to allow it to be unlocked again (phew) in union_lookup.
         */
         */
-       if ((cnp->cn_flags & ISDOTDOT) && !(cnp->cn_flags & ISLASTCN))
+       if (dvp != tdvp && !(cnp->cn_flags & ISLASTCN))
                VOP_LOCK(dvp);
 
        dvp = tdvp;
 
        /*
         * Lastly check if the current node is a mount point in
                VOP_LOCK(dvp);
 
        dvp = tdvp;
 
        /*
         * Lastly check if the current node is a mount point in
-        * which cse walk up the mount hierarchy making sure not to
+        * which case walk up the mount hierarchy making sure not to
         * bump into the root of the mount tree (ie. dvp != udvp).
         */
        while (dvp != udvp && (dvp->v_type == VDIR) &&
         * bump into the root of the mount tree (ie. dvp != udvp).
         */
        while (dvp != udvp && (dvp->v_type == VDIR) &&
-              (mp = dvp->v_mountedhere) &&
-              (cnp->cn_flags & NOCROSSMOUNT) == 0) {
+              (mp = dvp->v_mountedhere)) {
 
                if (mp->mnt_flag & MNT_MLOCK) {
                        mp->mnt_flag |= MNT_MWAIT;
 
                if (mp->mnt_flag & MNT_MLOCK) {
                        mp->mnt_flag |= MNT_MWAIT;
@@ -178,13 +120,14 @@ union_lookup(ap)
        struct componentname *cnp = ap->a_cnp;
        int lockparent = cnp->cn_flags & LOCKPARENT;
        int rdonly = cnp->cn_flags & RDONLY;
        struct componentname *cnp = ap->a_cnp;
        int lockparent = cnp->cn_flags & LOCKPARENT;
        int rdonly = cnp->cn_flags & RDONLY;
+       struct union_mount *um = MOUNTTOUNIONMOUNT(dvp->v_mount);
 
        cnp->cn_flags |= LOCKPARENT;
 
        upperdvp = dun->un_uppervp;
        lowerdvp = dun->un_lowervp;
 
        cnp->cn_flags |= LOCKPARENT;
 
        upperdvp = dun->un_uppervp;
        lowerdvp = dun->un_lowervp;
-       uppervp = 0;
-       lowervp = 0;
+       uppervp = NULLVP;
+       lowervp = NULLVP;
 
        /*
         * do the lookup in the upper level.
 
        /*
         * do the lookup in the upper level.
@@ -193,12 +136,10 @@ union_lookup(ap)
         * on and just return that vnode.
         */
        if (upperdvp) {
         * on and just return that vnode.
         */
        if (upperdvp) {
-               VOP_LOCK(upperdvp);
-               uerror = union_lookup1(
-                       MOUNTTOUNIONMOUNT(dvp->v_mount)->um_uppervp,
-                       upperdvp, &uppervp, cnp);
-               if (uppervp != upperdvp)
-                       VOP_UNLOCK(upperdvp);
+               uerror = union_lookup1(um->um_uppervp, upperdvp,
+                                       &uppervp, cnp);
+               /*if (uppervp == upperdvp)
+                       dun->un_flags |= UN_KLOCK;*/
 
                if (cnp->cn_consume != 0) {
                        *ap->a_vpp = uppervp;
 
                if (cnp->cn_consume != 0) {
                        *ap->a_vpp = uppervp;
@@ -218,17 +159,30 @@ union_lookup(ap)
         * instead.
         */
        if (lowerdvp) {
         * instead.
         */
        if (lowerdvp) {
+               int nameiop;
+
                VOP_LOCK(lowerdvp);
                VOP_LOCK(lowerdvp);
-               lerror = union_lookup1(
-                       MOUNTTOUNIONMOUNT(dvp->v_mount)->um_lowervp,
-                       lowerdvp, &lowervp, cnp);
+
+               /*
+                * Only do a LOOKUP on the bottom node, since
+                * we won't be making changes to it anyway.
+                */
+               nameiop = cnp->cn_nameiop;
+               cnp->cn_nameiop = LOOKUP;
+               lerror = union_lookup1(um->um_lowervp, lowerdvp,
+                               &lowervp, cnp);
+               cnp->cn_nameiop = nameiop;
+
                if (lowervp != lowerdvp)
                        VOP_UNLOCK(lowerdvp);
 
                if (cnp->cn_consume != 0) {
                        if (uppervp) {
                if (lowervp != lowerdvp)
                        VOP_UNLOCK(lowerdvp);
 
                if (cnp->cn_consume != 0) {
                        if (uppervp) {
-                               vput(uppervp);
-                               uppervp = 0;
+                               if (uppervp == upperdvp)
+                                       vrele(uppervp);
+                               else
+                                       vput(uppervp);
+                               uppervp = NULLVP;
                        }
                        *ap->a_vpp = lowervp;
                        if (!lockparent)
                        }
                        *ap->a_vpp = lowervp;
                        if (!lockparent)
@@ -266,40 +220,41 @@ union_lookup(ap)
         *    whatever the bottom layer returned.
         */
 
         *    whatever the bottom layer returned.
         */
 
+       *ap->a_vpp = NULLVP;
+
        /* case 1. */
        if ((uerror != 0) && (lerror != 0)) {
        /* case 1. */
        if ((uerror != 0) && (lerror != 0)) {
-               *ap->a_vpp = 0;
                return (uerror);
        }
 
        /* case 2. */
        if (uerror != 0 /* && (lerror == 0) */ ) {
                if (lowervp->v_type == VDIR) { /* case 2b. */
                return (uerror);
        }
 
        /* case 2. */
        if (uerror != 0 /* && (lerror == 0) */ ) {
                if (lowervp->v_type == VDIR) { /* case 2b. */
+                       dun->un_flags &= ~UN_ULOCK;
+                       VOP_UNLOCK(upperdvp);
+                       uerror = union_mkshadow(um, upperdvp, cnp, &uppervp);
                        VOP_LOCK(upperdvp);
                        VOP_LOCK(upperdvp);
-                       uerror = union_mkshadow(upperdvp, cnp, &uppervp);
-                       if (uppervp != upperdvp)
-                               VOP_UNLOCK(upperdvp);
+                       dun->un_flags |= UN_ULOCK;
+
                        if (uerror) {
                                if (lowervp) {
                                        vput(lowervp);
                        if (uerror) {
                                if (lowervp) {
                                        vput(lowervp);
-                                       lowervp = 0;
+                                       lowervp = NULLVP;
                                }
                                return (uerror);
                        }
                }
        }
 
                                }
                                return (uerror);
                        }
                }
        }
 
-       error = union_allocvp(ap->a_vpp, dvp->v_mount, dvp, upperdvp, cnp,
-                             uppervp, lowervp);
-
-       if (uppervp)
-               VOP_UNLOCK(uppervp);
        if (lowervp)
                VOP_UNLOCK(lowervp);
 
        if (lowervp)
                VOP_UNLOCK(lowervp);
 
+       error = union_allocvp(ap->a_vpp, dvp->v_mount, dvp, upperdvp, cnp,
+                             uppervp, lowervp);
+
        if (error) {
                if (uppervp)
        if (error) {
                if (uppervp)
-                       vrele(uppervp);
+                       vput(uppervp);
                if (lowervp)
                        vrele(lowervp);
        } else {
                if (lowervp)
                        vrele(lowervp);
        } else {
@@ -328,7 +283,7 @@ union_create(ap)
                struct vnode *vp;
 
                VREF(dvp);
                struct vnode *vp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                error = VOP_CREATE(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
                vput(ap->a_dvp);
                error = VOP_CREATE(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
@@ -342,9 +297,8 @@ union_create(ap)
                                ap->a_cnp,
                                vp,
                                NULLVP);
                                ap->a_cnp,
                                vp,
                                NULLVP);
-               VOP_UNLOCK(vp);
                if (error)
                if (error)
-                       vrele(vp);
+                       vput(vp);
                return (error);
        }
 
                return (error);
        }
 
@@ -369,7 +323,7 @@ union_mknod(ap)
                struct vnode *vp;
 
                VREF(dvp);
                struct vnode *vp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                error = VOP_MKNOD(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
                vput(ap->a_dvp);
                error = VOP_MKNOD(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
@@ -384,9 +338,8 @@ union_mknod(ap)
                                        ap->a_cnp,
                                        vp,
                                        NULLVP);
                                        ap->a_cnp,
                                        vp,
                                        NULLVP);
-                       VOP_UNLOCK(vp);
                        if (error)
                        if (error)
-                               vrele(vp);
+                               vput(vp);
                }
                return (error);
        }
                }
                return (error);
        }
@@ -424,11 +377,8 @@ union_open(ap)
                 */
                tvp = un->un_lowervp;
                if ((ap->a_mode & FWRITE) && (tvp->v_type == VREG)) {
                 */
                tvp = un->un_lowervp;
                if ((ap->a_mode & FWRITE) && (tvp->v_type == VREG)) {
-                       struct nameidata nd;
-                       struct filedesc *fdp = p->p_fd;
                        struct vnode *vp;
                        struct vnode *vp;
-                       /*int fmode;*/
-                       int cmode;
+                       int i;
 
                        /*
                         * Open the named file in the upper layer.  Note that
 
                        /*
                         * Open the named file in the upper layer.  Note that
@@ -437,19 +387,16 @@ union_open(ap)
                         * be a loopback mount of some other filesystem...
                         * so open the file with exclusive create and barf if
                         * it already exists.
                         * be a loopback mount of some other filesystem...
                         * so open the file with exclusive create and barf if
                         * it already exists.
-                        * XXX - perhaps shoudl re-lookup the node (once more
+                        * XXX - perhaps should re-lookup the node (once more
                         * with feeling) and simply open that.  Who knows.
                         */
                         * with feeling) and simply open that.  Who knows.
                         */
-                       /*
-                       NDINIT(&nd, CREATE, 0, UIO_SYSSPACE, un->un_path, p);
-                       fmode = (O_CREAT|O_TRUNC|O_EXCL);
-                       */
-                       cmode = UN_FILEMODE & ~fdp->fd_cmask;
-                       error = union_vn_create(&vp, un, cmode, p);
+                       error = union_vn_create(&vp, un, p);
                        if (error)
                                return (error);
                        if (error)
                                return (error);
-                       un->un_uppervp = vp;    /* XXX */
+
                        /* at this point, uppervp is locked */
                        /* at this point, uppervp is locked */
+                       union_newupper(un, vp);
+                       un->un_flags |= UN_ULOCK;
 
                        /*
                         * Now, if the file is being opened with truncation,
 
                        /*
                         * Now, if the file is being opened with truncation,
@@ -473,20 +420,49 @@ union_open(ap)
                                } else {
                                        VOP_UNLOCK(tvp);
                                }
                                } else {
                                        VOP_UNLOCK(tvp);
                                }
-                               VOP_UNLOCK(un->un_uppervp);
-                               (void) VOP_CLOSE(un->un_uppervp, FWRITE);
-                               VOP_LOCK(un->un_uppervp);
+
+                               if (!error)
+                                       uprintf("union: copied up %s\n",
+                                                               un->un_path);
+                       }
+
+                       un->un_flags &= ~UN_ULOCK;
+                       VOP_UNLOCK(un->un_uppervp);
+                       union_vn_close(un->un_uppervp, FWRITE, cred, p);
+                       VOP_LOCK(un->un_uppervp);
+                       un->un_flags |= UN_ULOCK;
+
+                       /*
+                        * Subsequent IOs will go to the top layer, so
+                        * call close on the lower vnode and open on the
+                        * upper vnode to ensure that the filesystem keeps
+                        * its references counts right.  This doesn't do
+                        * the right thing with (cred) and (FREAD) though.
+                        * Ignoring error returns is not righ, either.
+                        */
+                       for (i = 0; i < un->un_openl; i++) {
+                               (void) VOP_CLOSE(tvp, FREAD);
+                               (void) VOP_OPEN(un->un_uppervp, FREAD, cred, p);
                        }
                        }
+                       un->un_openl = 0;
+
                        if (error == 0)
                                error = VOP_OPEN(un->un_uppervp, mode, cred, p);
                        if (error == 0)
                                error = VOP_OPEN(un->un_uppervp, mode, cred, p);
-                       VOP_UNLOCK(un->un_uppervp);
                        return (error);
                }
                        return (error);
                }
+
+               /*
+                * Just open the lower vnode
+                */
+               un->un_openl++;
+               VOP_LOCK(tvp);
+               error = VOP_OPEN(tvp, mode, cred, p);
+               VOP_UNLOCK(tvp);
+
+               return (error);
        }
 
        }
 
-       VOP_LOCK(tvp);
        error = VOP_OPEN(tvp, mode, cred, p);
        error = VOP_OPEN(tvp, mode, cred, p);
-       VOP_UNLOCK(tvp);
 
        return (error);
 }
 
        return (error);
 }
@@ -500,8 +476,21 @@ union_close(ap)
                struct proc *a_p;
        } */ *ap;
 {
                struct proc *a_p;
        } */ *ap;
 {
+       struct union_node *un = VTOUNION(ap->a_vp);
+       struct vnode *vp;
+
+       if (un->un_uppervp) {
+               vp = un->un_uppervp;
+       } else {
+#ifdef UNION_DIAGNOSTIC
+               if (un->un_openl <= 0)
+                       panic("union: un_openl cnt");
+#endif
+               --un->un_openl;
+               vp = un->un_lowervp;
+       }
 
 
-       return (VOP_CLOSE(OTHERVP(ap->a_vp), ap->a_fflag, ap->a_cred, ap->a_p));
+       return (VOP_CLOSE(vp, ap->a_fflag, ap->a_cred, ap->a_p));
 }
 
 /*
 }
 
 /*
@@ -534,11 +523,8 @@ union_access(ap)
                        return (error);
        }
 
                        return (error);
        }
 
-       if (vp = un->un_uppervp) {
-               VOP_LOCK(vp);
+       if (vp = un->un_uppervp)
                error = VOP_ACCESS(vp, ap->a_mode, ap->a_cred, ap->a_p);
                error = VOP_ACCESS(vp, ap->a_mode, ap->a_cred, ap->a_p);
-               VOP_UNLOCK(vp);
-       }
 
        return (error);
 }
 
        return (error);
 }
@@ -557,10 +543,13 @@ union_getattr(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_GETATTR(vp, ap->a_vap, ap->a_cred, ap->a_p);
        error = VOP_GETATTR(vp, ap->a_vap, ap->a_cred, ap->a_p);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        /* Requires that arguments be restored. */
        ap->a_vap->va_fsid = ap->a_vp->v_mount->mnt_stat.f_fsid.val[0];
 
        /* Requires that arguments be restored. */
        ap->a_vap->va_fsid = ap->a_vp->v_mount->mnt_stat.f_fsid.val[0];
@@ -579,17 +568,38 @@ union_setattr(ap)
        struct union_node *un = VTOUNION(ap->a_vp);
        int error;
 
        struct union_node *un = VTOUNION(ap->a_vp);
        int error;
 
-       if (un->un_uppervp) {
-               VOP_LOCK(un->un_uppervp);
+       /*
+        * Handle case of truncating lower object to zero size,
+        * by creating a zero length upper object.  This is to
+        * handle the case of open with O_TRUNC and O_CREAT.
+        */
+       if ((un->un_uppervp == NULLVP) &&
+           /* assert(un->un_lowervp != NULLVP) */
+           (un->un_lowervp->v_type == VREG) &&
+           (ap->a_vap->va_size == 0)) {
+               struct vnode *vp;
+
+               error = union_vn_create(&vp, un, ap->a_p);
+               if (error)
+                       return (error);
+
+               /* at this point, uppervp is locked */
+               union_newupper(un, vp);
+
+               VOP_UNLOCK(vp);
+               union_vn_close(un->un_uppervp, FWRITE, ap->a_cred, ap->a_p);
+               VOP_LOCK(vp);
+               un->un_flags |= UN_ULOCK;
+       }
+
+       /*
+        * Try to set attributes in upper layer,
+        * otherwise return read-only filesystem error.
+        */
+       if (un->un_uppervp != NULLVP) {
                error = VOP_SETATTR(un->un_uppervp, ap->a_vap,
                                        ap->a_cred, ap->a_p);
                error = VOP_SETATTR(un->un_uppervp, ap->a_vap,
                                        ap->a_cred, ap->a_p);
-               VOP_UNLOCK(un->un_uppervp);
        } else {
        } else {
-               /*
-                * XXX should do a copyfile (perhaps only if
-                * the file permission change, which would not
-                * track va_ctime correctly).
-                */
                error = EROFS;
        }
 
                error = EROFS;
        }
 
@@ -607,10 +617,13 @@ union_read(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_READ(vp, ap->a_uio, ap->a_ioflag, ap->a_cred);
        error = VOP_READ(vp, ap->a_uio, ap->a_ioflag, ap->a_cred);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        return (error);
 }
 
        return (error);
 }
@@ -626,10 +639,13 @@ union_write(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_WRITE(vp, ap->a_uio, ap->a_ioflag, ap->a_cred);
        error = VOP_WRITE(vp, ap->a_uio, ap->a_ioflag, ap->a_cred);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        return (error);
 }
 
        return (error);
 }
@@ -692,10 +708,14 @@ union_fsync(ap)
        struct vnode *targetvp = OTHERVP(ap->a_vp);
 
        if (targetvp) {
        struct vnode *targetvp = OTHERVP(ap->a_vp);
 
        if (targetvp) {
-               VOP_LOCK(targetvp);
+               int dolock = (targetvp == LOWERVP(ap->a_vp));
+
+               if (dolock)
+                       VOP_LOCK(targetvp);
                error = VOP_FSYNC(targetvp, ap->a_cred,
                                        ap->a_waitfor, ap->a_p);
                error = VOP_FSYNC(targetvp, ap->a_cred,
                                        ap->a_waitfor, ap->a_p);
-               VOP_UNLOCK(targetvp);
+               if (dolock)
+                       VOP_UNLOCK(targetvp);
        }
 
        return (error);
        }
 
        return (error);
@@ -731,13 +751,19 @@ union_remove(ap)
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               dun->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                VREF(vp);
                vput(ap->a_dvp);
                VREF(vp);
-               VOP_LOCK(vp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_vp);
 
                error = VOP_REMOVE(dvp, vp, ap->a_cnp);
                vput(ap->a_vp);
 
                error = VOP_REMOVE(dvp, vp, ap->a_cnp);
+               if (!error)
+                       union_removed_upper(un);
+
+               /*
+                * XXX: should create a whiteout here
+                */
        } else {
                /*
                 * XXX: should create a whiteout here
        } else {
                /*
                 * XXX: should create a whiteout here
@@ -767,7 +793,7 @@ union_link(ap)
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               dun->un_flags |= UN_KLOCK;
                vput(ap->a_vp);
                VREF(vp);
                vrele(ap->a_tdvp);
                vput(ap->a_vp);
                VREF(vp);
                vrele(ap->a_tdvp);
@@ -806,7 +832,7 @@ union_rename(ap)
 
        if (fdvp->v_op == union_vnodeop_p) {    /* always true */
                struct union_node *un = VTOUNION(fdvp);
 
        if (fdvp->v_op == union_vnodeop_p) {    /* always true */
                struct union_node *un = VTOUNION(fdvp);
-               if (un->un_uppervp == 0) {
+               if (un->un_uppervp == NULLVP) {
                        error = EROFS;
                        goto bad;
                }
                        error = EROFS;
                        goto bad;
                }
@@ -818,7 +844,7 @@ union_rename(ap)
 
        if (fvp->v_op == union_vnodeop_p) {     /* always true */
                struct union_node *un = VTOUNION(fvp);
 
        if (fvp->v_op == union_vnodeop_p) {     /* always true */
                struct union_node *un = VTOUNION(fvp);
-               if (un->un_uppervp == 0) {
+               if (un->un_uppervp == NULLVP) {
                        error = EROFS;
                        goto bad;
                }
                        error = EROFS;
                        goto bad;
                }
@@ -830,27 +856,27 @@ union_rename(ap)
 
        if (tdvp->v_op == union_vnodeop_p) {
                struct union_node *un = VTOUNION(tdvp);
 
        if (tdvp->v_op == union_vnodeop_p) {
                struct union_node *un = VTOUNION(tdvp);
-               if (un->un_uppervp == 0) {
+               if (un->un_uppervp == NULLVP) {
                        error = EROFS;
                        goto bad;
                }
 
                tdvp = un->un_uppervp;
                VREF(tdvp);
                        error = EROFS;
                        goto bad;
                }
 
                tdvp = un->un_uppervp;
                VREF(tdvp);
-               VOP_LOCK(tdvp);
-               vput(ap->a_fdvp);
+               un->un_flags |= UN_KLOCK;
+               vput(ap->a_tdvp);
        }
 
        if (tvp && tvp->v_op == union_vnodeop_p) {
                struct union_node *un = VTOUNION(tvp);
        }
 
        if (tvp && tvp->v_op == union_vnodeop_p) {
                struct union_node *un = VTOUNION(tvp);
-               if (un->un_uppervp == 0) {
+               if (un->un_uppervp == NULLVP) {
                        error = EROFS;
                        goto bad;
                }
 
                tvp = un->un_uppervp;
                VREF(tvp);
                        error = EROFS;
                        goto bad;
                }
 
                tvp = un->un_uppervp;
                VREF(tvp);
-               VOP_LOCK(tvp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_tvp);
        }
 
                vput(ap->a_tvp);
        }
 
@@ -883,7 +909,7 @@ union_mkdir(ap)
                struct vnode *vp;
 
                VREF(dvp);
                struct vnode *vp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                error = VOP_MKDIR(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
                vput(ap->a_dvp);
                error = VOP_MKDIR(dvp, &vp, ap->a_cnp, ap->a_vap);
                if (error)
@@ -897,9 +923,8 @@ union_mkdir(ap)
                                ap->a_cnp,
                                vp,
                                NULLVP);
                                ap->a_cnp,
                                vp,
                                NULLVP);
-               VOP_UNLOCK(vp);
                if (error)
                if (error)
-                       vrele(vp);
+                       vput(vp);
                return (error);
        }
 
                return (error);
        }
 
@@ -924,13 +949,19 @@ union_rmdir(ap)
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
                struct vnode *vp = un->un_uppervp;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               dun->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                VREF(vp);
                vput(ap->a_dvp);
                VREF(vp);
-               VOP_LOCK(vp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_vp);
 
                vput(ap->a_vp);
 
-               error = VOP_REMOVE(dvp, vp, ap->a_cnp);
+               error = VOP_RMDIR(dvp, vp, ap->a_cnp);
+               if (!error)
+                       union_removed_upper(un);
+
+               /*
+                * XXX: should create a whiteout here
+                */
        } else {
                /*
                 * XXX: should create a whiteout here
        } else {
                /*
                 * XXX: should create a whiteout here
@@ -962,11 +993,11 @@ union_symlink(ap)
                struct mount *mp = ap->a_dvp->v_mount;
 
                VREF(dvp);
                struct mount *mp = ap->a_dvp->v_mount;
 
                VREF(dvp);
-               VOP_LOCK(dvp);
+               un->un_flags |= UN_KLOCK;
                vput(ap->a_dvp);
                error = VOP_SYMLINK(dvp, &vp, ap->a_cnp,
                                        ap->a_vap, ap->a_target);
                vput(ap->a_dvp);
                error = VOP_SYMLINK(dvp, &vp, ap->a_cnp,
                                        ap->a_vap, ap->a_target);
-               *ap->a_vpp = 0;
+               *ap->a_vpp = NULLVP;
                return (error);
        }
 
                return (error);
        }
 
@@ -993,13 +1024,8 @@ union_readdir(ap)
        int error = 0;
        struct union_node *un = VTOUNION(ap->a_vp);
 
        int error = 0;
        struct union_node *un = VTOUNION(ap->a_vp);
 
-       if (un->un_uppervp) {
-               struct vnode *vp = OTHERVP(ap->a_vp);
-
-               VOP_LOCK(vp);
-               error = VOP_READLINK(vp, ap->a_uio, ap->a_cred);
-               VOP_UNLOCK(vp);
-       }
+       if (un->un_uppervp)
+               error = VOP_READDIR(un->un_uppervp, ap->a_uio, ap->a_cred);
 
        return (error);
 }
 
        return (error);
 }
@@ -1014,10 +1040,13 @@ union_readlink(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_READLINK(vp, ap->a_uio, ap->a_cred);
        error = VOP_READLINK(vp, ap->a_uio, ap->a_cred);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        return (error);
 }
 
        return (error);
 }
@@ -1033,11 +1062,12 @@ union_abortop(ap)
        struct vnode *vp = OTHERVP(ap->a_dvp);
        struct union_node *un = VTOUNION(ap->a_dvp);
        int islocked = un->un_flags & UN_LOCKED;
        struct vnode *vp = OTHERVP(ap->a_dvp);
        struct union_node *un = VTOUNION(ap->a_dvp);
        int islocked = un->un_flags & UN_LOCKED;
+       int dolock = (vp == LOWERVP(ap->a_dvp));
 
 
-       if (islocked)
+       if (islocked && dolock)
                VOP_LOCK(vp);
        error = VOP_ABORTOP(vp, ap->a_cnp);
                VOP_LOCK(vp);
        error = VOP_ABORTOP(vp, ap->a_cnp);
-       if (islocked)
+       if (islocked && dolock)
                VOP_UNLOCK(vp);
 
        return (error);
                VOP_UNLOCK(vp);
 
        return (error);
@@ -1063,7 +1093,7 @@ union_inactive(ap)
         * That's too much work for now.
         */
 
         * That's too much work for now.
         */
 
-#ifdef DIAGNOSTIC
+#ifdef UNION_DIAGNOSTIC
        struct union_node *un = VTOUNION(ap->a_vp);
 
        if (un->un_flags & UN_LOCKED)
        struct union_node *un = VTOUNION(ap->a_vp);
 
        if (un->un_flags & UN_LOCKED)
@@ -1079,31 +1109,9 @@ union_reclaim(ap)
                struct vnode *a_vp;
        } */ *ap;
 {
                struct vnode *a_vp;
        } */ *ap;
 {
-       struct vnode *vp = ap->a_vp;
-       struct union_node *un = VTOUNION(vp);
-       struct vnode *uppervp = un->un_uppervp;
-       struct vnode *lowervp = un->un_lowervp;
-       struct vnode *dirvp = un->un_dirvp;
-       char *path = un->un_path;
 
 
-       /*
-        * Note: in vop_reclaim, vp->v_op == dead_vnodeop_p,
-        * so we can't call VOPs on ourself.
-        */
-       /* After this assignment, this node will not be re-used. */
-       un->un_uppervp = 0;
-       un->un_lowervp = 0;
-       un->un_dirvp = 0;
-       un->un_path = NULL;
-       union_freevp(vp);
-       if (uppervp)
-               vrele(uppervp);
-       if (lowervp)
-               vrele(lowervp);
-       if (dirvp)
-               vrele(dirvp);
-       if (path)
-               free(path, M_TEMP);
+       union_freevp(ap->a_vp);
+
        return (0);
 }
 
        return (0);
 }
 
@@ -1113,6 +1121,17 @@ union_lock(ap)
 {
        struct union_node *un = VTOUNION(ap->a_vp);
 
 {
        struct union_node *un = VTOUNION(ap->a_vp);
 
+       if (un->un_uppervp) {
+               if ((un->un_flags & UN_ULOCK) == 0) {
+                       VOP_LOCK(un->un_uppervp);
+                       un->un_flags |= UN_ULOCK;
+               }
+#ifdef DIAGNOSTIC
+               if (un->un_flags & UN_KLOCK)
+                       panic("union: dangling upper lock");
+#endif
+       }
+
        while (un->un_flags & UN_LOCKED) {
 #ifdef DIAGNOSTIC
                if (curproc && un->un_pid == curproc->p_pid &&
        while (un->un_flags & UN_LOCKED) {
 #ifdef DIAGNOSTIC
                if (curproc && un->un_pid == curproc->p_pid &&
@@ -1130,6 +1149,8 @@ union_lock(ap)
        else
                un->un_pid = -1;
 #endif
        else
                un->un_pid = -1;
 #endif
+
+       return (0);
 }
 
 int
 }
 
 int
@@ -1147,6 +1168,12 @@ union_unlock(ap)
 #endif
 
        un->un_flags &= ~UN_LOCKED;
 #endif
 
        un->un_flags &= ~UN_LOCKED;
+
+       if ((un->un_flags & (UN_ULOCK|UN_KLOCK)) == UN_ULOCK)
+               VOP_UNLOCK(un->un_uppervp);
+
+       un->un_flags &= ~(UN_ULOCK|UN_KLOCK);
+
        if (un->un_flags & UN_WANT) {
                un->un_flags &= ~UN_WANT;
                wakeup((caddr_t) &un->un_flags);
        if (un->un_flags & UN_WANT) {
                un->un_flags &= ~UN_WANT;
                wakeup((caddr_t) &un->un_flags);
@@ -1155,6 +1182,8 @@ union_unlock(ap)
 #ifdef DIAGNOSTIC
        un->un_pid = 0;
 #endif
 #ifdef DIAGNOSTIC
        un->un_pid = 0;
 #endif
+
+       return (0);
 }
 
 int
 }
 
 int
@@ -1169,10 +1198,13 @@ union_bmap(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_BMAP(vp, ap->a_bn, ap->a_vpp, ap->a_bnp, ap->a_runp);
        error = VOP_BMAP(vp, ap->a_bn, ap->a_vpp, ap->a_bnp, ap->a_runp);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        return (error);
 }
 
        return (error);
 }
@@ -1210,10 +1242,13 @@ union_pathconf(ap)
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
 {
        int error;
        struct vnode *vp = OTHERVP(ap->a_vp);
+       int dolock = (vp == LOWERVP(ap->a_vp));
 
 
-       VOP_LOCK(vp);
+       if (dolock)
+               VOP_LOCK(vp);
        error = VOP_PATHCONF(vp, ap->a_name, ap->a_retval);
        error = VOP_PATHCONF(vp, ap->a_name, ap->a_retval);
-       VOP_UNLOCK(vp);
+       if (dolock)
+               VOP_UNLOCK(vp);
 
        return (error);
 }
 
        return (error);
 }
@@ -1253,7 +1288,7 @@ union_strategy(ap)
        bp->b_vp = OTHERVP(bp->b_vp);
 
 #ifdef DIAGNOSTIC
        bp->b_vp = OTHERVP(bp->b_vp);
 
 #ifdef DIAGNOSTIC
-       if (bp->b_vp == 0)
+       if (bp->b_vp == NULLVP)
                panic("union_strategy: nil vp");
        if (((bp->b_flags & B_READ) == 0) &&
            (bp->b_vp == LOWERVP(savedvp)))
                panic("union_strategy: nil vp");
        if (((bp->b_flags & B_READ) == 0) &&
            (bp->b_vp == LOWERVP(savedvp)))