Date: Tue, 10 Apr 2012 11:28:34 +0900 From: Daichi GOTO <daichi@freebsd.org> To: kwhite@site.uottawa.ca Cc: freebsd-current@freebsd.org Subject: Re: (unionfs) panic: excl->share with r230341 and above Message-ID: <20120410112834.56bb2d02.daichi@freebsd.org> In-Reply-To: <14043.74.51.53.177.1333981899.squirrel@courriel.site.uottawa.ca> References: <55056.74.51.53.177.1333762589.squirrel@courriel.site.uottawa.ca> <20120408221127.969e8e71.daichi@freebsd.org> <14043.74.51.53.177.1333981899.squirrel@courriel.site.uottawa.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --Multipart=_Tue__10_Apr_2012_11_28_34_+0900_0_mygCSu_TA36cPV Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Thanks kwhite, I found an another lock issue. Please try a patch included. On Mon, 9 Apr 2012 10:31:39 -0400 (EDT) kwhite@site.uottawa.ca wrote: > > Hi > > > > Please try an attached patch that improves handlings of fs locks. > > I think that this patch can resolve this issue. If that works well, > > I'm going to refine and commit it to head. > > > > Thanks! > > I tried your patch but get the same panic: > > FreeBSD 10.0-CURRENT #6 r233856M: Mon Apr 9 09:47:42 EDT 2012 > ... > exclusive lock of (lockmgr) ufs @ > /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1897 > while share locked from /usr/src/sys/modules/unionfs/../../fs/unionfs/ > union_vnops.c:1897 > panic: excl->share > cpuid = 0 > KDB: enter: panic > [ thread pid 38 tid 100050 ] > Stopped at kdb_enter+0x3b: movl $0,kdb_why > db> show all locks > Process 38 (ls) thread 0xc56dd2e0 (100050) > exclusive sleep mutex vnode interlock (vnode interlock) r = 0 > (0xc5797d38) locked @ > /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1889 > shared lockmgr ufs (ufs) r = 0 (0xc5797d18) locked @ > /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1897 > db> > > > On Fri, 6 Apr 2012 21:36:29 -0400 (EDT) > > kwhite@site.uottawa.ca wrote: > >> Starting with r230341, I get the following panic when trying to run > >> an executable on a unionfs filesystem: > >> > >> exclusive lock of (lockmgr) ufs @ > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1843 > >> while share locked from > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1843 > >> panic: excl->share > >> cpuid = 0 > >> KDB: enter: panic > >> > >> Narrowing down with a binary search: r230340 (no panic), r230341 > >> (panic). > >> > >> How to repeat: > >> # uuname -a > >> FreeBSD 10.0-CURRENT FreeBSD 10.0-CURRENT #5 r233946M: Fri Apr > >> 6 > >> 21:09:32 EDT 2012 kwhite@demo:/usr/src/obj/usr/src/sys/GENERIC > >> i386 > >> > >> # mkdir /tmp/local > >> # mount -t unionfs -o noatime /tmp/local /usr/local > >> # cp /bin/ls /usr/local/bin/ls > >> # /usr/local/bin/ls > >> .... > >> exclusive lock of (lockmgr) ufs @ > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1843 > >> while share locked from > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1843 > >> panic: excl->share > >> cpuid = 0 > >> KDB: enter: panic > >> [ thread pid 68 tid 100054 ] > >> Stopped at kdb_enter+0x3b: movl $0,kdb_why > >> db> show all locks > >> Process 68 (ls) thread 0xc5780000 (100054) > >> exclusive sleep mutex vnode interlock (vnode interlock) r = 0 > >> (0xc57cec28) locked @ > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1835 > >> shared lockmgr ufs (ufs) r = 0 (0xc57cec08) locked @ > >> /usr/src/sys/modules/unionfs/../../fs/unionfs/union_vnops.c:1843 > >> db> > >> > >> Workaround? > >> After reverting the change from LK_EXCLUSIVE to LK_SHARED > >> in sys/kern/kern_exec.c, executables on union filesystems no > >> longer cause a panic. > >> > >> ...keith > >> > >> > >> _______________________________________________ > >> freebsd-current@freebsd.org mailing list > >> http://lists.freebsd.org/mailman/listinfo/freebsd-current > >> To unsubscribe, send any mail to > >> "freebsd-current-unsubscribe@freebsd.org" > > > > > > -- > > Daichi GOTO (daichi) > > FreeBSD Committer, http://www.FreeBSD.org The Power To Serve > > > > > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" -- Daichi GOTO (daichi) FreeBSD Committer, http://www.FreeBSD.org The Power To Serve --Multipart=_Tue__10_Apr_2012_11_28_34_+0900_0_mygCSu_TA36cPV Content-Type: text/x-diff; name="unionfs_20120410.diff" Content-Disposition: attachment; filename="unionfs_20120410.diff" Content-Transfer-Encoding: 7bit diff -urBN /usr/src.orig/sys/fs/unionfs/union_subr.c /usr/src/sys/fs/unionfs/union_subr.c --- /usr/src.orig/sys/fs/unionfs/union_subr.c 2012-04-08 14:01:23.000000000 +0900 +++ /usr/src/sys/fs/unionfs/union_subr.c 2012-04-10 02:22:38.000000000 +0900 @@ -350,19 +350,22 @@ uvp = unp->un_uppervp; dvp = unp->un_dvp; unp->un_lowervp = unp->un_uppervp = NULLVP; - vp->v_vnlock = &(vp->v_lock); vp->v_data = NULL; - lockmgr(vp->v_vnlock, LK_EXCLUSIVE | LK_INTERLOCK, VI_MTX(vp)); + vp->v_object = NULL; + VI_UNLOCK(vp); + if (lvp != NULLVP) - VOP_UNLOCK(lvp, 0); + VOP_UNLOCK(lvp, LK_RELEASE); if (uvp != NULLVP) - VOP_UNLOCK(uvp, 0); - vp->v_object = NULL; + VOP_UNLOCK(uvp, LK_RELEASE); if (dvp != NULLVP && unp->un_hash.le_prev != NULL) unionfs_rem_cached_vnode(unp, dvp); + if (lockmgr(vp->v_vnlock, LK_EXCLUSIVE, VI_MTX(vp)) != 0) + panic("the lock for deletion is unacquirable."); + if (lvp != NULLVP) { vfslocked = VFS_LOCK_GIANT(lvp->v_mount); vrele(lvp); @@ -550,7 +553,7 @@ cn->cn_flags |= (cnp->cn_flags & SAVESTART); vref(dvp); - VOP_UNLOCK(dvp, 0); + VOP_UNLOCK(dvp, LK_RELEASE); if ((error = relookup(dvp, vpp, cn))) { uma_zfree(namei_zone, cn->cn_pnbuf); @@ -957,7 +960,7 @@ *vpp = vp; unionfs_vn_create_on_upper_free_out1: - VOP_UNLOCK(udvp, 0); + VOP_UNLOCK(udvp, LK_RELEASE); unionfs_vn_create_on_upper_free_out2: if (cn.cn_flags & HASBUF) { diff -urBN /usr/src.orig/sys/fs/unionfs/union_vfsops.c /usr/src/sys/fs/unionfs/union_vfsops.c --- /usr/src.orig/sys/fs/unionfs/union_vfsops.c 2012-04-08 14:01:23.000000000 +0900 +++ /usr/src/sys/fs/unionfs/union_vfsops.c 2012-04-10 02:22:38.000000000 +0900 @@ -165,7 +165,7 @@ uid = va.va_uid; gid = va.va_gid; } - VOP_UNLOCK(mp->mnt_vnodecovered, 0); + VOP_UNLOCK(mp->mnt_vnodecovered, LK_RELEASE); if (error) return (error); @@ -250,7 +250,7 @@ * Save reference */ if (below) { - VOP_UNLOCK(upperrootvp, 0); + VOP_UNLOCK(upperrootvp, LK_RELEASE); vn_lock(lowerrootvp, LK_EXCLUSIVE | LK_RETRY); ump->um_lowervp = upperrootvp; ump->um_uppervp = lowerrootvp; @@ -281,7 +281,7 @@ /* * Unlock the node */ - VOP_UNLOCK(ump->um_uppervp, 0); + VOP_UNLOCK(ump->um_uppervp, LK_RELEASE); /* * Get the unionfs root vnode. diff -urBN /usr/src.orig/sys/fs/unionfs/union_vnops.c /usr/src/sys/fs/unionfs/union_vnops.c --- /usr/src.orig/sys/fs/unionfs/union_vnops.c 2012-04-08 14:01:23.000000000 +0900 +++ /usr/src/sys/fs/unionfs/union_vnops.c 2012-04-10 02:22:38.000000000 +0900 @@ -75,21 +75,6 @@ KASSERT(((vp)->v_op == &unionfs_vnodeops), \ ("unionfs: it is not unionfs-vnode")) -/* lockmgr lock <-> reverse table */ -struct lk_lr_table { - int lock; - int revlock; -}; - -static struct lk_lr_table un_llt[] = { - {LK_SHARED, LK_RELEASE}, - {LK_EXCLUSIVE, LK_RELEASE}, - {LK_UPGRADE, LK_DOWNGRADE}, - {LK_DOWNGRADE, LK_UPGRADE}, - {0, 0} -}; - - static int unionfs_lookup(struct vop_cachedlookup_args *ap) { @@ -141,7 +126,7 @@ if (udvp != NULLVP) { dtmpvp = udvp; if (ldvp != NULLVP) - VOP_UNLOCK(ldvp, 0); + VOP_UNLOCK(ldvp, LK_RELEASE); } else dtmpvp = ldvp; @@ -149,7 +134,7 @@ error = VOP_LOOKUP(dtmpvp, &vp, cnp); if (dtmpvp == udvp && ldvp != NULLVP) { - VOP_UNLOCK(udvp, 0); + VOP_UNLOCK(udvp, LK_RELEASE); vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY); } @@ -161,10 +146,10 @@ */ if (nameiop == DELETE || nameiop == RENAME || (cnp->cn_lkflags & LK_TYPE_MASK)) - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); vrele(vp); - VOP_UNLOCK(dvp, 0); + VOP_UNLOCK(dvp, LK_RELEASE); *(ap->a_vpp) = dunp->un_dvp; vref(dunp->un_dvp); @@ -202,7 +187,7 @@ } if (nameiop == DELETE || nameiop == RENAME || (cnp->cn_lkflags & LK_TYPE_MASK)) - VOP_UNLOCK(uvp, 0); + VOP_UNLOCK(uvp, LK_RELEASE); } /* check whiteout */ @@ -246,7 +231,7 @@ return (lerror); } if (cnp->cn_lkflags & LK_TYPE_MASK) - VOP_UNLOCK(lvp, 0); + VOP_UNLOCK(lvp, LK_RELEASE); } } @@ -281,7 +266,7 @@ goto unionfs_lookup_out; if (LK_SHARED == (cnp->cn_lkflags & LK_TYPE_MASK)) - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); if (LK_EXCLUSIVE != VOP_ISLOCKED(vp)) { vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); lockflag = 1; @@ -289,7 +274,7 @@ error = unionfs_mkshadowdir(MOUNTTOUNIONFSMOUNT(dvp->v_mount), udvp, VTOUNIONFS(vp), cnp, td); if (lockflag != 0) - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); if (error != 0) { UNIONFSDEBUG("unionfs_lookup: Unable to create shadow dir."); if ((cnp->cn_lkflags & LK_TYPE_MASK) == LK_EXCLUSIVE) @@ -386,7 +371,7 @@ if (vp->v_type == VSOCK) *(ap->a_vpp) = vp; else { - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); error = unionfs_nodeget(ap->a_dvp->v_mount, vp, NULLVP, ap->a_dvp, ap->a_vpp, cnp, curthread); vrele(vp); @@ -460,7 +445,7 @@ if (vp->v_type == VSOCK) *(ap->a_vpp) = vp; else { - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); error = unionfs_nodeget(ap->a_dvp->v_mount, vp, NULLVP, ap->a_dvp, ap->a_vpp, cnp, curthread); vrele(vp); @@ -564,6 +549,7 @@ struct unionfs_node_status *unsp; struct ucred *cred; struct thread *td; + struct vnode *vp; struct vnode *ovp; UNIONFS_INTERNAL_DEBUG("unionfs_close: enter\n"); @@ -571,12 +557,14 @@ KASSERT_UNIONFS_VNODE(ap->a_vp); locked = 0; - unp = VTOUNIONFS(ap->a_vp); + vp = ap->a_vp; + unp = VTOUNIONFS(vp); cred = ap->a_cred; td = ap->a_td; - if (VOP_ISLOCKED(ap->a_vp) != LK_EXCLUSIVE) { - vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY); + if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { + if (vn_lock(vp, LK_UPGRADE) != 0) + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); locked = 1; } unionfs_get_node_status(unp, td, &unsp); @@ -599,7 +587,7 @@ if (error != 0) goto unionfs_close_abort; - ap->a_vp->v_object = ovp->v_object; + vp->v_object = ovp->v_object; if (ovp == unp->un_uppervp) { unsp->uns_upper_opencnt--; @@ -610,7 +598,7 @@ unsp->uns_lower_opencnt--; } if (unsp->uns_lower_opencnt > 0) - ap->a_vp->v_object = unp->un_lowervp->v_object; + vp->v_object = unp->un_lowervp->v_object; } } else unsp->uns_lower_opencnt--; @@ -619,7 +607,7 @@ unionfs_tryrem_node_status(unp, unsp); if (locked != 0) - VOP_UNLOCK(ap->a_vp, 0); + vn_lock(vp, LK_DOWNGRADE | LK_RETRY); UNIONFS_INTERNAL_DEBUG("unionfs_close: leave (%d)\n", error); @@ -914,7 +902,7 @@ unionfs_get_node_status(unp, ap->a_td, &unsp); ovp = (unsp->uns_upper_opencnt ? unp->un_uppervp : unp->un_lowervp); unionfs_tryrem_node_status(unp, unsp); - VOP_UNLOCK(ap->a_vp, 0); + VOP_UNLOCK(ap->a_vp, LK_RELEASE); if (ovp == NULLVP) return (EBADF); @@ -941,7 +929,7 @@ unionfs_get_node_status(unp, ap->a_td, &unsp); ovp = (unsp->uns_upper_opencnt ? unp->un_uppervp : unp->un_lowervp); unionfs_tryrem_node_status(unp, unsp); - VOP_UNLOCK(ap->a_vp, 0); + VOP_UNLOCK(ap->a_vp, LK_RELEASE); if (ovp == NULLVP) return (EBADF); @@ -1001,7 +989,7 @@ ump = NULL; vp = uvp = lvp = NULLVP; /* search vnode */ - VOP_UNLOCK(ap->a_vp, 0); + VOP_UNLOCK(ap->a_vp, LK_RELEASE); error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr, strlen(cnp->cn_nameptr), DELETE); if (error != 0 && error != ENOENT) { @@ -1204,7 +1192,7 @@ if ((error = vn_lock(fvp, LK_EXCLUSIVE)) != 0) goto unionfs_rename_abort; error = unionfs_copyfile(unp, 1, fcnp->cn_cred, td); - VOP_UNLOCK(fvp, 0); + VOP_UNLOCK(fvp, LK_RELEASE); if (error != 0) goto unionfs_rename_abort; break; @@ -1212,7 +1200,7 @@ if ((error = vn_lock(fvp, LK_EXCLUSIVE)) != 0) goto unionfs_rename_abort; error = unionfs_mkshadowdir(ump, rfdvp, unp, fcnp, td); - VOP_UNLOCK(fvp, 0); + VOP_UNLOCK(fvp, LK_RELEASE); if (error != 0) goto unionfs_rename_abort; break; @@ -1269,13 +1257,13 @@ if ((error = vn_lock(fdvp, LK_EXCLUSIVE)) != 0) goto unionfs_rename_abort; error = unionfs_relookup_for_delete(fdvp, fcnp, td); - VOP_UNLOCK(fdvp, 0); + VOP_UNLOCK(fdvp, LK_RELEASE); if (error != 0) goto unionfs_rename_abort; /* Locke of tvp is canceled in order to avoid recursive lock. */ if (tvp != NULLVP && tvp != tdvp) - VOP_UNLOCK(tvp, 0); + VOP_UNLOCK(tvp, LK_RELEASE); error = unionfs_relookup_for_rename(tdvp, tcnp, td); if (tvp != NULLVP && tvp != tdvp) vn_lock(tvp, LK_EXCLUSIVE | LK_RETRY); @@ -1293,11 +1281,11 @@ } if (ltdvp != NULLVP) - VOP_UNLOCK(ltdvp, 0); + VOP_UNLOCK(ltdvp, LK_RELEASE); if (tdvp != rtdvp) vrele(tdvp); if (ltvp != NULLVP) - VOP_UNLOCK(ltvp, 0); + VOP_UNLOCK(ltvp, LK_RELEASE); if (tvp != rtvp && tvp != NULLVP) { if (rtvp == NULLVP) vput(tvp); @@ -1371,7 +1359,7 @@ } if ((error = VOP_MKDIR(udvp, &uvp, cnp, ap->a_vap)) == 0) { - VOP_UNLOCK(uvp, 0); + VOP_UNLOCK(uvp, LK_RELEASE); cnp->cn_lkflags = LK_EXCLUSIVE; error = unionfs_nodeget(ap->a_dvp->v_mount, uvp, NULLVP, ap->a_dvp, ap->a_vpp, cnp, td); @@ -1467,7 +1455,7 @@ if (udvp != NULLVP) { error = VOP_SYMLINK(udvp, &uvp, cnp, ap->a_vap, ap->a_target); if (error == 0) { - VOP_UNLOCK(uvp, 0); + VOP_UNLOCK(uvp, LK_RELEASE); cnp->cn_lkflags = LK_EXCLUSIVE; error = unionfs_nodeget(ap->a_dvp->v_mount, uvp, NULLVP, ap->a_dvp, ap->a_vpp, cnp, td); @@ -1490,6 +1478,7 @@ struct unionfs_node *unp; struct unionfs_node_status *unsp; struct uio *uio; + struct vnode *vp; struct vnode *uvp; struct vnode *lvp; struct thread *td; @@ -1505,41 +1494,49 @@ error = 0; eofflag = 0; locked = 0; - unp = VTOUNIONFS(ap->a_vp); uio = ap->a_uio; - uvp = unp->un_uppervp; - lvp = unp->un_lowervp; + uvp = NULLVP; + lvp = NULLVP; td = uio->uio_td; ncookies_bk = 0; cookies_bk = NULL; - if (ap->a_vp->v_type != VDIR) + vp = ap->a_vp; + if (vp->v_type != VDIR) return (ENOTDIR); - /* check opaque */ - if (uvp != NULLVP && lvp != NULLVP) { - if ((error = VOP_GETATTR(uvp, &va, ap->a_cred)) != 0) - goto unionfs_readdir_exit; - if (va.va_flags & OPAQUE) - lvp = NULLVP; - } - /* check the open count. unionfs needs to open before readdir. */ - if (VOP_ISLOCKED(ap->a_vp) != LK_EXCLUSIVE) { - vn_lock(ap->a_vp, LK_UPGRADE | LK_RETRY); + if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { + if (vn_lock(vp, LK_UPGRADE) != 0) + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); locked = 1; } - unionfs_get_node_status(unp, td, &unsp); - if ((uvp != NULLVP && unsp->uns_upper_opencnt <= 0) || - (lvp != NULLVP && unsp->uns_lower_opencnt <= 0)) { - unionfs_tryrem_node_status(unp, unsp); + unp = VTOUNIONFS(vp); + if (unp == NULL) error = EBADF; + else { + uvp = unp->un_uppervp; + lvp = unp->un_lowervp; + unionfs_get_node_status(unp, td, &unsp); + if ((uvp != NULLVP && unsp->uns_upper_opencnt <= 0) || + (lvp != NULLVP && unsp->uns_lower_opencnt <= 0)) { + unionfs_tryrem_node_status(unp, unsp); + error = EBADF; + } } - if (locked == 1) - vn_lock(ap->a_vp, LK_DOWNGRADE | LK_RETRY); + if (locked) + vn_lock(vp, LK_DOWNGRADE | LK_RETRY); if (error != 0) goto unionfs_readdir_exit; + /* check opaque */ + if (uvp != NULLVP && lvp != NULLVP) { + if ((error = VOP_GETATTR(uvp, &va, ap->a_cred)) != 0) + goto unionfs_readdir_exit; + if (va.va_flags & OPAQUE) + lvp = NULLVP; + } + /* upper only */ if (uvp != NULLVP && lvp == NULLVP) { error = VOP_READDIR(uvp, uio, ap->a_cred, ap->a_eofflag, @@ -1743,18 +1740,66 @@ } static int -unionfs_get_llt_revlock(int flags) +unionfs_islocked(struct vop_islocked_args *ap) { - int count; + struct unionfs_node *unp; - flags &= LK_TYPE_MASK; - for (count = 0; un_llt[count].lock != 0; count++) { - if (flags == un_llt[count].lock) { - return un_llt[count].revlock; - } + KASSERT_UNIONFS_VNODE(ap->a_vp); + + unp = VTOUNIONFS(ap->a_vp); + if (unp == NULL) + return (vop_stdislocked(ap)); + + if (unp->un_uppervp != NULLVP) + return (VOP_ISLOCKED(unp->un_uppervp)); + if (unp->un_lowervp != NULLVP) + return (VOP_ISLOCKED(unp->un_lowervp)); + return (vop_stdislocked(ap)); +} + +static int +unionfs_get_llt_revlock(struct vnode *vp, int flags) +{ + int revlock; + + revlock = 0; + + switch (flags & LK_TYPE_MASK) { + case LK_SHARED: + if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE) + revlock = LK_UPGRADE; + else + revlock = LK_RELEASE; + break; + case LK_EXCLUSIVE: + case LK_UPGRADE: + revlock = LK_RELEASE; + break; + case LK_DOWNGRADE: + revlock = LK_UPGRADE; + break; + default: + break; } - return 0; + return (revlock); +} + +/* + * The state of an acquired lock is adjusted similarly to + * the time of error generating. + * flags: LK_RELEASE or LK_UPGRADE + */ +static void +unionfs_revlock(struct vnode *vp, int flags) +{ + if (flags & LK_RELEASE) + VOP_UNLOCK(vp, flags); + else { + /* UPGRADE */ + if (vn_lock(vp, flags) != 0) + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); + } } static int @@ -1763,6 +1808,7 @@ int error; int flags; int revlock; + int interlock; int uhold; struct mount *mp; struct unionfs_mount *ump; @@ -1774,15 +1820,13 @@ KASSERT_UNIONFS_VNODE(ap->a_vp); error = 0; + interlock = 1; uhold = 0; flags = ap->a_flags; vp = ap->a_vp; if (LK_RELEASE == (flags & LK_TYPE_MASK) || !(flags & LK_TYPE_MASK)) - return (VOP_UNLOCK(vp, flags)); - - if ((revlock = unionfs_get_llt_revlock(flags)) == 0) - panic("unknown lock type: 0x%x", flags & LK_TYPE_MASK); + return (VOP_UNLOCK(vp, flags | LK_RELEASE)); if ((flags & LK_INTERLOCK) == 0) VI_LOCK(vp); @@ -1798,6 +1842,9 @@ lvp = unp->un_lowervp; uvp = unp->un_uppervp; + if ((revlock = unionfs_get_llt_revlock(vp, flags)) == 0) + panic("unknown lock type: 0x%x", flags & LK_TYPE_MASK); + if ((mp->mnt_kern_flag & MNTK_MPSAFE) != 0 && (vp->v_iflag & VI_OWEINACT) != 0) flags |= LK_NOWAIT; @@ -1811,6 +1858,23 @@ flags |= LK_CANRECURSE; if (lvp != NULLVP) { + if (uvp != NULLVP && flags & LK_UPGRADE) { + /* Share Lock is once released and a deadlock is avoided. */ + VI_LOCK_FLAGS(uvp, MTX_DUPOK); + vholdl(uvp); + uhold = 1; + VI_UNLOCK(vp); + VOP_UNLOCK(uvp, LK_RELEASE | LK_INTERLOCK); + VI_LOCK(vp); + unp = VTOUNIONFS(vp); + if (unp == NULL) { + /* vnode is released. */ + VI_UNLOCK(vp); + VOP_UNLOCK(lvp, LK_RELEASE); + vdrop(uvp); + return (EBUSY); + } + } VI_LOCK_FLAGS(lvp, MTX_DUPOK); flags |= LK_INTERLOCK; vholdl(lvp); @@ -1823,19 +1887,28 @@ VI_LOCK(vp); unp = VTOUNIONFS(vp); if (unp == NULL) { + /* vnode is released. */ VI_UNLOCK(vp); if (error == 0) - VOP_UNLOCK(lvp, 0); + VOP_UNLOCK(lvp, LK_RELEASE); vdrop(lvp); + if (uhold != 0) + vdrop(uvp); return (vop_stdlock(ap)); } } if (error == 0 && uvp != NULLVP) { + if (uhold && flags & LK_UPGRADE) { + flags &= ~LK_TYPE_MASK; + flags |= LK_EXCLUSIVE; + } VI_LOCK_FLAGS(uvp, MTX_DUPOK); flags |= LK_INTERLOCK; - vholdl(uvp); - uhold = 1; + if (uhold == 0) { + vholdl(uvp); + uhold = 1; + } VI_UNLOCK(vp); ap->a_flags &= ~LK_INTERLOCK; @@ -1845,30 +1918,27 @@ VI_LOCK(vp); unp = VTOUNIONFS(vp); if (unp == NULL) { + /* vnode is released. */ VI_UNLOCK(vp); - if (error == 0) { - VOP_UNLOCK(uvp, 0); - if (lvp != NULLVP) - VOP_UNLOCK(lvp, 0); - } - if (lvp != NULLVP) - vdrop(lvp); + if (error == 0) + VOP_UNLOCK(uvp, LK_RELEASE); vdrop(uvp); + if (lvp != NULLVP) { + VOP_UNLOCK(lvp, LK_RELEASE); + vdrop(lvp); + } return (vop_stdlock(ap)); } - if (error != 0 && lvp != NULLVP) { + /* rollback */ VI_UNLOCK(vp); - if ((revlock & LK_TYPE_MASK) == LK_RELEASE) - VOP_UNLOCK(lvp, revlock); - else - vn_lock(lvp, revlock | LK_RETRY); - goto unionfs_lock_abort; + unionfs_revlock(lvp, revlock); + interlock = 0; } } - VI_UNLOCK(vp); -unionfs_lock_abort: + if (interlock) + VI_UNLOCK(vp); if (lvp != NULLVP) vdrop(lvp); if (uhold != 0) @@ -2013,7 +2083,7 @@ unionfs_tryrem_node_status(unp, unsp); } - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); error = VOP_ADVLOCK(uvp, ap->a_id, ap->a_op, ap->a_fl, ap->a_flags); @@ -2022,7 +2092,7 @@ return error; unionfs_advlock_abort: - VOP_UNLOCK(vp, 0); + VOP_UNLOCK(vp, LK_RELEASE); UNIONFS_INTERNAL_DEBUG("unionfs_advlock: leave (%d)\n", error); @@ -2150,7 +2220,8 @@ error = VOP_OPENEXTATTR(tvp, ap->a_cred, ap->a_td); if (error == 0) { - vn_lock(vp, LK_UPGRADE | LK_RETRY); + if (vn_lock(vp, LK_UPGRADE) != 0) + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); if (tvp == unp->un_uppervp) unp->un_flag |= UNIONFS_OPENEXTU; else @@ -2186,7 +2257,8 @@ error = VOP_CLOSEEXTATTR(tvp, ap->a_commit, ap->a_cred, ap->a_td); if (error == 0) { - vn_lock(vp, LK_UPGRADE | LK_RETRY); + if (vn_lock(vp, LK_UPGRADE) != 0) + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); if (tvp == unp->un_uppervp) unp->un_flag &= ~UNIONFS_OPENEXTU; else @@ -2435,6 +2507,7 @@ .vop_getextattr = unionfs_getextattr, .vop_getwritemount = unionfs_getwritemount, .vop_inactive = unionfs_inactive, + .vop_islocked = unionfs_islocked, .vop_ioctl = unionfs_ioctl, .vop_link = unionfs_link, .vop_listextattr = unionfs_listextattr, --Multipart=_Tue__10_Apr_2012_11_28_34_+0900_0_mygCSu_TA36cPV--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120410112834.56bb2d02.daichi>