From owner-svn-src-all@freebsd.org Fri Sep 13 15:49:05 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 8C6BAF4EC9; Fri, 13 Sep 2019 15:49:05 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46VKnF37XHz3LQ1; Fri, 13 Sep 2019 15:49:05 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 505F7961E; Fri, 13 Sep 2019 15:49:05 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x8DFn5w5064110; Fri, 13 Sep 2019 15:49:05 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x8DFn5di064109; Fri, 13 Sep 2019 15:49:05 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <201909131549.x8DFn5di064109@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Fri, 13 Sep 2019 15:49:05 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r352300 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 352300 X-SVN-Commit-Repository: base 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.29 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: Fri, 13 Sep 2019 15:49:05 -0000 Author: mjg Date: Fri Sep 13 15:49:04 2019 New Revision: 352300 URL: https://svnweb.freebsd.org/changeset/base/352300 Log: vfs: release usecount using fetchadd 1. If we release the last usecount we take ownership of the hold count, which means the vnode will remain allocated until we vdrop it. 2. If someone else vrefs they will find no usecount and will proceed to add their own hold count. 3. No code has a problem with v_usecount transitioning to 0 without the interlock These facts combined mean we can fetchadd instead of having a cmpset loop. Reviewed by: kib (previous version) Tested by: pho Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D21528 Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Fri Sep 13 15:49:02 2019 (r352299) +++ head/sys/kern/vfs_subr.c Fri Sep 13 15:49:04 2019 (r352300) @@ -2667,12 +2667,13 @@ v_decr_devcount(struct vnode *vp) * see doomed vnodes. If inactive processing was delayed in * vput try to do it here. * - * Notes on lockless counter manipulation: - * _vhold, vputx and other routines make various decisions based - * on either holdcnt or usecount being 0. As long as either counter - * is not transitioning 0->1 nor 1->0, the manipulation can be done - * with atomic operations. Otherwise the interlock is taken covering - * both the atomic and additional actions. + * Both holdcnt and usecount can be manipulated using atomics without holding + * any locks except in these cases which require the vnode interlock: + * holdcnt: 1->0 and 0->1 + * usecount: 0->1 + * + * usecount is permitted to transition 1->0 without the interlock because + * vnode is kept live by holdcnt. */ static enum vgetstate _vget_prep(struct vnode *vp, bool interlock) @@ -2784,6 +2785,29 @@ vget_finish(struct vnode *vp, int flags, enum vgetstat * Upgrade our holdcnt to a usecount. */ VI_LOCK(vp); + /* + * See the previous section. By the time we get here we may find + * ourselves in the same spot. + */ + if (vp->v_type != VCHR) { + if (refcount_acquire_if_not_zero(&vp->v_usecount)) { +#ifdef INVARIANTS + int old = atomic_fetchadd_int(&vp->v_holdcnt, -1) - 1; + VNASSERT(old > 0, vp, ("%s: wrong hold count", __func__)); +#else + refcount_release(&vp->v_holdcnt); +#endif + VNODE_REFCOUNT_FENCE_ACQ(); + VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp, + ("%s: vnode with usecount and VI_OWEINACT set", + __func__)); + VI_UNLOCK(vp); + return (0); + } + } else { + if (vp->v_usecount > 0) + refcount_release(&vp->v_holdcnt); + } if ((vp->v_iflag & VI_OWEINACT) == 0) { oweinact = 0; } else { @@ -2791,8 +2815,6 @@ vget_finish(struct vnode *vp, int flags, enum vgetstat vp->v_iflag &= ~VI_OWEINACT; VNODE_REFCOUNT_FENCE_REL(); } - if (vp->v_usecount > 0) - refcount_release(&vp->v_holdcnt); v_incr_devcount(vp); refcount_acquire(&vp->v_usecount); if (oweinact && VOP_ISLOCKED(vp) == LK_EXCLUSIVE && @@ -2832,6 +2854,15 @@ vrefl(struct vnode *vp) ASSERT_VI_LOCKED(vp, __func__); CTR2(KTR_VFS, "%s: vp %p", __func__, vp); + if (vp->v_type != VCHR && + refcount_acquire_if_not_zero(&vp->v_usecount)) { + VNODE_REFCOUNT_FENCE_ACQ(); + VNASSERT(vp->v_holdcnt > 0, vp, + ("%s: active vnode not held", __func__)); + VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp, + ("%s: vnode with usecount and VI_OWEINACT set", __func__)); + return; + } if (vp->v_usecount == 0) vholdl(vp); if ((vp->v_iflag & VI_OWEINACT) != 0) { @@ -2917,21 +2948,35 @@ vputx(struct vnode *vp, int func) if (func == VPUTX_VPUT) VOP_UNLOCK(vp, 0); - if (vp->v_type != VCHR && - refcount_release_if_not_last(&vp->v_usecount)) - return; - - VI_LOCK(vp); - /* * We want to hold the vnode until the inactive finishes to * prevent vgone() races. We drop the use count here and the * hold count below when we're done. */ - v_decr_devcount(vp); - if (!refcount_release(&vp->v_usecount)) { - VI_UNLOCK(vp); - return; + if (vp->v_type != VCHR) { + /* + * If we release the last usecount we take ownership of the hold + * count which provides liveness of the vnode, in which case we + * have to vdrop. + */ + if (!refcount_release(&vp->v_usecount)) + return; + VI_LOCK(vp); + /* + * By the time we got here someone else might have transitioned + * the count back to > 0. + */ + if (vp->v_usecount > 0) { + vdropl(vp); + return; + } + } else { + VI_LOCK(vp); + v_decr_devcount(vp); + if (!refcount_release(&vp->v_usecount)) { + VI_UNLOCK(vp); + return; + } } if (vp->v_iflag & VI_DOINGINACT) { vdropl(vp);