Subject: Re: VOP locking issue
To: Antti Kantee <email@example.com>
From: Manuel Bouyer <firstname.lastname@example.org>
Date: 11/14/2006 23:07:47
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 <email@example.com>
NetBSD: 26 ans d'experience feront toujours la difference