Nevermind my previous message, I've fixed it and it seems to be working properly now.
On Tue, 2009-05-26 at 16:02 -0700, Matthew Dillon wrote: > :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 > <[email protected]>
