Re: VOP_*LOCK* cleanup proposal

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

On Tue, Jun 22, 2010 at 10:52:13AM +0000, Andrew Doran wrote:
> 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.

So what I am saying is by applying your changes and fixing the uses of
vget(), we no longer have a need for LK_INTERLOCK.