Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Jan 2021 00:45:00 GMT
From:      Kirk McKusick <mckusick@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 2d4422e7991a - main - Eliminate lock order reversal in UFS ffs_unmount().
Message-ID:  <202101120045.10C0j0TD040603@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by mckusick:

URL: https://cgit.FreeBSD.org/src/commit/?id=2d4422e7991a8d69af395e93bffbc2ea344f7ebe

commit 2d4422e7991a8d69af395e93bffbc2ea344f7ebe
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2021-01-12 00:44:41 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
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) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101120045.10C0j0TD040603>