Date: Tue, 17 Apr 2012 21:46:59 +0000 (UTC) From: Kirk McKusick <mckusick@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r234400 - in head/sys: kern sys Message-ID: <201204172146.q3HLkxS6000736@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mckusick Date: Tue Apr 17 21:46:59 2012 New Revision: 234400 URL: http://svn.freebsd.org/changeset/base/234400 Log: Drop export of vdestroy() function from kern/vfs_subr.c as it is used only as a helper function in that file. Replace sole call to vbusy() with inline code in vholdl(). Replace sole calls to vfree() and vdestroy() with inline code in vdropl(). The Clang compiler already inlines these functions, so they do not show up in a kernel backtrace which is confusing. Also you cannot set their frame in kgdb which means that it is impossible to view their local variables. So, while the produced code is unchanged, the debugging should be easier. Discussed with: kib MFC after: 2 weeks Modified: head/sys/kern/vfs_subr.c head/sys/sys/vnode.h Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Tue Apr 17 20:54:32 2012 (r234399) +++ head/sys/kern/vfs_subr.c Tue Apr 17 21:46:59 2012 (r234400) @@ -102,12 +102,10 @@ static int flushbuflist(struct bufv *buf int slpflag, int slptimeo); static void syncer_shutdown(void *arg, int howto); static int vtryrecycle(struct vnode *vp); -static void vbusy(struct vnode *vp); static void v_incr_usecount(struct vnode *); static void v_decr_usecount(struct vnode *); static void v_decr_useonly(struct vnode *); static void v_upgrade_usecount(struct vnode *); -static void vfree(struct vnode *); static void vnlru_free(int); static void vgonel(struct vnode *); static void vfs_knllock(void *arg); @@ -118,8 +116,7 @@ static void destroy_vpollinfo(struct vpo /* * Number of vnodes in existence. Increased whenever getnewvnode() - * allocates a new vnode, decreased on vdestroy() called on VI_DOOMed - * vnode. + * allocates a new vnode, decreased in vdropl() for VI_DOOMED vnode. */ static unsigned long numvnodes; @@ -878,46 +875,6 @@ SYSINIT(vnlru, SI_SUB_KTHREAD_UPDATE, SI * Routines having to do with the management of the vnode table. */ -void -vdestroy(struct vnode *vp) -{ - struct bufobj *bo; - - CTR2(KTR_VFS, "%s: vp %p", __func__, vp); - mtx_lock(&vnode_free_list_mtx); - numvnodes--; - mtx_unlock(&vnode_free_list_mtx); - bo = &vp->v_bufobj; - VNASSERT((vp->v_iflag & VI_FREE) == 0, vp, - ("cleaned vnode still on the free list.")); - VNASSERT(vp->v_data == NULL, vp, ("cleaned vnode isn't")); - VNASSERT(vp->v_holdcnt == 0, vp, ("Non-zero hold count")); - VNASSERT(vp->v_usecount == 0, vp, ("Non-zero use count")); - VNASSERT(vp->v_writecount == 0, vp, ("Non-zero write count")); - VNASSERT(bo->bo_numoutput == 0, vp, ("Clean vnode has pending I/O's")); - VNASSERT(bo->bo_clean.bv_cnt == 0, vp, ("cleanbufcnt not 0")); - VNASSERT(bo->bo_clean.bv_root == NULL, vp, ("cleanblkroot not NULL")); - VNASSERT(bo->bo_dirty.bv_cnt == 0, vp, ("dirtybufcnt not 0")); - VNASSERT(bo->bo_dirty.bv_root == NULL, vp, ("dirtyblkroot not NULL")); - VNASSERT(TAILQ_EMPTY(&vp->v_cache_dst), vp, ("vp has namecache dst")); - VNASSERT(LIST_EMPTY(&vp->v_cache_src), vp, ("vp has namecache src")); - VNASSERT(vp->v_cache_dd == NULL, vp, ("vp has namecache for ..")); - VI_UNLOCK(vp); -#ifdef MAC - mac_vnode_destroy(vp); -#endif - if (vp->v_pollinfo != NULL) - destroy_vpollinfo(vp->v_pollinfo); -#ifdef INVARIANTS - /* XXX Elsewhere we can detect an already freed vnode via NULL v_op. */ - vp->v_op = NULL; -#endif - lockdestroy(vp->v_vnlock); - mtx_destroy(&vp->v_interlock); - mtx_destroy(BO_MTX(bo)); - uma_zfree(vnode_zone, vp); -} - /* * Try to recycle a freed vnode. We abort if anyone picks up a reference * before we actually vgone(). This function must be called with the vnode @@ -2346,19 +2303,33 @@ vhold(struct vnode *vp) VI_UNLOCK(vp); } +/* + * Increase the hold count and activate if this is the first reference. + */ void vholdl(struct vnode *vp) { CTR2(KTR_VFS, "%s: vp %p", __func__, vp); vp->v_holdcnt++; - if (VSHOULDBUSY(vp)) - vbusy(vp); + if (!VSHOULDBUSY(vp)) + return; + ASSERT_VI_LOCKED(vp, "vholdl"); + VNASSERT((vp->v_iflag & VI_FREE) != 0, vp, ("vnode not free")); + VNASSERT(vp->v_op != NULL, vp, ("vholdl: vnode already reclaimed.")); + /* + * Remove a vnode from the free list and mark it as in use. + */ + mtx_lock(&vnode_free_list_mtx); + TAILQ_REMOVE(&vnode_free_list, vp, v_freelist); + freevnodes--; + vp->v_iflag &= ~(VI_FREE|VI_AGE); + mtx_unlock(&vnode_free_list_mtx); } /* - * Note that there is one less who cares about this vnode. vdrop() is the - * opposite of vhold(). + * Note that there is one less who cares about this vnode. + * vdrop() is the opposite of vhold(). */ void vdrop(struct vnode *vp) @@ -2370,28 +2341,84 @@ vdrop(struct vnode *vp) /* * Drop the hold count of the vnode. If this is the last reference to - * the vnode we will free it if it has been vgone'd otherwise it is - * placed on the free list. + * the vnode we place it on the free list unless it has been vgone'd + * (marked VI_DOOMED) in which case we will free it. */ void vdropl(struct vnode *vp) { + struct bufobj *bo; ASSERT_VI_LOCKED(vp, "vdropl"); CTR2(KTR_VFS, "%s: vp %p", __func__, vp); if (vp->v_holdcnt <= 0) panic("vdrop: holdcnt %d", vp->v_holdcnt); vp->v_holdcnt--; - if (vp->v_holdcnt == 0) { - if (vp->v_iflag & VI_DOOMED) { - CTR2(KTR_VFS, "%s: destroying the vnode %p", __func__, - vp); - vdestroy(vp); - return; - } else - vfree(vp); + if (vp->v_holdcnt > 0) { + VI_UNLOCK(vp); + return; + } + if ((vp->v_iflag & VI_DOOMED) == 0) { + /* + * Mark a vnode as free, putting it up for recycling. + */ + mtx_lock(&vnode_free_list_mtx); + VNASSERT(vp->v_op != NULL, vp, + ("vdropl: vnode already reclaimed.")); + VNASSERT((vp->v_iflag & VI_FREE) == 0, vp, + ("vnode already free")); + VNASSERT(VSHOULDFREE(vp), vp, + ("vdropl: freeing when we shouldn't")); + VNASSERT((vp->v_iflag & VI_DOOMED) == 0, vp, + ("vdropl: Freeing doomed vnode")); + if (vp->v_iflag & VI_AGE) { + TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist); + } else { + TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_freelist); + } + freevnodes++; + vp->v_iflag &= ~VI_AGE; + vp->v_iflag |= VI_FREE; + mtx_unlock(&vnode_free_list_mtx); + VI_UNLOCK(vp); + return; } + /* + * The vnode has been marked for destruction, so free it. + */ + CTR2(KTR_VFS, "%s: destroying the vnode %p", __func__, vp); + mtx_lock(&vnode_free_list_mtx); + numvnodes--; + mtx_unlock(&vnode_free_list_mtx); + bo = &vp->v_bufobj; + VNASSERT((vp->v_iflag & VI_FREE) == 0, vp, + ("cleaned vnode still on the free list.")); + VNASSERT(vp->v_data == NULL, vp, ("cleaned vnode isn't")); + VNASSERT(vp->v_holdcnt == 0, vp, ("Non-zero hold count")); + VNASSERT(vp->v_usecount == 0, vp, ("Non-zero use count")); + VNASSERT(vp->v_writecount == 0, vp, ("Non-zero write count")); + VNASSERT(bo->bo_numoutput == 0, vp, ("Clean vnode has pending I/O's")); + VNASSERT(bo->bo_clean.bv_cnt == 0, vp, ("cleanbufcnt not 0")); + VNASSERT(bo->bo_clean.bv_root == NULL, vp, ("cleanblkroot not NULL")); + VNASSERT(bo->bo_dirty.bv_cnt == 0, vp, ("dirtybufcnt not 0")); + VNASSERT(bo->bo_dirty.bv_root == NULL, vp, ("dirtyblkroot not NULL")); + VNASSERT(TAILQ_EMPTY(&vp->v_cache_dst), vp, ("vp has namecache dst")); + VNASSERT(LIST_EMPTY(&vp->v_cache_src), vp, ("vp has namecache src")); + VNASSERT(vp->v_cache_dd == NULL, vp, ("vp has namecache for ..")); VI_UNLOCK(vp); +#ifdef MAC + mac_vnode_destroy(vp); +#endif + if (vp->v_pollinfo != NULL) + destroy_vpollinfo(vp->v_pollinfo); +#ifdef INVARIANTS + /* XXX Elsewhere we detect an already freed vnode via NULL v_op. */ + vp->v_op = NULL; +#endif + lockdestroy(vp->v_vnlock); + mtx_destroy(&vp->v_interlock); + mtx_destroy(BO_MTX(bo)); + uma_zfree(vnode_zone, vp); } /* @@ -3298,50 +3325,6 @@ vfs_msync(struct mount *mp, int flags) } } -/* - * Mark a vnode as free, putting it up for recycling. - */ -static void -vfree(struct vnode *vp) -{ - - ASSERT_VI_LOCKED(vp, "vfree"); - mtx_lock(&vnode_free_list_mtx); - VNASSERT(vp->v_op != NULL, vp, ("vfree: vnode already reclaimed.")); - VNASSERT((vp->v_iflag & VI_FREE) == 0, vp, ("vnode already free")); - VNASSERT(VSHOULDFREE(vp), vp, ("vfree: freeing when we shouldn't")); - VNASSERT((vp->v_iflag & VI_DOOMED) == 0, vp, - ("vfree: Freeing doomed vnode")); - CTR2(KTR_VFS, "%s: vp %p", __func__, vp); - if (vp->v_iflag & VI_AGE) { - TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist); - } else { - TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_freelist); - } - freevnodes++; - vp->v_iflag &= ~VI_AGE; - vp->v_iflag |= VI_FREE; - mtx_unlock(&vnode_free_list_mtx); -} - -/* - * Opposite of vfree() - mark a vnode as in use. - */ -static void -vbusy(struct vnode *vp) -{ - ASSERT_VI_LOCKED(vp, "vbusy"); - VNASSERT((vp->v_iflag & VI_FREE) != 0, vp, ("vnode not free")); - VNASSERT(vp->v_op != NULL, vp, ("vbusy: vnode already reclaimed.")); - CTR2(KTR_VFS, "%s: vp %p", __func__, vp); - - mtx_lock(&vnode_free_list_mtx); - TAILQ_REMOVE(&vnode_free_list, vp, v_freelist); - freevnodes--; - vp->v_iflag &= ~(VI_FREE|VI_AGE); - mtx_unlock(&vnode_free_list_mtx); -} - static void destroy_vpollinfo(struct vpollinfo *vi) { Modified: head/sys/sys/vnode.h ============================================================================== --- head/sys/sys/vnode.h Tue Apr 17 20:54:32 2012 (r234399) +++ head/sys/sys/vnode.h Tue Apr 17 21:46:59 2012 (r234400) @@ -626,7 +626,6 @@ void vattr_null(struct vattr *vap); int vcount(struct vnode *vp); void vdrop(struct vnode *); void vdropl(struct vnode *); -void vdestroy(struct vnode *); int vflush(struct mount *mp, int rootrefs, int flags, struct thread *td); int vget(struct vnode *vp, int lockflag, struct thread *td); void vgone(struct vnode *vp);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204172146.q3HLkxS6000736>