Re: Replace lockmgr for vnodes

[David Holland <dholland-tech%netbsd.org@localhost>]

[lots of quoting because I let this sit for a month]
On Sat, Feb 02, 2008 at 04:00:48PM +0900, YAMAMOTO Takashi wrote:
 > > > > No, they don't. In the long run I'm intending to fix at least the
 > > > > vnode locks, because IMO they should be interruptible, but that'll
 > > > > likely be a slow process.
 > > > 
 > > > i believe vnode locks should not be interruptible.
 > > 
 > > ok then...
 > > 
 > > My reasoning is that all sleeps should be interruptible where
 > > reasonably possible, because it increases the chance of being able to
 > > recover in a semi-controlled fashion from a big deadlock or other
 > > similar situation. This is particularly true of locks that might be
 > > held during I/Os that might go off and never come back, and also
 > > particularly true of locks that are manipulated by bodgy code that has
 > > a record of causing trouble. Both these cases apply to vnode locks.
 > > Meanwhile, the worst case for recovery is rename, and I've just been
 > > over that code and I don't anticipate serious problems.
 > > 
 > > Your turn :-)
 > 
 > - what making troubles in your examples are I/Os which never come back
 >  and bodgy code.  they should be fixed instead of locks.
 > 
 > - interruptible locks are actually harder to manipulate correctly,
 >  esp. for bodgy code. :)

That's true; but finding every deadlock is Hard(TM), while checking
every call to vn_lock to make sure the return value isn't ignored can
be done with grep, and visiting them all to hand-inspect the error
path doesn't really take all that much time or effort.

Plus, in general, dealing with I/Os that don't come back is also a
matter of making things interruptible that traditionally aren't, only
deeper down inside with harder and more problematic recovery paths.

However, if vnode locks are not going to be interruptible, vn_lock
should probably return void, not int. (Does anything use LK_NOWAIT on
vnode locks? grep doesn't think so, but that's not conclusive.)

(And while I'm at it, does anyone object to creating vn_unlock() to
wrap VOP_UNLOCK, so things pair up properly?)

-- 
David A. Holland
dholland%netbsd.org@localhost