On 15/02/21(Mon) 16:58, Visa Hankala wrote:
> On Mon, Feb 15, 2021 at 11:37:45AM +0100, Martin Pieuchot wrote:
> > Diagnostic function rw_enter_diag() should be called before
> > WITNESS_CHECKORDER() to have proper locking/debugging information.
> >
> > In the case of 'locking against myself' it is currently impossible
> > to know where the lock has been previously acquired. Diff below fixes
> > that, ok?
>
> Based on this description alone, it is not clear to me what exactly
> gets solved. Doesn't the code reach rw_enter_diag() inside the loop
> when trying to recurse on the rwlock?
It does but before reaching WITNESS_CHECKORDER(). So if a panic is
triggered "show all locks" will point to the previous place where the
same lock has been acquired.
Currently it points to the place triggering the panic which doesn't
help figuring the problem.
> Does this change have implications with (panicstr || db_active)?
Indeed, updated diff below.
Index: kern/kern_rwlock.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.47
diff -u -p -r1.47 kern_rwlock.c
--- kern/kern_rwlock.c 8 Feb 2021 08:18:45 -0000 1.47
+++ kern/kern_rwlock.c 16 Feb 2021 09:04:04 -0000
@@ -167,6 +167,9 @@ rw_exit_write(struct rwlock *rwl)
static void
rw_enter_diag(struct rwlock *rwl, int flags)
{
+ if (panicstr || db_active)
+ return;
+
switch (flags & RW_OPMASK) {
case RW_WRITE:
case RW_READ:
@@ -237,7 +240,11 @@ rw_enter(struct rwlock *rwl, int flags)
int error, prio;
#ifdef WITNESS
int lop_flags;
+#endif
+
+ rw_enter_diag(rwl, flags);
+#ifdef WITNESS
lop_flags = LOP_NEWORDER;
if (flags & RW_WRITE)
lop_flags |= LOP_EXCLUSIVE;
@@ -270,8 +277,6 @@ retry:
continue;
}
#endif
-
- rw_enter_diag(rwl, flags);
if (flags & RW_NOSLEEP)
return (EBUSY);
