Date: Fri, 24 Jan 2020 07:47:44 +0000 (UTC) From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357072 - head/sys/kern Message-ID: <202001240747.00O7li83021323@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mjg Date: Fri Jan 24 07:47:44 2020 New Revision: 357072 URL: https://svnweb.freebsd.org/changeset/base/357072 Log: vfs: allow v_usecount to transition 0->1 without the interlock There is nothing to do but to bump the count even during said transition. There are 2 places which can do it: - vget only does this after locking the vnode, meaning there is no change in contract versus inactive or reclamantion - vref only ever did it with the interlock held which did not protect against either (that is, it would always succeed) VCHR vnodes retain special casing due to the need to maintain dev use count. Reviewed by: jeff, kib Tested by: pho (previous version) Differential Revision: https://reviews.freebsd.org/D23185 Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Fri Jan 24 07:45:59 2020 (r357071) +++ head/sys/kern/vfs_subr.c Fri Jan 24 07:47:44 2020 (r357072) @@ -2826,11 +2826,10 @@ v_decr_devcount(struct vnode *vp) * see doomed vnodes. If inactive processing was delayed in * vput try to do it here. * - * usecount is manipulated using atomics without holding any locks, - * except when transitioning 0->1 in which case the interlock is held. - - * holdcnt is manipulated using atomics without holding any locks, - * except when transitioning 1->0 in which case the interlock is held. + * usecount is manipulated using atomics without holding any locks. + * + * holdcnt can be manipulated using atomics without holding any locks, + * except when transitioning 1<->0, in which case the interlock is held. */ enum vgetstate vget_prep(struct vnode *vp) @@ -2857,10 +2856,46 @@ vget(struct vnode *vp, int flags, struct thread *td) return (vget_finish(vp, flags, vs)); } +static int __noinline +vget_finish_vchr(struct vnode *vp) +{ + + VNASSERT(vp->v_type == VCHR, vp, ("type != VCHR)")); + + /* + * See the comment in vget_finish before usecount bump. + */ + if (refcount_acquire_if_not_zero(&vp->v_usecount)) { +#ifdef INVARIANTS + int old = atomic_fetchadd_int(&vp->v_holdcnt, -1); + VNASSERT(old > 0, vp, ("%s: wrong hold count %d", __func__, old)); +#else + refcount_release(&vp->v_holdcnt); +#endif + return (0); + } + + VI_LOCK(vp); + if (refcount_acquire_if_not_zero(&vp->v_usecount)) { +#ifdef INVARIANTS + int old = atomic_fetchadd_int(&vp->v_holdcnt, -1); + VNASSERT(old > 1, vp, ("%s: wrong hold count %d", __func__, old)); +#else + refcount_release(&vp->v_holdcnt); +#endif + VI_UNLOCK(vp); + return (0); + } + v_incr_devcount(vp); + refcount_acquire(&vp->v_usecount); + VI_UNLOCK(vp); + return (0); +} + int vget_finish(struct vnode *vp, int flags, enum vgetstate vs) { - int error; + int error, old; VNASSERT((flags & LK_TYPE_MASK) != 0, vp, ("%s: invalid lock operation", __func__)); @@ -2890,69 +2925,106 @@ vget_finish(struct vnode *vp, int flags, enum vgetstat return (0); } + if (__predict_false(vp->v_type == VCHR)) + return (vget_finish_vchr(vp)); + /* * We hold the vnode. If the usecount is 0 it will be utilized to keep * the vnode around. Otherwise someone else lended their hold count and * we have to drop ours. */ - if (refcount_acquire_if_not_zero(&vp->v_usecount)) { + old = atomic_fetchadd_int(&vp->v_usecount, 1); + VNASSERT(old >= 0, vp, ("%s: wrong use count %d", __func__, old)); + if (old != 0) { #ifdef INVARIANTS - int old = atomic_fetchadd_int(&vp->v_holdcnt, -1); + old = atomic_fetchadd_int(&vp->v_holdcnt, -1); VNASSERT(old > 1, vp, ("%s: wrong hold count %d", __func__, old)); #else refcount_release(&vp->v_holdcnt); #endif - return (0); } + return (0); +} +/* + * Increase the reference (use) and hold count of a vnode. + * This will also remove the vnode from the free list if it is presently free. + */ +static void __noinline +vref_vchr(struct vnode *vp, bool interlock) +{ + /* - * We don't guarantee that any particular close will - * trigger inactive processing so just make a best effort - * here at preventing a reference to a removed file. If - * we don't succeed no harm is done. - * - * Upgrade our holdcnt to a usecount. + * See the comment in vget_finish before usecount bump. */ - VI_LOCK(vp); - /* - * See the previous section. By the time we get here we may find - * ourselves in the same spot. - */ + if (!interlock) { + if (refcount_acquire_if_not_zero(&vp->v_usecount)) { + VNODE_REFCOUNT_FENCE_ACQ(); + VNASSERT(vp->v_holdcnt > 0, vp, + ("%s: active vnode not held", __func__)); + return; + } + VI_LOCK(vp); + /* + * By the time we get here the vnode might have been doomed, at + * which point the 0->1 use count transition is no longer + * protected by the interlock. Since it can't bounce back to + * VCHR and requires vref semantics, punt it back + */ + if (__predict_false(vp->v_type == VBAD)) { + VI_UNLOCK(vp); + vref(vp); + return; + } + } + VNASSERT(vp->v_type == VCHR, vp, ("type != VCHR)")); if (refcount_acquire_if_not_zero(&vp->v_usecount)) { -#ifdef INVARIANTS - int old = atomic_fetchadd_int(&vp->v_holdcnt, -1); - VNASSERT(old > 1, vp, ("%s: wrong hold count %d", __func__, old)); -#else - refcount_release(&vp->v_holdcnt); -#endif - VI_UNLOCK(vp); - return (0); + VNODE_REFCOUNT_FENCE_ACQ(); + VNASSERT(vp->v_holdcnt > 0, vp, + ("%s: active vnode not held", __func__)); + if (!interlock) + VI_UNLOCK(vp); + return; } + vhold(vp); v_incr_devcount(vp); refcount_acquire(&vp->v_usecount); - VI_UNLOCK(vp); - return (0); + if (!interlock) + VI_UNLOCK(vp); + return; } -/* - * Increase the reference (use) and hold count of a vnode. - * This will also remove the vnode from the free list if it is presently free. - */ void vref(struct vnode *vp) { + int old; - ASSERT_VI_UNLOCKED(vp, __func__); CTR2(KTR_VFS, "%s: vp %p", __func__, vp); + if (__predict_false(vp->v_type == VCHR)) { + vref_vchr(vp, false); + return; + } + if (refcount_acquire_if_not_zero(&vp->v_usecount)) { VNODE_REFCOUNT_FENCE_ACQ(); VNASSERT(vp->v_holdcnt > 0, vp, ("%s: active vnode not held", __func__)); return; } - VI_LOCK(vp); - vrefl(vp); - VI_UNLOCK(vp); + vhold(vp); + /* + * See the comment in vget_finish. + */ + old = atomic_fetchadd_int(&vp->v_usecount, 1); + VNASSERT(old >= 0, vp, ("%s: wrong use count %d", __func__, old)); + if (old != 0) { +#ifdef INVARIANTS + old = atomic_fetchadd_int(&vp->v_holdcnt, -1); + VNASSERT(old > 1, vp, ("%s: wrong hold count %d", __func__, old)); +#else + refcount_release(&vp->v_holdcnt); +#endif + } } void @@ -2961,15 +3033,11 @@ vrefl(struct vnode *vp) ASSERT_VI_LOCKED(vp, __func__); CTR2(KTR_VFS, "%s: vp %p", __func__, vp); - if (refcount_acquire_if_not_zero(&vp->v_usecount)) { - VNODE_REFCOUNT_FENCE_ACQ(); - VNASSERT(vp->v_holdcnt > 0, vp, - ("%s: active vnode not held", __func__)); + if (__predict_false(vp->v_type == VCHR)) { + vref_vchr(vp, true); return; } - vholdl(vp); - v_incr_devcount(vp); - refcount_acquire(&vp->v_usecount); + vref(vp); } void @@ -3147,8 +3215,6 @@ vputx(struct vnode *vp, enum vputx_op func) } break; } - VNASSERT(vp->v_usecount == 0 || (vp->v_iflag & VI_OWEINACT) == 0, vp, - ("vnode with usecount and VI_OWEINACT set")); if (error == 0) { vinactive(vp); if (func != VPUTX_VUNREF)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202001240747.00O7li83021323>