DragonFly kernel List (threaded) for 2009-05
Re: Problems with vnode locking/unlocking
In the devfs_root->devfs_allocv issue I've decided to check inside
allocv to see if devfs_node->v_node is NULL and only THEN allocate a new
vnode, as it should be nulled out everry time a vnode is reclaimed.
Also I've taken into account your suggestions regarding locking and
yesterday's problems are gone (hang when reading the content of a
subdirectory) but I've found a new one.
Now I get a panic in lockmgr due to the vn_unlock in nresolve when
creating, or trying to create, a symlink. I suppose I'm trying to unlock
an unlocked vnode and that is causing the problem. Is it right after all
to unlock the vnode in nresolve? I don't think nsymlink has even been
called yet at the time of the panic.
The new code is at:
This is the panic:
kernel: type 12 trap, code=0
Stopped at lockmgr+0x8b: movl 0x4(%ebx),%ecx
lockmgr(c0,6) at lockmgr+0x8b
vn_unlock(0,0,d4484328,cf79c518,d4453af8) at vn_unlock+0x13
cache_resolve(d1f54bd4,c1d45f58) at cache_resolve+0x30f
nlookup(d1f54c80,0,d1f54c80,d1f54cf0,d1f54cc4) at nlookup+0x25e
kern_stat(d1f54c80,d1f54c18,d1f54c34,c048f383,d1f067f4) at kern_stat+0xf
sys_lstat(d1f54cf0,6,0,0,cf79c2d8) at sys_lstat+0x35
syscall2(d1f54d40) at syscall2+0x1ef
Xint0x80_syscall() at Xint0x80_syscall+0x36
On Tue, 2009-05-26 at 16:02 -0700, Matthew Dillon wrote:
> :I'm running into quite a lot of trouble with vnode locking and
> :unlocking, releasing, putting, getting, ... I'm 100% sure I got most if
> :not all of the vnode locking wrong and I reallly need some help
> :understanding it. It is still unclear to me what vnops need to do what
> :with regard to locking/unlocking.
> :My current code is here:
> :Right now my concerns are mainly in devfs_vnops.c, where all the
> :locking/unlocking/... occurs or should occur.
> :I'd appreciate some insight/comments/corrections/... on this issue!
> :Alex Hornung
> One thing I noticed is that the devfs_root() code path does not
> look right. This routine can be called any number of times by
> the kernel, you don't want to allocate a new root vnode every
> devfs_root()->devfs_allocv()->getnewvnode()-> ...
> If the root node already has a vnode associated with it you have to
> ref and vn_lock and return that vnode, not allocate a new vnode.
> I think you might be overwriting previously acquired vnode pointers
> in that root node and that will certainly mess things up.
> Another thing I noticed is that you need to remember that when you
> return a vnode in *vpp, the caller is expected that vnode to be
> referenced (and possibly also locked), which means you don't release
> the vnode that you are also returning unless you have extra references
> (2 or more) and you need to get them down to one reference for the
> So for example the devfs_nsymlink() call is calling devfs_allocvp()
> but it is also returning the vp in *ap->a_vpp. In the case of
> devfs_nsymlink() I believe it is expected to return a referenced
> but NOT locked vnode, so you would unlock it but not dereference it
> before returning.
> You will want to check all the VNOPS that return a vnode in *ap->a_vpp
> for similar issues.
> In the case if nresolve I believe you are doing it correctly...
> VOP_NRESOLVE() does not return a vnode (there is no ap->a_vpp),
> it just expects the namecache entry to be resolved and the
> cache_setvp() call doesn't inherit any refs or anything so you unlock
> and release the vnode like you are doing.
> Matthew Dillon