From owner-svn-src-all@freebsd.org Mon Oct 19 19:20:24 2020 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 78F7B43211F; Mon, 19 Oct 2020 19:20:24 +0000 (UTC) (envelope-from kib@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) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 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 4CFRRX2g4Nz43Gm; Mon, 19 Oct 2020 19:20:24 +0000 (UTC) (envelope-from kib@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 37D848746; Mon, 19 Oct 2020 19:20:24 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 09JJKOe0029899; Mon, 19 Oct 2020 19:20:24 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 09JJKOva029898; Mon, 19 Oct 2020 19:20:24 GMT (envelope-from kib@FreeBSD.org) Message-Id: <202010191920.09JJKOva029898@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Mon, 19 Oct 2020 19:20:24 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r366848 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: kib X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 366848 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.33 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: Mon, 19 Oct 2020 19:20:24 -0000 Author: kib Date: Mon Oct 19 19:20:23 2020 New Revision: 366848 URL: https://svnweb.freebsd.org/changeset/base/366848 Log: vgonel(): avoid recursing into VOP_INACTIVE(). It is a common pattern for filesystems' VOP_INACTIVE() implementation to forcibly reclaim the vnode when its state is final. For instance, UFS vnode with zero link count is removed, and since it is inactivated, the last open reference on it is dropped. On the other hand, vnode might get spurious usecount reference for many reasons. If the spurious reference exists while vgonel() checks for active state of the vnode, it would recurse into VOP_INACTIVE(). Fix it by checking and not doing inactivation when vgone() was called from inactive VOP. Reported and tested by: pho Discussed with: mjg 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 Mon Oct 19 18:54:44 2020 (r366847) +++ head/sys/kern/vfs_subr.c Mon Oct 19 19:20:23 2020 (r366848) @@ -1794,6 +1794,8 @@ freevnode(struct vnode *vp) VNASSERT(vp->v_cache_dd == NULL, vp, ("vp has namecache for ..")); VNASSERT(TAILQ_EMPTY(&vp->v_rl.rl_waiters), vp, ("Dangling rangelock waiters")); + VNASSERT((vp->v_iflag & (VI_DOINGINACT | VI_OWEINACT)) == 0, vp, + ("Leaked inactivation")); VI_UNLOCK(vp); #ifdef MAC mac_vnode_destroy(vp); @@ -3803,7 +3805,7 @@ vgonel(struct vnode *vp) struct thread *td; struct mount *mp; vm_object_t object; - bool active, oweinact; + bool active, doinginact, oweinact; ASSERT_VOP_ELOCKED(vp, "vgonel"); ASSERT_VI_LOCKED(vp, "vgonel"); @@ -3825,11 +3827,17 @@ vgonel(struct vnode *vp) vp->v_irflag |= VIRF_DOOMED; /* - * Check to see if the vnode is in use. If so, we have to call - * VOP_CLOSE() and VOP_INACTIVE(). + * Check to see if the vnode is in use. If so, we have to + * call VOP_CLOSE() and VOP_INACTIVE(). + * + * It could be that VOP_INACTIVE() requested reclamation, in + * which case we should avoid recursion, so check + * VI_DOINGINACT. This is not precise but good enough. */ active = vp->v_usecount > 0; oweinact = (vp->v_iflag & VI_OWEINACT) != 0; + doinginact = (vp->v_iflag & VI_DOINGINACT) != 0; + /* * If we need to do inactive VI_OWEINACT will be set. */ @@ -3850,7 +3858,7 @@ vgonel(struct vnode *vp) */ if (active) VOP_CLOSE(vp, FNONBLOCK, NOCRED, td); - if (oweinact || active) { + if ((oweinact || active) && !doinginact) { VI_LOCK(vp); vinactivef(vp); VI_UNLOCK(vp);