Date: Wed, 7 Mar 2012 18:33:11 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r232665 - stable/8/sys/fs/nullfs Message-ID: <201203071833.q27IXBZK005344@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Wed Mar 7 18:33:11 2012 New Revision: 232665 URL: http://svn.freebsd.org/changeset/base/232665 Log: Synchronize nullfs with HEAD, mostly merge all locking changes. Tested by: pho MFC r229428: Document the state of the lowervp vnode for null_nodeget(). MFC r229431: Do the vput() for the lowervp in the null_nodeget() for error case too. Several callers of null_nodeget() did the cleanup itself, but several missed it, most prominent being null_bypass(). Remove the cleanup from the callers, now null_nodeget() handles lowervp free itself. MFC r229600 (by dim): In sys/fs/nullfs/null_subr.c, in a KASSERT, output the correct vnode pointer 'lowervp' instead of 'vp', which is uninitialized at that point. MFC r230304 (by rea): Use hashdestroy() instead of naive free(). MFC r232299: Move the code to destroy half-contructed nullfs vnode into helper function null_destroy_proto() from null_insmntque_dtr(). Also apply null_destroy_proto() in null_nodeget() when we raced and a vnode is found in the hash, so the currently allocated protonode shall be destroyed. Lock the vnode interlock around reassigning the v_vnlock. MFC r232301: Always request exclusive lock for the lower vnode in nullfs_vget(). The null_nodeget() requires exclusive lock on lowervp to be able to insmntque() new vnode. MFC r232303: In null_reclaim(), assert that reclaimed vnode is fully constructed, instead of accepting half-constructed vnode. Previous code cannot decide what to do with such vnode anyway, and although processing it for hash removal, paniced later when getting rid of nullfs reference on lowervp. While there, remove initializations from the declaration block. MFC r232304: Document that null_nodeget() cannot take shared-locked lowervp due to insmntque() requirements. MFC r232305: Allow shared locks for reads when lower filesystem accept shared locking. MFC r232383: Do not expose unlocked unconstructed nullfs vnode on mount list. Lock the native nullfs vnode lock before switching the locks. Modified: stable/8/sys/fs/nullfs/null_subr.c stable/8/sys/fs/nullfs/null_vfsops.c stable/8/sys/fs/nullfs/null_vnops.c Directory Properties: stable/8/sys/ (props changed) Modified: stable/8/sys/fs/nullfs/null_subr.c ============================================================================== --- stable/8/sys/fs/nullfs/null_subr.c Wed Mar 7 18:29:12 2012 (r232664) +++ stable/8/sys/fs/nullfs/null_subr.c Wed Mar 7 18:33:11 2012 (r232665) @@ -90,7 +90,7 @@ nullfs_uninit(vfsp) { mtx_destroy(&null_hashmtx); - free(null_node_hashtbl, M_NULLFSHASH); + hashdestroy(null_node_hashtbl, M_NULLFSHASH, null_node_hash); return (0); } @@ -169,15 +169,26 @@ null_hashins(mp, xp) } static void -null_insmntque_dtr(struct vnode *vp, void *xp) +null_destroy_proto(struct vnode *vp, void *xp) { + + lockmgr(&vp->v_lock, LK_EXCLUSIVE, NULL); + VI_LOCK(vp); vp->v_data = NULL; vp->v_vnlock = &vp->v_lock; - free(xp, M_NULLFSNODE); vp->v_op = &dead_vnodeops; - (void) vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); + VI_UNLOCK(vp); vgone(vp); vput(vp); + free(xp, M_NULLFSNODE); +} + +static void +null_insmntque_dtr(struct vnode *vp, void *xp) +{ + + vput(((struct null_node *)xp)->null_lowervp); + null_destroy_proto(vp, xp); } /* @@ -198,6 +209,13 @@ null_nodeget(mp, lowervp, vpp) struct vnode *vp; int error; + /* + * The insmntque1() call below requires the exclusive lock on + * the nullfs vnode. + */ + ASSERT_VOP_ELOCKED(lowervp, "lowervp"); + KASSERT(lowervp->v_usecount >= 1, ("Unreferenced vnode %p\n", lowervp)); + /* Lookup the hash firstly */ *vpp = null_hashget(mp, lowervp); if (*vpp != NULL) { @@ -220,6 +238,7 @@ null_nodeget(mp, lowervp, vpp) error = getnewvnode("null", mp, &null_vnodeops, &vp); if (error) { + vput(lowervp); free(xp, M_NULLFSNODE); return (error); } @@ -241,9 +260,7 @@ null_nodeget(mp, lowervp, vpp) *vpp = null_hashins(mp, xp); if (*vpp != NULL) { vrele(lowervp); - vp->v_vnlock = &vp->v_lock; - xp->null_lowervp = NULL; - vrele(vp); + null_destroy_proto(vp, xp); return (0); } *vpp = vp; Modified: stable/8/sys/fs/nullfs/null_vfsops.c ============================================================================== --- stable/8/sys/fs/nullfs/null_vfsops.c Wed Mar 7 18:29:12 2012 (r232664) +++ stable/8/sys/fs/nullfs/null_vfsops.c Wed Mar 7 18:33:11 2012 (r232665) @@ -157,8 +157,7 @@ nullfs_mount(struct mount *mp) * Make sure the node alias worked */ if (error) { - vrele(lowerrootvp); - free(xmp, M_NULLFSMNT); /* XXX */ + free(xmp, M_NULLFSMNT); return (error); } @@ -181,7 +180,8 @@ nullfs_mount(struct mount *mp) MNT_IUNLOCK(mp); } MNT_ILOCK(mp); - mp->mnt_kern_flag |= lowerrootvp->v_mount->mnt_kern_flag & MNTK_MPSAFE; + mp->mnt_kern_flag |= lowerrootvp->v_mount->mnt_kern_flag & + (MNTK_MPSAFE | MNTK_SHARED_WRITES); MNT_IUNLOCK(mp); mp->mnt_data = xmp; vfs_getnewfsid(mp); @@ -308,6 +308,12 @@ nullfs_vget(mp, ino, flags, vpp) struct vnode **vpp; { int error; + + KASSERT((flags & LK_TYPE_MASK) != 0, + ("nullfs_vget: no lock requested")); + flags &= ~LK_TYPE_MASK; + flags |= LK_EXCLUSIVE; + error = VFS_VGET(MOUNTTONULLMOUNT(mp)->nullm_vfs, ino, flags, vpp); if (error) return (error); Modified: stable/8/sys/fs/nullfs/null_vnops.c ============================================================================== --- stable/8/sys/fs/nullfs/null_vnops.c Wed Mar 7 18:29:12 2012 (r232664) +++ stable/8/sys/fs/nullfs/null_vnops.c Wed Mar 7 18:33:11 2012 (r232665) @@ -365,9 +365,7 @@ null_lookup(struct vop_lookup_args *ap) vrele(lvp); } else { error = null_nodeget(dvp->v_mount, lvp, &vp); - if (error) - vput(lvp); - else + if (error == 0) *ap->a_vpp = vp; } } @@ -699,12 +697,18 @@ null_inactive(struct vop_inactive_args * static int null_reclaim(struct vop_reclaim_args *ap) { - struct vnode *vp = ap->a_vp; - struct null_node *xp = VTONULL(vp); - struct vnode *lowervp = xp->null_lowervp; + struct vnode *vp; + struct null_node *xp; + struct vnode *lowervp; - if (lowervp) - null_hashrem(xp); + vp = ap->a_vp; + xp = VTONULL(vp); + lowervp = xp->null_lowervp; + + KASSERT(lowervp != NULL && vp->v_vnlock != &vp->v_lock, + ("Reclaiming inclomplete null vnode %p", vp)); + + null_hashrem(xp); /* * Use the interlock to protect the clearing of v_data to * prevent faults in null_lock(). @@ -715,10 +719,7 @@ null_reclaim(struct vop_reclaim_args *ap vp->v_object = NULL; vp->v_vnlock = &vp->v_lock; VI_UNLOCK(vp); - if (lowervp) - vput(lowervp); - else - panic("null_reclaim: reclaiming a node with no lowervp"); + vput(lowervp); free(xp, M_NULLFSNODE); return (0); @@ -810,9 +811,7 @@ null_vptocnp(struct vop_vptocnp_args *ap #endif vhold(*dvp); vput(*dvp); - } else - vput(ldvp); - + } vn_lock(vp, locked | LK_RETRY); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203071833.q27IXBZK005344>