From owner-dev-commits-src-main@freebsd.org Sat Jan 16 00:01:08 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 02D5E4F60F6; Sat, 16 Jan 2021 00:01:08 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DHdVq6jrJz4Tfg; Sat, 16 Jan 2021 00:01:07 +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 D958B25630; Sat, 16 Jan 2021 00:01:07 +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 10G017LO062993; Sat, 16 Jan 2021 00:01:07 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10G017rm062992; Sat, 16 Jan 2021 00:01:07 GMT (envelope-from git) Date: Sat, 16 Jan 2021 00:01:07 GMT Message-Id: <202101160001.10G017rm062992@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: 173779b98f10 - main - Eliminate lock order reversal in UFS when unmounting filesystems with snapshots. 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: 173779b98f104d52ca77e104fee5e4abd09bea58 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: Sat, 16 Jan 2021 00:01:08 -0000 The branch main has been updated by mckusick: URL: https://cgit.FreeBSD.org/src/commit/?id=173779b98f104d52ca77e104fee5e4abd09bea58 commit 173779b98f104d52ca77e104fee5e4abd09bea58 Author: Kirk McKusick AuthorDate: 2021-01-16 00:00:17 +0000 Commit: Kirk McKusick 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;