From e2a2f3a732c54c31e87edb0d0a789e899c13ad24 Mon Sep 17 00:00:00 2001 From: Jan-Simon Pendry Date: Thu, 10 Feb 1994 22:08:26 -0800 Subject: [PATCH] new locking scheme SCCS-vsn: sys/miscfs/union/union.h 2.1 SCCS-vsn: sys/miscfs/union/union_subr.c 2.1 SCCS-vsn: sys/miscfs/union/union_vnops.c 2.1 SCCS-vsn: sys/miscfs/union/union_vfsops.c 2.1 --- usr/src/sys/miscfs/union/union.h | 15 +- usr/src/sys/miscfs/union/union_subr.c | 186 +++++++++++++++++------- usr/src/sys/miscfs/union/union_vfsops.c | 3 +- usr/src/sys/miscfs/union/union_vnops.c | 177 ++++++++++++++-------- 4 files changed, 261 insertions(+), 120 deletions(-) diff --git a/usr/src/sys/miscfs/union/union.h b/usr/src/sys/miscfs/union/union.h index 9657ba9987..8041fb77a2 100644 --- a/usr/src/sys/miscfs/union/union.h +++ b/usr/src/sys/miscfs/union/union.h @@ -8,7 +8,7 @@ * * %sccs.include.redist.c% * - * @(#)union.h 1.9 (Berkeley) %G% + * @(#)union.h 2.1 (Berkeley) %G% */ struct union_args { @@ -56,8 +56,17 @@ struct union_node { #endif }; -#define UN_WANT 0x01 -#define UN_LOCKED 0x02 +#define UN_WANT 0x01 +#define UN_LOCKED 0x02 +#define UN_ULOCK 0x04 /* Upper node is locked */ +#define UN_KLOCK 0x08 /* Keep upper node locked on vput */ + +#define LOCKUVP(un) \ + (((un)->un_flags & UN_ULOCK) ? \ + (0) : \ + (((un)->un_flags |= UN_ULOCK), VOP_LOCK((un)->un_uppervp))) +#define UNLOCKUVP(un) \ + ((un)->un_flags &= ~UN_ULOCK) extern int union_allocvp __P((struct vnode **, struct mount *, struct vnode *, struct vnode *, diff --git a/usr/src/sys/miscfs/union/union_subr.c b/usr/src/sys/miscfs/union/union_subr.c index 5e13601db1..3319a34853 100644 --- a/usr/src/sys/miscfs/union/union_subr.c +++ b/usr/src/sys/miscfs/union/union_subr.c @@ -8,7 +8,7 @@ * * %sccs.include.redist.c% * - * @(#)union_subr.c 1.9 (Berkeley) %G% + * @(#)union_subr.c 2.1 (Berkeley) %G% */ #include @@ -37,6 +37,20 @@ union_init() unvplock = 0; } +static void +union_remlist(un) + struct union_node *un; +{ + struct union_node **unpp; + + for (unpp = &unhead; *unpp != 0; unpp = &(*unpp)->un_next) { + if (*unpp == un) { + *unpp = un->un_next; + break; + } + } +} + /* * allocate a union_node/vnode pair. the vnode is * referenced and locked. the new vnode is returned @@ -47,6 +61,7 @@ union_init() * 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. * the reference is either maintained in the new union_node * object which is allocated, or they are vrele'd. * @@ -99,50 +114,102 @@ loop: (UNIONTOV(un)->v_mount == mp)) { if (vget(UNIONTOV(un), 0)) goto loop; - if (UNIONTOV(un) != undvp) - VOP_LOCK(UNIONTOV(un)); + break; + } + } + if (un) { + /* + * Obtain a lock on the union_node. + * uppervp is locked, though un->un_uppervp + * may not be. this doesn't break the locking + * hierarchy since in the case that un->un_uppervp + * is not yet locked it will be vrele'd and replaced + * with uppervp. + */ + + if ((dvp != NULLVP) && (uppervp == dvp)) { /* - * Save information about the upper layer. + * Access ``.'', so (un) will already + * be locked. Since this process has + * the lock on (uppervp) no other + * process can hold the lock on (un). */ - if (uppervp != un->un_uppervp) { - if (un->un_uppervp) - vrele(un->un_uppervp); - un->un_uppervp = uppervp; - } else if (uppervp) { - vrele(uppervp); +#ifdef DIAGNOSTIC + if ((un->un_flags & UN_LOCKED) == 0) + panic("union: . not locked"); + else if (curproc && un->un_pid != curproc->p_pid && + un->un_pid > -1 && curproc->p_pid > -1) + panic("union: allocvp not lock owner"); +#endif + } else { + if (un->un_flags & UN_LOCKED) { + vrele(UNIONTOV(un)); + un->un_flags |= UN_WANT; + sleep((caddr_t) &un->un_flags, PINOD); + goto loop; } + un->un_flags |= UN_LOCKED; - /* - * Save information about the lower layer. - * This needs to keep track of pathname - * and directory information which union_vn_create - * might need. - */ - if (lowervp != un->un_lowervp) { - if (un->un_lowervp) { - vrele(un->un_lowervp); - free(un->un_path, M_TEMP); - vrele(un->un_dirvp); - } - un->un_lowervp = lowervp; - if (cnp && (lowervp != NULLVP) && - (lowervp->v_type == VREG)) { - un->un_hash = cnp->cn_hash; - un->un_path = malloc(cnp->cn_namelen+1, - M_TEMP, M_WAITOK); - bcopy(cnp->cn_nameptr, un->un_path, - cnp->cn_namelen); - un->un_path[cnp->cn_namelen] = '\0'; - VREF(dvp); - un->un_dirvp = dvp; - } - } else if (lowervp) { - vrele(lowervp); +#ifdef DIAGNOSTIC + if (curproc) + un->un_pid = curproc->p_pid; + else + un->un_pid = -1; +#endif + } + + /* + * At this point, the union_node is locked, + * un->un_uppervp may not be locked, and uppervp + * is locked or nil. + */ + + /* + * Save information about the upper layer. + */ + if (uppervp != un->un_uppervp) { + if (un->un_uppervp) + vrele(un->un_uppervp); + un->un_uppervp = uppervp; + } else if (uppervp) { + vrele(uppervp); + } + + if (un->un_uppervp) { + un->un_flags |= UN_ULOCK; + un->un_flags &= ~UN_KLOCK; + } + + /* + * Save information about the lower layer. + * This needs to keep track of pathname + * and directory information which union_vn_create + * might need. + */ + if (lowervp != un->un_lowervp) { + if (un->un_lowervp) { + vrele(un->un_lowervp); + free(un->un_path, M_TEMP); + vrele(un->un_dirvp); + } + un->un_lowervp = lowervp; + if (cnp && (lowervp != NULLVP) && + (lowervp->v_type == VREG)) { + un->un_hash = cnp->cn_hash; + un->un_path = malloc(cnp->cn_namelen+1, + M_TEMP, M_WAITOK); + bcopy(cnp->cn_nameptr, un->un_path, + cnp->cn_namelen); + un->un_path[cnp->cn_namelen] = '\0'; + VREF(dvp); + un->un_dirvp = dvp; } - *vpp = UNIONTOV(un); - return (0); + } else if (lowervp) { + vrele(lowervp); } + *vpp = UNIONTOV(un); + return (0); } /* @@ -157,8 +224,18 @@ loop: unvplock |= UN_LOCKED; error = getnewvnode(VT_UNION, mp, union_vnodeop_p, vpp); - if (error) + if (error) { + if (uppervp) { + if (dvp == uppervp) + vrele(uppervp); + else + vput(uppervp); + } + if (lowervp) + vrele(lowervp); + goto out; + } MALLOC((*vpp)->v_data, void *, sizeof(struct union_node), M_TEMP, M_WAITOK); @@ -173,7 +250,15 @@ loop: un->un_uppervp = uppervp; un->un_lowervp = lowervp; un->un_openl = 0; - un->un_flags = 0; + un->un_flags = UN_LOCKED; + if (un->un_uppervp) + un->un_flags |= UN_ULOCK; +#ifdef DIAGNOSTIC + if (curproc) + un->un_pid = curproc->p_pid; + else + un->un_pid = -1; +#endif if (cnp && (lowervp != NULLVP) && (lowervp->v_type == VREG)) { un->un_hash = cnp->cn_hash; un->un_path = malloc(cnp->cn_namelen+1, M_TEMP, M_WAITOK); @@ -192,12 +277,6 @@ loop: continue; *pp = un; - un->un_flags |= UN_LOCKED; - -#ifdef DIAGNOSTIC - un->un_pid = curproc->p_pid; -#endif - if (xlowervp) vrele(xlowervp); @@ -216,15 +295,9 @@ int union_freevp(vp) struct vnode *vp; { - struct union_node **unpp; struct union_node *un = VTOUNION(vp); - for (unpp = &unhead; *unpp != 0; unpp = &(*unpp)->un_next) { - if (*unpp == un) { - *unpp = un->un_next; - break; - } - } + union_remlist(un); FREE(vp->v_data, M_TEMP); vp->v_data = 0; @@ -504,7 +577,12 @@ void union_removed_upper(un) struct union_node *un; { - vrele(un->un_uppervp); + if (un->un_flags & UN_ULOCK) { + un->un_flags &= ~UN_ULOCK; + vput(un->un_uppervp); + } else { + vrele(un->un_uppervp); + } un->un_uppervp = NULLVP; } diff --git a/usr/src/sys/miscfs/union/union_vfsops.c b/usr/src/sys/miscfs/union/union_vfsops.c index 213939204b..0df9f551ea 100644 --- a/usr/src/sys/miscfs/union/union_vfsops.c +++ b/usr/src/sys/miscfs/union/union_vfsops.c @@ -8,7 +8,7 @@ * * %sccs.include.redist.c% * - * @(#)union_vfsops.c 1.9 (Berkeley) %G% + * @(#)union_vfsops.c 2.1 (Berkeley) %G% */ /* @@ -309,6 +309,7 @@ union_root(mp, vpp) * Return locked reference to root. */ VREF(um->um_uppervp); + VOP_LOCK(um->um_uppervp); if (um->um_lowervp) VREF(um->um_lowervp); error = union_allocvp(vpp, mp, diff --git a/usr/src/sys/miscfs/union/union_vnops.c b/usr/src/sys/miscfs/union/union_vnops.c index 6363f712d3..9a2e2483bd 100644 --- a/usr/src/sys/miscfs/union/union_vnops.c +++ b/usr/src/sys/miscfs/union/union_vnops.c @@ -8,7 +8,7 @@ * * %sccs.include.redist.c% * - * @(#)union_vnops.c 1.9 (Berkeley) %G% + * @(#)union_vnops.c 2.1 (Berkeley) %G% */ #include @@ -135,11 +135,10 @@ union_lookup(ap) * on and just return that vnode. */ if (upperdvp) { - VOP_LOCK(upperdvp); uerror = union_lookup1(um->um_uppervp, upperdvp, &uppervp, cnp); - if (uppervp != upperdvp) - VOP_UNLOCK(upperdvp); + /*if (uppervp == upperdvp) + dun->un_flags |= UN_KLOCK;*/ if (cnp->cn_consume != 0) { *ap->a_vpp = uppervp; @@ -159,15 +158,29 @@ union_lookup(ap) * instead. */ if (lowerdvp) { + int nameiop; + VOP_LOCK(lowerdvp); + + /* + * 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); + &lowervp, cnp); + cnp->cn_nameiop = nameiop; + if (lowervp != lowerdvp) VOP_UNLOCK(lowerdvp); if (cnp->cn_consume != 0) { if (uppervp) { - vput(uppervp); + if (uppervp == upperdvp) + vrele(uppervp); + else + vput(uppervp); uppervp = NULLVP; } *ap->a_vpp = lowervp; @@ -216,7 +229,12 @@ union_lookup(ap) /* 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); + dun->un_flags |= UN_ULOCK; + if (uerror) { if (lowervp) { vput(lowervp); @@ -227,8 +245,6 @@ union_lookup(ap) } } - if (uppervp) - VOP_UNLOCK(uppervp); if (lowervp) VOP_UNLOCK(lowervp); @@ -237,7 +253,7 @@ union_lookup(ap) if (error) { if (uppervp) - vrele(uppervp); + vput(uppervp); if (lowervp) vrele(lowervp); } else { @@ -266,14 +282,12 @@ union_create(ap) 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) return (error); - VOP_UNLOCK(vp); - error = union_allocvp( ap->a_vpp, ap->a_dvp->v_mount, @@ -283,7 +297,7 @@ union_create(ap) vp, NULLVP); if (error) - vrele(vp); + vput(vp); return (error); } @@ -308,15 +322,13 @@ union_mknod(ap) 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) return (error); if (vp) { - VOP_UNLOCK(vp); - error = union_allocvp( ap->a_vpp, ap->a_dvp->v_mount, @@ -326,7 +338,7 @@ union_mknod(ap) vp, NULLVP); if (error) - vrele(vp); + vput(vp); } return (error); } @@ -380,8 +392,10 @@ union_open(ap) error = union_vn_create(&vp, un, p); if (error) return (error); - un->un_uppervp = vp; /* XXX */ + /* at this point, uppervp is locked */ + un->un_uppervp = vp; /* XXX */ + un->un_flags |= UN_ULOCK; /* * Now, if the file is being opened with truncation, @@ -405,9 +419,13 @@ union_open(ap) } else { VOP_UNLOCK(tvp); } + + 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; + if (!error) uprintf("union: copied up %s\n", un->un_path); @@ -429,15 +447,21 @@ union_open(ap) if (error == 0) error = VOP_OPEN(un->un_uppervp, mode, cred, p); - VOP_UNLOCK(un->un_uppervp); 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); - VOP_UNLOCK(tvp); return (error); } @@ -498,11 +522,8 @@ union_access(ap) 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); - VOP_UNLOCK(vp); - } return (error); } @@ -521,10 +542,13 @@ union_getattr(ap) { 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); - 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]; @@ -544,10 +568,8 @@ union_setattr(ap) int error; if (un->un_uppervp) { - VOP_LOCK(un->un_uppervp); error = VOP_SETATTR(un->un_uppervp, ap->a_vap, ap->a_cred, ap->a_p); - VOP_UNLOCK(un->un_uppervp); } else { /* * XXX should do a copyfile (perhaps only if @@ -571,10 +593,13 @@ union_read(ap) { 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); - VOP_UNLOCK(vp); + if (dolock) + VOP_UNLOCK(vp); return (error); } @@ -590,10 +615,13 @@ union_write(ap) { 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); - VOP_UNLOCK(vp); + if (dolock) + VOP_UNLOCK(vp); return (error); } @@ -656,10 +684,14 @@ union_fsync(ap) 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); - VOP_UNLOCK(targetvp); + if (dolock) + VOP_UNLOCK(targetvp); } return (error); @@ -695,10 +727,10 @@ union_remove(ap) struct vnode *vp = un->un_uppervp; VREF(dvp); - VOP_LOCK(dvp); + dun->un_flags |= UN_KLOCK; 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); @@ -737,7 +769,7 @@ union_link(ap) 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); @@ -807,7 +839,7 @@ union_rename(ap) tdvp = un->un_uppervp; VREF(tdvp); - VOP_LOCK(tdvp); + un->un_flags |= UN_KLOCK; vput(ap->a_tdvp); } @@ -820,7 +852,7 @@ union_rename(ap) tvp = un->un_uppervp; VREF(tvp); - VOP_LOCK(tvp); + un->un_flags |= UN_KLOCK; vput(ap->a_tvp); } @@ -853,13 +885,12 @@ union_mkdir(ap) 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) return (error); - VOP_UNLOCK(vp); error = union_allocvp( ap->a_vpp, ap->a_dvp->v_mount, @@ -869,7 +900,7 @@ union_mkdir(ap) vp, NULLVP); if (error) - vrele(vp); + vput(vp); return (error); } @@ -894,13 +925,13 @@ union_rmdir(ap) struct vnode *vp = un->un_uppervp; VREF(dvp); - VOP_LOCK(dvp); + dun->un_flags |= UN_KLOCK; 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); + error = VOP_RMDIR(dvp, vp, ap->a_cnp); if (!error) union_removed_upper(un); @@ -938,7 +969,7 @@ union_symlink(ap) 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); @@ -969,13 +1000,8 @@ union_readdir(ap) int error = 0; struct union_node *un = VTOUNION(ap->a_vp); - if (un->un_uppervp) { - struct vnode *vp = un->un_uppervp; - - 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); } @@ -990,10 +1016,13 @@ union_readlink(ap) { 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); - VOP_UNLOCK(vp); + if (dolock) + VOP_UNLOCK(vp); return (error); } @@ -1009,11 +1038,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; + int dolock = (vp == LOWERVP(ap->a_dvp)); - if (islocked) + if (islocked && dolock) VOP_LOCK(vp); error = VOP_ABORTOP(vp, ap->a_cnp); - if (islocked) + if (islocked && dolock) VOP_UNLOCK(vp); return (error); @@ -1089,6 +1119,17 @@ union_lock(ap) { 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 && @@ -1125,6 +1166,12 @@ union_unlock(ap) #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); @@ -1149,10 +1196,13 @@ union_bmap(ap) { 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); - VOP_UNLOCK(vp); + if (dolock) + VOP_UNLOCK(vp); return (error); } @@ -1190,10 +1240,13 @@ union_pathconf(ap) { 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); - VOP_UNLOCK(vp); + if (dolock) + VOP_UNLOCK(vp); return (error); } -- 2.20.1