From owner-dev-commits-src-main@freebsd.org Tue Jan 12 00:45:00 2021 Return-Path: Delivered-To: dev-commits-src-main@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 6EDE44EB37D; Tue, 12 Jan 2021 00:45:00 +0000 (UTC) (envelope-from git@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 4DFBgJ2Wm6z3QSL; Tue, 12 Jan 2021 00:45:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 3B49824292; Tue, 12 Jan 2021 00:45:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 10C0j0ox040604; Tue, 12 Jan 2021 00:45:00 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10C0j0TD040603; Tue, 12 Jan 2021 00:45:00 GMT (envelope-from git) Date: Tue, 12 Jan 2021 00:45:00 GMT Message-Id: <202101120045.10C0j0TD040603@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kirk McKusick Subject: git: 2d4422e7991a - main - Eliminate lock order reversal in UFS ffs_unmount(). MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mckusick X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2d4422e7991a8d69af395e93bffbc2ea344f7ebe Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jan 2021 00:45:00 -0000 The branch main has been updated by mckusick: URL: https://cgit.FreeBSD.org/src/commit/?id=2d4422e7991a8d69af395e93bffbc2ea344f7ebe commit 2d4422e7991a8d69af395e93bffbc2ea344f7ebe Author: Kirk McKusick AuthorDate: 2021-01-12 00:44:41 +0000 Commit: Kirk McKusick CommitDate: 2021-01-12 00:49:07 +0000 Eliminate lock order reversal in UFS ffs_unmount(). UFS uses a new "mntfs" pseudo file system which provides private device vnodes for a file system to safely access its disk device. The original device vnode is saved in um_odevvp to hold the exclusive lock on the device so that any attempts to open it for writing will fail. But it is otherwise unused and has its BO_NOBUFS flag set to enforce that file systems using mntfs vnodes do not accidentally use the original devfs vnode. When the file system is unmounted, um_odevvp is no longer needed and is released. The lock order reversal happens because device vnodes must be locked before UFS vnodes. During unmount, the root directory vnode lock is held. When when calling vrele() on um_odevvp, vrele() attempts to exclusive lock um_odevvp causing the lock order reversal. The problem is eliminated by doing a non-blocking exclusive lock on um_odevvp which will always succeed since there are no users of um_odevvp. With um_odevvp locked, it can be released using vput which does not attempt to do a blocking exclusive lock request and thus avoids the lock order reversal. Sponsored by: Netflix --- sys/ufs/ffs/ffs_vfsops.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 0b3110c955f5..52b9b860f817 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -1546,7 +1546,14 @@ ffs_unmount(mp, mntflags) BO_UNLOCK(&ump->um_odevvp->v_bufobj); atomic_store_rel_ptr((uintptr_t *)&ump->um_dev->si_mountpt, 0); mntfs_freevp(ump->um_devvp); - vrele(ump->um_odevvp); + /* Avoid LOR in vrele by passing in locked vnode and using vput */ + if (vn_lock(ump->um_odevvp, LK_EXCLUSIVE | LK_NOWAIT) == 0) { + vput(ump->um_odevvp); + } else { + /* This should never happen, see commit message for details */ + printf("ffs_unmount: Unexpected LK_NOWAIT failure\n"); + vrele(ump->um_odevvp); + } dev_rel(ump->um_dev); mtx_destroy(UFS_MTX(ump)); if (mp->mnt_gjprovider != NULL) {