d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove one-time use macros which check for the vnode lifecycle. More,

Browse Source 
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
This commit is contained in:
Konstantin Belousov 2014-07-29 16:42:34 +00:00
parent 9753faf553
commit 634012b917
1 changed files with 25 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
sys/kern/vfs_subr.c
View File

@ -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;