Re: VOP_*LOCK* cleanup proposal

[Andrew Doran <ad%NetBSD.org@localhost>]

On Tue, Jun 22, 2010 at 10:27:13AM +0200, Juergen Hannken-Illjes wrote:

> The vnode lock operations still carry some arguments and semantics from
> our old lockmgr(9).  I propose to clean them up as:
> 
> 1) VOP_LOCK(vp, flags): Limit the set of allowed flags to LK_EXCLUSIVE,
>    LK_SHARED and LK_NOWAIT.  LK_INTERLOCK is no longer allowed as it
>    makes no sense here.
> 
> 2) VOP_UNLOCK(vp, flags): Remove the flags argument.  It must be zero
>    for some time now.
> 
> 3) VOP_ISLOCKED(vp): Remove the for some time unused return value
>    LK_EXCLOTHER.  Mark this operation as "diagnostic only".  Making a
>    lock decision based on this operation is no longer allowed.
> 
> A diff covering 1) and 3) is attached.  The result of the substitution
> 's/\(VOP_UNLOCK([^,]*\), 0/\1/' to kill the second argument of
> VOP_UNLOCK() is omitted as it is completely mechanic.
>    
> Comments or objections anyone?

Go for it.  I looked into similar changes before but got sidetracked.

There is another closely related item.  Strictly speaking, vget() should
never be called without holding v_interlock.  I had a scan of the places
we are not specifying LK_INTERLOCK and they fall into two categories:

- Should be using v_interlock + vget() but is racy.  vnode can be
  reincarnated with a different identity, or freed or...

- Abusing vget() to mean vref() + VOP_LOCK().  At these points the
  vnode's identity isn't going to change, so vget() isn't appropriate.