git: 173779b98f10 - main - Eliminate lock order reversal in UFS when unmounting filesystems with snapshots.

Kirk McKusick mckusick at FreeBSD.org
Sat Jan 16 00:01:08 UTC 2021


The branch main has been updated by mckusick:

URL: https://cgit.FreeBSD.org/src/commit/?id=173779b98f104d52ca77e104fee5e4abd09bea58

commit 173779b98f104d52ca77e104fee5e4abd09bea58
Author:     Kirk McKusick <mckusick at FreeBSD.org>
AuthorDate: 2021-01-16 00:00:17 +0000
Commit:     Kirk McKusick <mckusick at 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;


More information about the dev-commits-src-all mailing list