Re: Problems with vnode locking/unlocking
:Hi,
:
: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:
:http://gitweb.dragonflybsd.org/~alexh/dragonfly.git/tree/9de1a66518d104077521a13d7b13ae958fd79d98:/sys/vfs/devfs
:
: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
time!
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
return.
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.
-Matt
Matthew Dillon
<dillon@backplane.com>
討論串 (同標題文章)
完整討論串 (本文為第 2 之 4 篇)：