On Tue, Nov 14, 2006 at 01:49:31AM +0200, Antti Kantee wrote:
> On Tue Nov 14 2006 at 00:01:07 +0100, Manuel Bouyer wrote:
> > OK, thanks for the explainations. As you're familiar with this could you
> > tell me if the attached patch looks good ? Especially I'm not sure
> 
> I hope the analysis is correct.  Determining anything for sure in the
> vfs code would require two years of zen (xen ?) meditation for me ;)
> 
> But, I don't really know if vn_lock(vp, LK_RETRY | LK_EXCLUSIVE) can
> fail, especially since we're talking about a device vnode, which gets
> unassociated from a file system if its host fs is unmounted.  There's no
> clear rule about vn_lock() failing in the code, so I'd maybe just go
> with a KASSERT() at most instead of an error branch.  Would be nice to
> formulate a clear rule for this in any case.  I've got some tangentials
> for stuff like this on my TODO-list.

Well, I'll leave the error branch for now. I doesn't cost much here.

> 
> > about the vrele(), I don't know if bdevvp() will vref() the vnode (I took
> > this from dkwedge/dk.c).
> > BTW, my code never calls vput() or vrele() so I don't know what
> > decrements v_usecount for normal exit (vn_close() maybe?)
> 
> Yea, the returned node is referenced.  Otherwise it could be cleaned
> out before we ever got a chance to touch it.
> 
> I didn't read your entire code, but yes, vn_close() vput()s (or vrele()s
> from the caller's POV, since it locks the vnode internally).

Ok, thanks for the clarification. 

> 
> > BTW, these files:
> > dev/ata/ata_raid_adaptec.c
> > dev/ata/ata_raid_promise.c
> > dev/ata/ld_ataraid.c
> > dev/raidframe/rf_netbsdkintf.c
> > 
> > seems to not do proper vn_lock() before VOP_OPEN(). 
> 
> You forgot mountroot ;)

OK, I see your point.  But I comment would be good, because I copied this
code for the xen disk backends :)

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--