From 44b1cf3dde5a3bcad373ed2dbc821befd0035531 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 9 Aug 2006 22:47:36 +0000 Subject: [PATCH] VNode sequencing and locking - part 1/4. Separate vref() for the case where the ref count is already non-zero (which is nearly all uses of vref()), vs the case where it might be zero. Clean up the code in preparation for putting it under a spinlock. --- sys/emulation/linux/linux_util.c | 4 +- sys/kern/vfs_cache.c | 7 +--- sys/kern/vfs_lock.c | 72 +++++++++++++++++--------------- sys/kern/vfs_mount.c | 15 +++---- sys/kern/vfs_subr.c | 13 +++--- sys/sys/vnode.h | 4 +- sys/vfs/fdesc/fdesc_vfsops.c | 11 ++--- sys/vfs/nullfs/null_vfsops.c | 19 +++------ sys/vfs/portal/portal_vfsops.c | 11 ++--- 9 files changed, 73 insertions(+), 83 deletions(-) diff --git a/sys/emulation/linux/linux_util.c b/sys/emulation/linux/linux_util.c index ef0a79866e..aa62f55f08 100644 --- a/sys/emulation/linux/linux_util.c +++ b/sys/emulation/linux/linux_util.c @@ -28,7 +28,7 @@ * * from: svr4_util.c,v 1.5 1995/01/22 23:44:50 christos Exp * $FreeBSD: src/sys/compat/linux/linux_util.c,v 1.12.2.2 2001/11/05 19:08:23 marcel Exp $ - * $DragonFly: src/sys/emulation/linux/linux_util.c,v 1.11 2006/05/06 02:43:11 dillon Exp $ + * $DragonFly: src/sys/emulation/linux/linux_util.c,v 1.12 2006/08/09 22:47:31 dillon Exp $ */ #include @@ -132,7 +132,7 @@ linux_copyin_path(char *uname, char **kname, int flags) vproot = NULL; if (error == 0) { error = cache_vref(ndroot.nl_ncp, ndroot.nl_cred, - &vproot); + &vproot); } nlookup_done(&ndroot); if (error) { diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index ef0b9c820a..1f99f1c234 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -67,7 +67,7 @@ * * @(#)vfs_cache.c 8.5 (Berkeley) 3/22/95 * $FreeBSD: src/sys/kern/vfs_cache.c,v 1.42.2.6 2001/10/05 20:07:03 dillon Exp $ - * $DragonFly: src/sys/kern/vfs_cache.c,v 1.72 2006/06/05 07:26:10 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_cache.c,v 1.73 2006/08/09 22:47:32 dillon Exp $ */ #include @@ -242,9 +242,6 @@ _cache_drop(struct namecache *ncp) /* * Link a new namecache entry to its parent. Be careful to avoid races * if vhold() blocks in the future. - * - * If we are creating a child under an oldapi parent we must mark the - * child as being an oldapi entry as well. */ static void cache_link_parent(struct namecache *ncp, struct namecache *par) @@ -883,7 +880,7 @@ again: cache_unlock(ncp); goto again; } - vref(vp); + vref_initial(vp, 1); } if (error == 0 && vp == NULL) error = ENOENT; diff --git a/sys/kern/vfs_lock.c b/sys/kern/vfs_lock.c index 8a5789fc56..b8ae8c807d 100644 --- a/sys/kern/vfs_lock.c +++ b/sys/kern/vfs_lock.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/kern/vfs_lock.c,v 1.19 2006/05/27 20:17:16 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_lock.c,v 1.20 2006/08/09 22:47:32 dillon Exp $ */ /* @@ -94,7 +94,6 @@ static __inline void __vbusy(struct vnode *vp) { - KKASSERT(vp->v_flag & VFREE); TAILQ_REMOVE(&vnode_free_list, vp, v_freelist); freevnodes--; vp->v_flag &= ~(VFREE|VAGE); @@ -104,7 +103,6 @@ static __inline void __vfree(struct vnode *vp) { - KKASSERT((vp->v_flag & VFREE) == 0); if (vp->v_flag & (VAGE|VRECLAIMED)) TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist); else @@ -120,6 +118,8 @@ __vfree(struct vnode *vp) static __inline int vshouldfree(struct vnode *vp, int usecount) { + if (vp->v_flag & VFREE) + return (0); /* already free */ if (vp->v_holdcnt != 0 || vp->v_usecount != usecount) return (0); /* other holderse */ if (vp->v_object && @@ -151,20 +151,31 @@ __vref(struct vnode *vp) } /* - * This is a rare case where callers are allowed to hold spinlocks, so - * we can't ourselves. In such cases the vnode must already have at least - * one reference because we cannot get the spinlock required to move - * the vnode off the free list. - * - * If the usecount & holdcnt are 0 the caller must be holding the - * free list spinlock since we will be removing the vnode from the - * freelist in that case. + * Add another ref to a vnode. The vnode must already have at least one + * ref. */ void vref(struct vnode *vp) +{ + KKASSERT(vp->v_usecount > 0 && (vp->v_flag & (VFREE|VINACTIVE)) == 0); + atomic_add_int(&vp->v_usecount, 1); +} + +/* + * Add a ref to a vnode which may not have any refs. This routine is called + * from the namecache and vx_get(). The vnode is explicitly removed from + * the freelist if necessary. If requested, the vnode will be reactivated. + * + * vget(), cache_vget(), and cache_vref() reactives vnodes. vx_get() does + * not. + */ +void +vref_initial(struct vnode *vp, int reactivate) { crit_enter(); __vref(vp); + if (reactivate) + vp->v_flag &= ~VINACTIVE; crit_exit(); } @@ -173,7 +184,8 @@ vrele(struct vnode *vp) { crit_enter(); if (vp->v_usecount == 1) { - KASSERT(lockcountnb(&vp->v_lock) == 0, ("last vrele vp %p still locked", vp)); + KASSERT(lockcountnb(&vp->v_lock) == 0, + ("last vrele vp %p still locked", vp)); /* * Deactivation requires an exclusive v_lock (vx_lock()), and @@ -199,22 +211,15 @@ vrele(struct vnode *vp) } /* - * Hold a vnode or drop the hold on a vnode. The vnode will be taken off - * the freelist if it is on it and cannot be recycled. However, the - * vnode can be deactivated and reactivated while held. + * Hold a vnode, preventing it from being recycled (unless it is already + * undergoing a recyclement or already has been recycled). * - * Special cases: The last drop of a vnode does nothing special, allowing it - * to be called from an interrupt. vrele() on the otherhand cannot be called - * from an interrupt. + * NOTE: vhold() will defer removal of the vnode from the freelist. */ void vhold(struct vnode *vp) { - crit_enter(); - ++vp->v_holdcnt; - if (vp->v_flag & VFREE) - __vbusy(vp); - crit_exit(); + atomic_add_int(&vp->v_holdcnt, 1); } void @@ -236,11 +241,13 @@ vdrop(struct vnode *vp) * VX LOCKING FUNCTIONS * **************************************************************** * - * These functions lock vnodes for reclamation and deactivation ops. - * Only vp->v_lock, the top layer of the VFS, is locked. You must be - * holding a normal reference in order to be able to safely call vx_lock() - * and vx_unlock(). vx_get() and vx_put() are combination functions which - * vref+vx_lock and vrele+vx_unlock. + * These functions lock vnodes for reclamation and deactivation related + * activities. Only vp->v_lock, the top layer of the VFS, is locked. + * You must be holding a normal reference in order to be able to safely + * call vx_lock() and vx_unlock(). + * + * vx_get() also differs from vget() in that it does not clear the + * VINACTIVE bit on a vnode. */ #define VXLOCKFLAGS (LK_EXCLUSIVE|LK_RETRY) @@ -275,7 +282,7 @@ vx_get(struct vnode *vp) { int error; - vref(vp); + vref_initial(vp, 0); if ((error = __vxlock(vp, VXLOCKFLAGS)) != 0) vrele(vp); return(error); @@ -286,7 +293,7 @@ vx_get_nonblock(struct vnode *vp) { int error; - vref(vp); + vref_initial(vp, 0); if ((error = __vxlock(vp, VXLOCKFLAGS_NB)) != 0) vrele(vp); return(error); @@ -418,7 +425,7 @@ allocvnode(int lktimeout, int lkflags) * Note the lack of a critical section. We vx_get() * the vnode before we check it for validity, reducing * the number of checks we have to make. The vx_get() - * will pull it off the freelist. + * will also pull the vnode off of the freelist. */ if (vx_get(vp)) { vp = NULL; @@ -443,8 +450,7 @@ allocvnode(int lktimeout, int lkflags) * Ok, we can reclaim the vnode if it isn't already * in a reclaimed state. If the reclamation fails, * or if someone else is referencing the vnode after - * we have vgone()'d it, we recycle the vnode on the - * freelist or hold it (by calling vx_put()). + * we have vgone()'d it, we stop here. */ if ((vp->v_flag & VRECLAIMED) == 0) { vgone(vp); diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c index b313457195..5654dd61b8 100644 --- a/sys/kern/vfs_mount.c +++ b/sys/kern/vfs_mount.c @@ -67,7 +67,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/kern/vfs_mount.c,v 1.19 2006/08/08 03:52:40 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_mount.c,v 1.20 2006/08/09 22:47:32 dillon Exp $ */ /* @@ -394,17 +394,18 @@ vfs_getnewfsid(struct mount *mp) * candidate for being (eventually) vgone()'d. Returns 0 if the vnode is * not a good candidate, 1 if it is. * - * vnodes marked VFREE are already on the free list, but may still need - * to be recycled due to eating namecache resources and potentially blocking - * the namecache directory chain and related vnodes from being freed. + * Note that a vnode can be marked VFREE without really being free, so + * we don't use the flag for any tests. */ static __inline int vmightfree(struct vnode *vp, int page_count) { if (vp->v_flag & VRECLAIMED) return (0); +#if 0 if ((vp->v_flag & VFREE) && TAILQ_EMPTY(&vp->v_namecache)) return (0); +#endif if (vp->v_usecount != 0) return (0); if (vp->v_object && vp->v_object->resident_page_count >= page_count) @@ -917,9 +918,6 @@ vmntvnodescan( case VMSC_GETVX: error = vx_get(vp); break; - case VMSC_REFVP: - vref(vp); - /* fall through */ default: error = 0; break; @@ -945,9 +943,6 @@ vmntvnodescan( case VMSC_GETVX: vx_put(vp); break; - case VMSC_REFVP: - vrele(vp); - /* fall through */ default: break; } diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index 2d425ee0ab..9057dd4695 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -37,7 +37,7 @@ * * @(#)vfs_subr.c 8.31 (Berkeley) 5/26/95 * $FreeBSD: src/sys/kern/vfs_subr.c,v 1.249.2.30 2003/04/04 20:35:57 tegge Exp $ - * $DragonFly: src/sys/kern/vfs_subr.c,v 1.91 2006/07/18 22:22:12 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_subr.c,v 1.92 2006/08/09 22:47:32 dillon Exp $ */ /* @@ -149,16 +149,13 @@ rb_buf_compare(struct buf *b1, struct buf *b2) } /* - * Return 0 if the vnode is already on the free list or cannot be placed - * on the free list. Return 1 if the vnode can be placed on the free list. + * Returns non-zero if the vnode is a candidate for lazy msyncing. */ static __inline int -vshouldfree(struct vnode *vp, int usecount) +vshouldmsync(struct vnode *vp, int usecount) { - if (vp->v_flag & VFREE) - return (0); /* already free */ if (vp->v_holdcnt != 0 || vp->v_usecount != usecount) - return (0); /* other holderse */ + return (0); /* other holders */ if (vp->v_object && (vp->v_object->ref_count || vp->v_object->resident_page_count)) { return (0); @@ -1861,7 +1858,7 @@ vfs_msync_scan1(struct mount *mp, struct vnode *vp, void *data) int flags = (int)data; if ((vp->v_flag & VRECLAIMED) == 0) { - if (vshouldfree(vp, 0)) + if (vshouldmsync(vp, 0)) return(0); /* call scan2 */ if ((mp->mnt_flag & MNT_RDONLY) == 0 && (vp->v_flag & VOBJDIRTY) && diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index 1c579568cf..2021ba5721 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -32,7 +32,7 @@ * * @(#)vnode.h 8.7 (Berkeley) 2/4/94 * $FreeBSD: src/sys/sys/vnode.h,v 1.111.2.19 2002/12/29 18:19:53 dillon Exp $ - * $DragonFly: src/sys/sys/vnode.h,v 1.62 2006/07/19 06:08:07 dillon Exp $ + * $DragonFly: src/sys/sys/vnode.h,v 1.63 2006/08/09 22:47:33 dillon Exp $ */ #ifndef _SYS_VNODE_H_ @@ -282,7 +282,6 @@ struct vnode { */ #define VMSC_GETVP 1 #define VMSC_GETVX 2 -#define VMSC_REFVP 3 #define VMSC_NOWAIT 0x10 /* @@ -569,6 +568,7 @@ void vput (struct vnode *vp); void vhold (struct vnode *); void vdrop (struct vnode *); void vref (struct vnode *vp); +void vref_initial (struct vnode *vp, int reactivate); void vrele (struct vnode *vp); void vsetflags (struct vnode *vp, int flags); void vclrflags (struct vnode *vp, int flags); diff --git a/sys/vfs/fdesc/fdesc_vfsops.c b/sys/vfs/fdesc/fdesc_vfsops.c index a5d5cc114a..875887f3b3 100644 --- a/sys/vfs/fdesc/fdesc_vfsops.c +++ b/sys/vfs/fdesc/fdesc_vfsops.c @@ -36,7 +36,7 @@ * @(#)fdesc_vfsops.c 8.4 (Berkeley) 1/21/94 * * $FreeBSD: src/sys/miscfs/fdesc/fdesc_vfsops.c,v 1.22.2.3 2002/08/23 17:42:39 njl Exp $ - * $DragonFly: src/sys/vfs/fdesc/fdesc_vfsops.c,v 1.20 2006/07/18 22:22:15 dillon Exp $ + * $DragonFly: src/sys/vfs/fdesc/fdesc_vfsops.c,v 1.21 2006/08/09 22:47:34 dillon Exp $ */ /* @@ -141,15 +141,16 @@ int fdesc_root(struct mount *mp, struct vnode **vpp) { struct vnode *vp; + int error; /* * Return locked reference to root. */ vp = VFSTOFDESC(mp)->f_root; - vref(vp); - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - *vpp = vp; - return (0); + error = vget(vp, LK_EXCLUSIVE | LK_RETRY); + if (error == 0) + *vpp = vp; + return (error); } static int diff --git a/sys/vfs/nullfs/null_vfsops.c b/sys/vfs/nullfs/null_vfsops.c index 6ced01aeb5..3409145a0e 100644 --- a/sys/vfs/nullfs/null_vfsops.c +++ b/sys/vfs/nullfs/null_vfsops.c @@ -37,7 +37,7 @@ * * @(#)lofs_vfsops.c 1.2 (Berkeley) 6/18/92 * $FreeBSD: src/sys/miscfs/nullfs/null_vfsops.c,v 1.35.2.3 2001/07/26 20:37:11 iedowse Exp $ - * $DragonFly: src/sys/vfs/nullfs/null_vfsops.c,v 1.22 2006/07/18 22:22:15 dillon Exp $ + * $DragonFly: src/sys/vfs/nullfs/null_vfsops.c,v 1.23 2006/08/09 22:47:35 dillon Exp $ */ /* @@ -172,6 +172,7 @@ static int nullfs_root(struct mount *mp, struct vnode **vpp) { struct vnode *vp; + int error; NULLFSDEBUG("nullfs_root(mp = %p, vp = %p)\n", (void *)mp, (void *)MOUNTTONULLMOUNT(mp)->nullm_rootvp); @@ -180,18 +181,10 @@ nullfs_root(struct mount *mp, struct vnode **vpp) * Return locked reference to root. */ vp = MOUNTTONULLMOUNT(mp)->nullm_rootvp; - vref(vp); - -#ifdef NULLFS_DEBUG - if (VOP_ISLOCKED(vp, NULL)) { - Debugger("root vnode is locked.\n"); - vrele(vp); - return (EDEADLK); - } -#endif - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - *vpp = vp; - return 0; + error = vget(vp, LK_EXCLUSIVE | LK_RETRY); + if (error == 0) + *vpp = vp; + return (error); } static int diff --git a/sys/vfs/portal/portal_vfsops.c b/sys/vfs/portal/portal_vfsops.c index dd98c28dbb..a18acd2f78 100644 --- a/sys/vfs/portal/portal_vfsops.c +++ b/sys/vfs/portal/portal_vfsops.c @@ -36,7 +36,7 @@ * @(#)portal_vfsops.c 8.11 (Berkeley) 5/14/95 * * $FreeBSD: src/sys/miscfs/portal/portal_vfsops.c,v 1.26.2.2 2001/07/26 20:37:16 iedowse Exp $ - * $DragonFly: src/sys/vfs/portal/portal_vfsops.c,v 1.21 2006/07/18 22:22:16 dillon Exp $ + * $DragonFly: src/sys/vfs/portal/portal_vfsops.c,v 1.22 2006/08/09 22:47:36 dillon Exp $ */ /* @@ -196,15 +196,16 @@ static int portal_root(struct mount *mp, struct vnode **vpp) { struct vnode *vp; + int error; /* * Return locked reference to root. */ vp = VFSTOPORTAL(mp)->pm_root; - vref(vp); - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - *vpp = vp; - return (0); + error = vget(vp, LK_EXCLUSIVE | LK_RETRY); + if (error == 0) + *vpp = vp; + return (error); } static int -- 2.41.0