Date: Sun, 9 Apr 2023 19:14:35 -0500 From: "Jason A. Harmening" <jah@freebsd.org> To: Konstantin Belousov <kib@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 7b6fe2428a97 - main - DEBUG_VFS_LOCKS: use witness if available Message-ID: <ZDNU6whgpWktdeiw@corona> In-Reply-To: <202304092134.339LYYY5081080@gitrepo.freebsd.org> References: <202304092134.339LYYY5081080@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Apr 09, 2023 at 09:34:34PM +0000, Konstantin Belousov wrote: > The branch main has been updated by kib: > > URL: https://cgit.FreeBSD.org/src/commit/?id=7b6fe2428a97921e8df882d0a24b87094c37b468 > > commit 7b6fe2428a97921e8df882d0a24b87094c37b468 > Author: Konstantin Belousov <kib@FreeBSD.org> > AuthorDate: 2023-04-08 06:15:00 +0000 > Commit: Konstantin Belousov <kib@FreeBSD.org> > CommitDate: 2023-04-09 21:34:12 +0000 > > DEBUG_VFS_LOCKS: use witness if available > > The assert_vop_locked messages are ignored, and file/line information > is not too useful. Fixing this without changing both witness and VFS > asserts KPIs is not possible. What was the motivation for this change? At first glance it seems regressive. I've at least found the assert messages, as well as the vnode state dumping done by vfs_badlock(), to be really useful for quick debugging of locking issues. This is especially true when optimization makes the backtrace and frame info less than useful; see commit 5a4a83fd0e67a0d7787d2f3e09ef0e5552a1ffb6 for a recent example. > > Reviewed by: markj (previous version) > Tested by: pho > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > Differential revision: https://reviews.freebsd.org/D39464 > --- > sys/kern/vfs_lookup.c | 1 + > sys/kern/vfs_subr.c | 16 +++++++++++++--- > sys/sys/vnode.h | 1 + > 3 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c > index e7f1deea0fae..172aa4b4f576 100644 > --- a/sys/kern/vfs_lookup.c > +++ b/sys/kern/vfs_lookup.c > @@ -162,6 +162,7 @@ nameiinit(void *dummy __unused) > vfs_vector_op_register(&crossmp_vnodeops); > getnewvnode("crossmp", NULL, &crossmp_vnodeops, &vp_crossmp); > vp_crossmp->v_state = VSTATE_CONSTRUCTED; > + vp_crossmp->v_irflag |= VIRF_CROSSMP; > } > SYSINIT(vfs, SI_SUB_VFS, SI_ORDER_SECOND, nameiinit, NULL); > > diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c > index c2b1f71502cd..7e7315f827a1 100644 > --- a/sys/kern/vfs_subr.c > +++ b/sys/kern/vfs_subr.c > @@ -5452,14 +5452,18 @@ assert_vi_unlocked(struct vnode *vp, const char *str) > void > assert_vop_locked(struct vnode *vp, const char *str) > { > - int locked; > - > if (KERNEL_PANICKED() || vp == NULL) > return; > > - locked = VOP_ISLOCKED(vp); > +#ifdef WITNESS > + if ((vp->v_irflag & VIRF_CROSSMP) == 0) > + witness_assert(&vp->v_vnlock->lock_object, LA_LOCKED, > + __FILE__, __LINE__); > +#else > + int locked = VOP_ISLOCKED(vp); > if (locked == 0 || locked == LK_EXCLOTHER) > vfs_badlock("is not locked but should be", str, vp); > +#endif > } > > void > @@ -5468,8 +5472,14 @@ assert_vop_unlocked(struct vnode *vp, const char *str) > if (KERNEL_PANICKED() || vp == NULL) > return; > > +#ifdef WITNESS > + if ((vp->v_irflag & VIRF_CROSSMP) == 0) > + witness_assert(&vp->v_vnlock->lock_object, LA_UNLOCKED, > + __FILE__, __LINE__); > +#else > if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE) > vfs_badlock("is locked but should not be", str, vp); > +#endif > } > > void > diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h > index 5e3f81de0236..fa889826867e 100644 > --- a/sys/sys/vnode.h > +++ b/sys/sys/vnode.h > @@ -228,6 +228,7 @@ _Static_assert(sizeof(struct vnode) <= 448, "vnode size crosses 448 bytes"); > never cleared once set */ > #define VIRF_MOUNTPOINT 0x0004 /* This vnode is mounted on */ > #define VIRF_TEXT_REF 0x0008 /* Executable mappings ref the vnode */ > +#define VIRF_CROSSMP 0x0010 /* Cross-mp vnode, no locking */ > > #define VI_UNUSED0 0x0001 /* unused */ > #define VI_MOUNT 0x0002 /* Mount in progress */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ZDNU6whgpWktdeiw>