DragonFly kernel List (threaded) for 2009-05
Re: Problems with vnode locking/unlocking
Nevermind my previous message, I've fixed it and it seems to be working
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