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);