Re: git: 7b6fe2428a97 - main - DEBUG_VFS_LOCKS: use witness if available

From: Jason A. Harmening <jah_at_freebsd.org>
Date: Mon, 10 Apr 2023 14:33:08 UTC 
On Mon, Apr 10, 2023 at 07:58:11AM +0300, Konstantin Belousov wrote:
> On Sun, Apr 09, 2023 at 07:14:35PM -0500, Jason A. Harmening wrote:
> > 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?
> I thought it was obvious, please look at the old assertion conditions,
> now in the #else blocks.  They are completely unable to handle shared
> locks.  In other words, if the current thread owns the lock shared,
> or does not, ASSERT_VOP_UNLOCKED/ASSERT_VOP_LOCKED silently do nothing.

Ah right, sorry.  I keep forgetting we have a separate
assert_vop_elocked() function for checking exclusive locks, while
assert_vop_locked() just sort of gives up when it comes to shared locks.

In any case, I like your proposal for restoring the diagnostics, thank
you for posting it so quickly.

> 
> > 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.
> This is fair, and I noted it in the commit message.  But better working
> asserts are IMO worth it.  Eventually it might be fixed.