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"