Re: Old LOR between devfs & devfsmount resurfacing?

On Thursday 07 February 2008 09:11:46 am Attilio Rao wrote:
> 2008/2/7, Kostik Belousov <kostikbel_at_gmail.com>:
> > On Thu, Feb 07, 2008 at 01:21:09PM +0100, Attilio Rao wrote:
> > > 2008/2/7, Kostik Belousov <kostikbel_at_gmail.com>:
> > > > On Thu, Feb 07, 2008 at 12:04:28PM +0100, Attilio Rao wrote:
> > > >  > 2008/2/7, Kostik Belousov <kostikbel_at_gmail.com>:
> > > >  > > On Thu, Feb 07, 2008 at 11:16:08AM +0100, Attilio Rao wrote:
> > > >  > >  > 2008/2/7, Kostik Belousov <kostikbel_at_gmail.com>:
> > > >  > >  > > On Wed, Feb 06, 2008 at 11:11:06AM -0800, Marcel Moolenaar 
wrote:
> > > >  > >  > >  > All,
> > > >  > >  > >  >
> > > >  > >  > >  > I just ran into the following LOR after upgrading my 
PowerPC box:
> > > >  > >  > >  >
> > > >  > >  > >  > lock order reversal:
> > > >  > >  > >  >  1st 0xdbee94 devfs (devfs) 
_at_ /nfs/freebsd/8.x/src/sys/kern/
> > > >  > >  > >  > vfs_subr.c:2061
> > > >  > >  > >  >  2nd 0xdfb014 devfsmount (devfsmount) 
_at_ /nfs/freebsd/8.x/src/sys/fs/
> > > >  > >  > >  > devfs/devfs_vnops.c:201
> > > >  > >  > >  > KDB: stack backtrace:
> > > >  > >  > >  > 0xdc0febc8: at kdb_backtrace+0x4c
> > > >  > >  > >  > 0xdc0febd8: at witness_checkorder+0x704
> > > >  > >  > >  > 0xdc0fec28: at _sx_xlock+0x8c
> > > >  > >  > >  > 0xdc0fec48: at devfs_allocv+0x138
> > > >  > >  > >  > 0xdc0fec88: at devfs_root+0x5c
> > > >  > >  > >  > 0xdc0fecb8: at set_rootvnode+0x44
> > > >  > >  > >  > 0xdc0fece8: at vfs_mountroot+0x344
> > > >  > >  > >  > 0xdc0fed48: at start_init+0x88
> > > >  > >  > >  > 0xdc0feda8: at fork_exit+0xb4
> > > >  > >  > >  > 0xdc0fedc8: at fork_trampoline+0xc
> > > >  > >  > >  > KDB: enter: witness_checkorder
> > > >  > >  > >  > [thread pid 1 tid 100001 ]
> > > >  > >  > >  > Stopped at      0x28ee68:       addi    r0, r0, 0x0
> > > >  > >  > >  >
> > > >  > >  > >  > It seems that this is a LOR reported in 2006 and fixed
> > > >  > >  > >  > in 2006 as well. Do other people see this too, or should
> > > >  > >  > >  > I suspect my sources?
> > > >  > >  > >
> > > >  > >  > >
> > > >  > >  > > I believe this is a false positive, caused by the way the 
witness works.
> > > >  > >  > >  Attilio recently added the witness support for the lockmgr, 
that caused
> > > >  > >  > >  this and at least two more LORs to be printed on startup.
> > > >  > >  > >
> > > >  > >  > >  Correct lock order is devfs vnode -> devfs mount sx lock. 
When
> > > >  > >  > >  allocating new devfs vnode, see devfs_allocv(), the newly 
created
> > > >  > >  > >  vnode is locked while devfs mount lock already held (see 
line 250 of
> > > >  > >  > >  fs/devfs/devfs_vnops.c). Nonetheless, this cannot cause 
deadlock since
> > > >  > >  > >  no other thread can find the new vnode, and thus perform 
the other lock
> > > >  > >  > >  order for this vnode lock.
> > > >  > >  > >
> > > >  > >  > >  The fix is to shut the witness in this particular case. 
Attilio, how to
> > > >  > >  > >  do this ?
> > > >  > >  >
> > > >  > >  > Just add LK_NOWITNESS for one of the lock involved in the 
lockinit().
> > > >  > >
> > > >  > >
> > > >  > > Then, we loss the useful reports of the actual LORs later, isn't 
it ?
> > > >  >
> > > >  > Another solution would be to rewamp BLESSING option which allow to
> > > >  > 'bless' some LORs.
> > > >  > jhb and me, btw, didn't want to enable it because it could lead 
some
> > > >  > less experienced developer to hide LORs under this label and this 
is
> > > >  > something we want to avoid.
> > > >
> > > >
> > > > This LOR shall not be ignored globally. When real, it caused the 
easily
> > > >  reproducable lockup of the machine.
> > > >
> > > >  It would be better to introduce some lockmgr flag to ignore _this_ 
locking.
> > >
> > > flag to pass where?
> > To the lockmgr itself at the point of aquisition, like
> >        lockmgr(&lk, LK_EXCLUSIVE | LK_INTERLOCK | LK_NOWARN, 
&interlk, ...);
> 
> No, I really want a general WITNESS support for this (as I also think
> that having something more fine grained than BLESSING will break all
> concerns jhb and me are considering now).
> A simple way to do it would mean hard-coding file and line in a
> witness table. While file is ok, line makes trouble so we should find
> an alternative way to do this. Otherwise we can consider skiping
> checks for a whole function, this should be not so difficult to
> achive.
> 
> I need to think more about this.

I think allowing a flag is fine, just as you can specify MTX_QUIET to quiet 
KTR logs in specific mtx_lock() instances.  You would specify LK_NOWITNESS or 
some such and have it not do a witness_checkorder() in that case.

-- 
John Baldwin