kgess-mirror/freebsd-src
1
0
Fork 0
mirror of https://git.freebsd.org/src.git synced 2026-01-16 23:02:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

vfs: Move DEBUG_VFS_LOCKS checks to INVARIANTS

Browse source 
It is easy to forget to configure DEBUG_VFS_LOCKS, and when one does, no
vnode lock assertions are checked when INVARIANTS is configured, so bugs
can arise.  This has happened to me more than once, and the overhead
over DEBUG_VFS_LOCKS does not appear to be high enough to prohibit
folding it into INVARIANTS, so let's do that.

The change makes vnode lock assertions useful in plain INVARIANTS
kernels, and guards VOP debug routines on INVARIANTS rather than
DEBUG_VFS_LOCKS.  Further, invariants are now checked by plain
assertions rather than having various sysctls to finely control what
happens the checks fail.  The extra complexity didn't seem particularly
useful and is at odds with how we handle debugging most everywhere else.

Reviewed by:	kib
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D51402
This commit is contained in:
Mark Johnston 2025-07-17 21:54:32 +00:00
parent 22afc76786
commit 3bd8fab241
3 changed files with 37 additions and 80 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

103
sys/kern/vfs_subr.c
View file

@ -97,10 +97,6 @@
#include <vm/vnode_pager.h>
#include <vm/uma.h>
#if defined(DEBUG_VFS_LOCKS) && (!defined(INVARIANTS) || !defined(WITNESS))
#error DEBUG_VFS_LOCKS requires INVARIANTS and WITNESS
#endif
#ifdef DDB
#include <ddb/ddb.h>
#endif
@ -5736,102 +5732,69 @@ extattr_check_cred(struct vnode *vp, int attrnamespace, struct ucred *cred,
}
}
#ifdef DEBUG_VFS_LOCKS
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");
int vfs_badlock_mutex = 1; /* Check for interlock across VOPs. */
SYSCTL_INT(_debug, OID_AUTO, vfs_badlock_mutex, CTLFLAG_RW, &vfs_badlock_mutex,
0, "Check for interlock across VOPs");
int vfs_badlock_print = 1; /* Print lock violations. */
SYSCTL_INT(_debug, OID_AUTO, vfs_badlock_print, CTLFLAG_RW, &vfs_badlock_print,
0, "Print lock violations");
int vfs_badlock_vnode = 1; /* Print vnode details on lock violations. */
SYSCTL_INT(_debug, OID_AUTO, vfs_badlock_vnode, CTLFLAG_RW, &vfs_badlock_vnode,
0, "Print vnode details on lock violations");
#ifdef KDB
int vfs_badlock_backtrace = 1; /* Print backtrace at lock violations. */
SYSCTL_INT(_debug, OID_AUTO, vfs_badlock_backtrace, CTLFLAG_RW,
&vfs_badlock_backtrace, 0, "Print backtrace at lock violations");
#endif
static void
vfs_badlock(const char *msg, const char *str, struct vnode *vp)
{
#ifdef KDB
if (vfs_badlock_backtrace)
kdb_backtrace();
#endif
if (vfs_badlock_vnode)
vn_printf(vp, "vnode ");
if (vfs_badlock_print)
printf("%s: %p %s\n", str, (void *)vp, msg);
if (vfs_badlock_ddb)
kdb_enter(KDB_WHY_VFSLOCK, "lock violation");
}
#ifdef INVARIANTS
void
assert_vi_locked(struct vnode *vp, const char *str)
{
if (vfs_badlock_mutex && !mtx_owned(VI_MTX(vp)))
vfs_badlock("interlock is not locked but should be", str, vp);
VNASSERT(mtx_owned(VI_MTX(vp)), vp,
("%s: vnode interlock is not locked but should be", str));
}
void
assert_vi_unlocked(struct vnode *vp, const char *str)
{
if (vfs_badlock_mutex && mtx_owned(VI_MTX(vp)))
vfs_badlock("interlock is locked but should not be", str, vp);
VNASSERT(!mtx_owned(VI_MTX(vp)), vp,
("%s: vnode interlock is locked but should not be", str));
}
void
assert_vop_locked(struct vnode *vp, const char *str)
{
bool locked;
if (KERNEL_PANICKED() || vp == NULL)
return;
#ifdef WITNESS
if ((vp->v_irflag & VIRF_CROSSMP) == 0 &&
witness_is_owned(&vp->v_vnlock->lock_object) == -1)
locked = !((vp->v_irflag & VIRF_CROSSMP) == 0 &&
witness_is_owned(&vp->v_vnlock->lock_object) == -1);
#else
int locked = VOP_ISLOCKED(vp);
if (locked == 0 || locked == LK_EXCLOTHER)
int state = VOP_ISLOCKED(vp);
locked = state != 0 && state != LK_EXCLOTHER;
#endif
vfs_badlock("is not locked but should be", str, vp);
VNASSERT(locked, vp, ("%s: vnode is not locked but should be", str));
}
void
assert_vop_unlocked(struct vnode *vp, const char *str)
{
bool locked;
if (KERNEL_PANICKED() || vp == NULL)
return;
#ifdef WITNESS
if ((vp->v_irflag & VIRF_CROSSMP) == 0 &&
witness_is_owned(&vp->v_vnlock->lock_object) == 1)
locked = (vp->v_irflag & VIRF_CROSSMP) == 0 &&
witness_is_owned(&vp->v_vnlock->lock_object) == 1;
#else
if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE)
locked = VOP_ISLOCKED(vp) == LK_EXCLUSIVE;
#endif
vfs_badlock("is locked but should not be", str, vp);
VNASSERT(!locked, vp, ("%s: vnode is locked but should not be", str));
}
void
assert_vop_elocked(struct vnode *vp, const char *str)
{
bool locked;
if (KERNEL_PANICKED() || vp == NULL)
return;
if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE)
vfs_badlock("is not exclusive locked but should be", str, vp);
locked = VOP_ISLOCKED(vp) == LK_EXCLUSIVE;
VNASSERT(locked, vp,
("%s: vnode is not exclusive locked but should be", str));
}
#endif /* DEBUG_VFS_LOCKS */
#endif /* INVARIANTS */
void
vop_rename_fail(struct vop_rename_args *ap)
@ -5852,7 +5815,7 @@ vop_rename_pre(void *ap)
{
struct vop_rename_args *a = ap;
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
struct mount *tmp;
if (a->a_tvp)
@ -5895,7 +5858,7 @@ vop_rename_pre(void *ap)
vhold(a->a_tvp);
}
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
void
vop_fplookup_vexec_debugpre(void *ap __unused)
{
@ -6008,13 +5971,7 @@ vop_strategy_debugpre(void *ap)
if ((bp->b_flags & B_CLUSTER) != 0)
return;
if (!KERNEL_PANICKED() && !BUF_ISLOCKED(bp)) {
if (vfs_badlock_print)
printf(
"VOP_STRATEGY: bp is not locked but should be\n");
if (vfs_badlock_ddb)
kdb_enter(KDB_WHY_VFSLOCK, "lock violation");
}
BUF_ASSERT_LOCKED(bp);
}
void
@ -6063,7 +6020,7 @@ vop_need_inactive_debugpost(void *ap, int rc)
ASSERT_VI_LOCKED(a->a_vp, "VOP_NEED_INACTIVE");
}
#endif
#endif /* INVARIANTS */
void
vop_allocate_post(void *ap, int rc)
@ -6229,7 +6186,7 @@ vop_mkdir_post(void *ap, int rc)
}
}
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
void
vop_mkdir_debugpost(void *ap, int rc)
{
@ -6677,7 +6634,7 @@ vfs_knlunlock(void *arg)
static void
vfs_knl_assert_lock(void *arg, int what)
{
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
struct vnode *vp = arg;
if (what == LA_LOCKED)

8
sys/sys/vnode.h
View file

@ -538,7 +538,7 @@ extern struct vnodeop_desc *vnodeop_descs[];
#define VOPARG_OFFSETTO(s_type, s_offset, struct_p) \
((s_type)(((char*)(struct_p)) + (s_offset)))
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
/*
* Support code to aid in debugging VFS locking problems. Not totally
* reliable since if the thread sleeps between changing the lock
@ -572,7 +572,7 @@ void assert_vop_unlocked(struct vnode *vp, const char *str);
VNPASS(!seqc_in_modify(_vp->v_seqc), _vp); \
} while (0)
#else /* !DEBUG_VFS_LOCKS */
#else /* !INVARIANTS */
#define ASSERT_VI_LOCKED(vp, str) ((void)0)
#define ASSERT_VI_UNLOCKED(vp, str) ((void)0)
@ -583,7 +583,7 @@ void assert_vop_unlocked(struct vnode *vp, const char *str);
#define ASSERT_VOP_IN_SEQC(vp) ((void)0)
#define ASSERT_VOP_NOT_IN_SEQC(vp) ((void)0)
#endif /* DEBUG_VFS_LOCKS */
#endif /* INVARIANTS */
/*
* This call works for vnodes in the kernel.
@ -956,7 +956,7 @@ void vop_symlink_pre(void *a);
void vop_symlink_post(void *a, int rc);
int vop_sigdefer(struct vop_vector *vop, struct vop_generic_args *a);
#ifdef DEBUG_VFS_LOCKS
#ifdef INVARIANTS
void vop_fdatasync_debugpre(void *a);
void vop_fdatasync_debugpost(void *a, int rc);
void vop_fplookup_vexec_debugpre(void *a);

6
sys/tools/vnode_if.awk
View file

@ -86,7 +86,7 @@ function add_debug_code(name, arg, pos, ind)
function add_debugpre(name)
{
if (lockdata[name, "debugpre"]) {
printc("#ifdef DEBUG_VFS_LOCKS");
printc("#ifdef INVARIANTS");
printc("\t"lockdata[name, "debugpre"]"(a);");
printc("#endif");
}
@ -95,7 +95,7 @@ function add_debugpre(name)
function add_debugpost(name)
{
if (lockdata[name, "debugpost"]) {
printc("#ifdef DEBUG_VFS_LOCKS");
printc("#ifdef INVARIANTS");
printc("\t"lockdata[name, "debugpost"]"(a, rc);");
printc("#endif");
}
@ -340,7 +340,7 @@ while ((getline < srcfile) > 0) {
for (i = 0; i < numargs; ++i)
printh("\ta.a_" args[i] " = " args[i] ";");
if (can_inline(name)) {
printh("\n#if !defined(DEBUG_VFS_LOCKS) && !defined(INVARIANTS) && !defined(KTR)");
printh("\n#if !defined(INVARIANTS) && !defined(KTR)");
printh("\tif (!SDT_PROBES_ENABLED())");
printh("\t\treturn (" args[0]"->v_op->"name"(&a));");
printh("\telse");