Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 16 Jan 2021 00:01:07 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: 173779b98f10 - main - Eliminate lock order reversal in UFS when unmounting filesystems with snapshots.
Message-ID:  <202101160001.10G017rm062992@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=173779b98f104d52ca77e104fee5e4abd09bea58

commit 173779b98f104d52ca77e104fee5e4abd09bea58
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2021-01-16 00:00:17 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2021-01-16 00:03:01 +0000

    Eliminate lock order reversal in UFS when unmounting filesystems
    with snapshots.
    
    Each vnode has an embedded lock that controls access to its contents.
    However vnodes describing a UFS snapshot all share a single snapshot
    lock to coordinate their access and update.  As part of mounting a
    UFS filesystem with snapshots, each of the vnodes describing a
    snapshot has its individual lock replaced with the snapshot lock.
    When the filesystem is unmounted the vnode's original lock is
    returned replacing the snapshot lock.
    
    The lock order reversal happens because vnode locks must be acquired
    before snapshot locks. When unmounting we must lock both the snapshot
    lock and the vnode lock before swapping them so that the vnode will
    be continuously locked during the swap. For each vnode representing
    a snapshot, we must first acquire the snapshot lock to ensure
    exclusive access to it and its original lock.  We then face a lock
    order reversal when we try to acquire the original vnode lock. The
    problem is eliminated by doing a non-blocking exclusive lock on the
    original lock which will always succeed since there are no users
    of that lock.
    
    Sponsored by: Netflix
---
 sys/ufs/ffs/ffs_snapshot.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sys/ufs/ffs/ffs_snapshot.c b/sys/ufs/ffs/ffs_snapshot.c
index 201dbf6000de..32dc47653d18 100644
--- a/sys/ufs/ffs/ffs_snapshot.c
+++ b/sys/ufs/ffs/ffs_snapshot.c
@@ -2142,7 +2142,16 @@ ffs_snapshot_unmount(mp)
 		xp->i_nextsnap.tqe_prev = 0;
 		lockmgr(&sn->sn_lock, LK_INTERLOCK | LK_EXCLUSIVE,
 		    VI_MTX(devvp));
-		lockmgr(&vp->v_lock, LK_EXCLUSIVE, NULL);
+		/*
+		 * Avoid LOR with above snapshot lock. The LK_NOWAIT should
+		 * never fail as the lock is currently unused. Rather than
+		 * panic, we recover by doing the blocking lock.
+		 */
+		if (lockmgr(&vp->v_lock, LK_EXCLUSIVE | LK_NOWAIT, NULL) != 0) {
+			printf("ffs_snapshot_unmount: Unexpected LK_NOWAIT "
+			    "failure\n");
+			lockmgr(&vp->v_lock, LK_EXCLUSIVE, NULL);
+		}
 		KASSERT(vp->v_vnlock == &sn->sn_lock,
 		("ffs_snapshot_unmount: lost lock mutation")); 
 		vp->v_vnlock = &vp->v_lock;



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