Subject: Re: VOP locking issue
To: Manuel Bouyer <bouyer@antioche.eu.org>
From: Antti Kantee <pooka@cs.hut.fi>
List: tech-kern
Date: 11/14/2006 00:28:58
On Mon Nov 13 2006 at 23:04:40 +0100, Manuel Bouyer wrote:
> On Mon, Nov 13, 2006 at 10:46:23PM +0200, Antti Kantee wrote:
> > On Sun Nov 12 2006 at 22:33:38 +0100, Manuel Bouyer wrote:
> > > This maps to this code in xbdback_xenbus.c (with error handling removed for
> > > better reading)
> > > 	err = bdevvp(xbdi->xbdi_dev, &xbdi->xbdi_vp);
> > >         err  = VOP_OPEN(xbdi->xbdi_vp, FREAD, NOCRED, 0);
> > > 	VOP_UNLOCK(xbdi->xbdi_vp, 0); 
> > > 
> > > I suspect the block device opened here was also open in userland 
> > > (by the qemu-dm process). It's also possible that it was mounted.
> > > Does anyone have an idea if this is the problem, and if it can be avoided ?
> > 
> > My theory: I suspect that checkalias() matches in bdevvp(), so you
> > get a vnode which actually supports locking (pure specfs does not).
> > Now, bdevvp() returns an unlocked vnode, so you should call vn_lock()
> > before calling VOP_OPEN() to be correct.
> 
> Thanks. But what does lock the vnode in my case then ? Or would VOP_UNLOCK()
> not complain if it's called on an unlocked vnode ? Note that the above
> code works most of the time; I've got only one report of failure.

The complement of the theory: checkalias() in bdevvp() does not match
and you get a VT_NON/specfs vnode, for which locking operations are nops
and VOP_UNLOCK() doesn't do lockmgr at all.

If checkalias() does match, someone else using that vnode might have the
lock and you are trying to release it on that someone's behalf, hence
you trigger "if (WEHOLDIT(lkp, pid, lid, cpu_num) == 0)" in lockmgr.
I think the other possibility would be to have a panic for releasing an
unlocked vnode if timing is different.

Getting vnode locking wrong, especially for corner cases and
error-branches, seems all too common.  This IMHO speaks more about vnode
locking than the people using it.

-- 
Antti Kantee <pooka@iki.fi>                     Of course he runs NetBSD
http://www.iki.fi/pooka/                          http://www.NetBSD.org/
    "la qualité la plus indispensable du cuisinier est l'exactitude"