Subject: Re: VOP locking issue
To: Manuel Bouyer <bouyer@antioche.eu.org>
From: Antti Kantee <pooka@cs.hut.fi>
List: tech-kern
Date: 11/14/2006 01:49:31
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.
> 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).
> 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 ;)
--
Antti Kantee <pooka@iki.fi> Of course he runs NetBSD
http://www.iki.fi/pooka/ http://www.NetBSD.org/
"la qualité la plus indispensable du cuisinier est l'exactitude"