git: d032cda0d047 - main - DEBUG_VFS_LOCKS: stop excluding devfs and doomed vnode from asserts
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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 */