git: d032cda0d047 - main - DEBUG_VFS_LOCKS: stop excluding devfs and doomed vnode from asserts

From: Konstantin Belousov <kib_at_FreeBSD.org>
Date: Fri, 12 Nov 2021 23:04:21 UTC
The branch main has been updated by kib:

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

commit d032cda0d047869139f03cb6d34a18216a166735
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-10-31 21:34:57 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-11-12 23:02:42 +0000

    DEBUG_VFS_LOCKS: stop excluding devfs and doomed vnode from asserts
    
    We do not require devvp vnode locked for metadata io.  It is typically
    not needed indeed, since correctness of the file system using
    corresponding block device ensures that there is no incorrect or racy
    manipulations.
    
    But right now DEBUG_VFS_LOCKS option excludes both character device
    vnodes and completely destroyed (VBAD) vnodes from asserts.  This is not
    too bad since WITNESS still ensures that we do not leak locks.  On the
    other hand, asserts do not mean what they should, to the reader, and
    reliance on them being enforced might result in wrong code.
    
    Note that ASSERT_VOP_LOCKED() still silently accepts NULLVP, I think it
    is worth fixing as well, in the next round.
    
    In collaboration with:  pho
    Reviewed by:    markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential revision:  https://reviews.freebsd.org/D32761
---
 sys/kern/vfs_subr.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 046b74218b80..cd784cd67961 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -5365,13 +5365,6 @@ extattr_check_cred(struct vnode *vp, int attrnamespace, struct ucred *cred,
 }
 
 #ifdef DEBUG_VFS_LOCKS
-/*
- * This only exists to suppress warnings from unlocked specfs accesses.  It is
- * no longer ok to have an unlocked VFS.
- */
-#define	IGNORE_LOCK(vp) (KERNEL_PANICKED() || (vp) == NULL ||		\
-	(vp)->v_type == VCHR ||	(vp)->v_type == VBAD)
-
 int vfs_badlock_ddb = 1;	/* Drop into debugger on violation. */
 SYSCTL_INT(_debug, OID_AUTO, vfs_badlock_ddb, CTLFLAG_RW, &vfs_badlock_ddb, 0,
     "Drop into debugger on lock violation");
@@ -5431,26 +5424,31 @@ assert_vop_locked(struct vnode *vp, const char *str)
 {
 	int locked;
 
-	if (!IGNORE_LOCK(vp)) {
-		locked = VOP_ISLOCKED(vp);
-		if (locked == 0 || locked == LK_EXCLOTHER)
-			vfs_badlock("is not locked but should be", str, vp);
-	}
+	if (KERNEL_PANICKED() || vp == NULL)
+		return;
+
+	locked = VOP_ISLOCKED(vp);
+	if (locked == 0 || locked == LK_EXCLOTHER)
+		vfs_badlock("is not locked but should be", str, vp);
 }
 
 void
 assert_vop_unlocked(struct vnode *vp, const char *str)
 {
+	if (KERNEL_PANICKED() || vp == NULL)
+		return;
 
-	if (!IGNORE_LOCK(vp) && VOP_ISLOCKED(vp) == LK_EXCLUSIVE)
+	if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE)
 		vfs_badlock("is locked but should not be", str, vp);
 }
 
 void
 assert_vop_elocked(struct vnode *vp, const char *str)
 {
+	if (KERNEL_PANICKED() || vp == NULL)
+		return;
 
-	if (!IGNORE_LOCK(vp) && VOP_ISLOCKED(vp) != LK_EXCLUSIVE)
+	if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE)
 		vfs_badlock("is not exclusive locked but should be", str, vp);
 }
 #endif /* DEBUG_VFS_LOCKS */