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

From: Konstantin Belousov <kostikbel_at_gmail.com>
Date: Mon, 10 Apr 2023 04:58:11 UTC
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.

> 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.