From owner-svn-src-all@FreeBSD.ORG Tue Jul 29 16:42:35 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id EFBD5FCD; Tue, 29 Jul 2014 16:42:34 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DCE2726F1; Tue, 29 Jul 2014 16:42:34 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id s6TGgYwQ089548; Tue, 29 Jul 2014 16:42:34 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id s6TGgY8t089547; Tue, 29 Jul 2014 16:42:34 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201407291642.s6TGgY8t089547@svn.freebsd.org> From: Konstantin Belousov Date: Tue, 29 Jul 2014 16:42:34 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r269244 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jul 2014 16:42:35 -0000 Author: kib Date: Tue Jul 29 16:42:34 2014 New Revision: 269244 URL: http://svnweb.freebsd.org/changeset/base/269244 Log: Remove one-time use macros which check for the vnode lifecycle. More, some parts of the checks are in fact redundand in the surrounding code, and it is more clear what the conditions are by direct testing of the flags. Two of the three macros were only used in assertions. In vnlru_free(), all relevant parts of vholdl() were already inlined, except the increment of v_holdcnt itself. Do not call vholdl() to do the increment as well, this allows to make assertions in vholdl()/vhold() more strict. In v_incr_usecount(), call vholdl() before incrementing other ref counters. The change is no-op, but it makes less surprising to see the vnode state in debugger if interrupted inside v_incr_usecount(). Tested by: pho Sponsored by: The FreeBSD Foundation MFC after: 1 week Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Tue Jul 29 15:01:29 2014 (r269243) +++ head/sys/kern/vfs_subr.c Tue Jul 29 16:42:34 2014 (r269244) @@ -275,14 +275,6 @@ static int vnlru_nowhere; SYSCTL_INT(_debug, OID_AUTO, vnlru_nowhere, CTLFLAG_RW, &vnlru_nowhere, 0, "Number of times the vnlru process ran without success"); -/* - * Macros to control when a vnode is freed and recycled. All require - * the vnode interlock. - */ -#define VCANRECYCLE(vp) (((vp)->v_iflag & VI_FREE) && !(vp)->v_holdcnt) -#define VSHOULDFREE(vp) (!((vp)->v_iflag & VI_FREE) && !(vp)->v_holdcnt) -#define VSHOULDBUSY(vp) (((vp)->v_iflag & VI_FREE) && (vp)->v_holdcnt) - /* Shift count for (uintptr_t)vp to initialize vp->v_hash. */ static int vnsz2log; @@ -849,11 +841,21 @@ vnlru_free(int count) TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_actfreelist); continue; } - VNASSERT(VCANRECYCLE(vp), vp, - ("vp inconsistent on freelist")); + VNASSERT((vp->v_iflag & VI_FREE) != 0 && vp->v_holdcnt == 0, + vp, ("vp inconsistent on freelist")); + + /* + * The clear of VI_FREE prevents activation of the + * vnode. There is no sense in putting the vnode on + * the mount point active list, only to remove it + * later during recycling. Inline the relevant part + * of vholdl(), to avoid triggering assertions or + * activating. + */ freevnodes--; vp->v_iflag &= ~VI_FREE; - vholdl(vp); + vp->v_holdcnt++; + mtx_unlock(&vnode_free_list_mtx); VI_UNLOCK(vp); vtryrecycle(vp); @@ -2042,13 +2044,13 @@ v_incr_usecount(struct vnode *vp) { CTR2(KTR_VFS, "%s: vp %p", __func__, vp); + vholdl(vp); vp->v_usecount++; if (vp->v_type == VCHR && vp->v_rdev != NULL) { dev_lock(); vp->v_rdev->si_usecount++; dev_unlock(); } - vholdl(vp); } /* @@ -2325,11 +2327,18 @@ vholdl(struct vnode *vp) struct mount *mp; CTR2(KTR_VFS, "%s: vp %p", __func__, vp); +#ifdef INVARIANTS + /* getnewvnode() calls v_incr_usecount() without holding interlock. */ + if (vp->v_type != VNON || vp->v_data != NULL) { + ASSERT_VI_LOCKED(vp, "vholdl"); + VNASSERT(vp->v_holdcnt > 0 || (vp->v_iflag & VI_FREE) != 0, + vp, ("vholdl: free vnode is held")); + } +#endif vp->v_holdcnt++; - if (!VSHOULDBUSY(vp)) + if ((vp->v_iflag & VI_FREE) == 0) return; - ASSERT_VI_LOCKED(vp, "vholdl"); - VNASSERT((vp->v_iflag & VI_FREE) != 0, vp, ("vnode not free")); + VNASSERT(vp->v_holdcnt == 1, vp, ("vholdl: wrong hold count")); VNASSERT(vp->v_op != NULL, vp, ("vholdl: vnode already reclaimed.")); /* * Remove a vnode from the free list, mark it as in use, @@ -2392,7 +2401,7 @@ vdropl(struct vnode *vp) ("vdropl: vnode already reclaimed.")); VNASSERT((vp->v_iflag & VI_FREE) == 0, vp, ("vnode already free")); - VNASSERT(VSHOULDFREE(vp), vp, + VNASSERT(vp->v_holdcnt == 0, vp, ("vdropl: freeing when we shouldn't")); active = vp->v_iflag & VI_ACTIVE; vp->v_iflag &= ~VI_ACTIVE;