Re: vnode locking oddities, again

[David Holland <dholland-tech%netbsd.org@localhost>]

On Thu, Oct 22, 2009 at 06:38:21PM +0200, Manuel Bouyer wrote:
 > Now the problems.
 > First is in getcleanvnode(). After selecting a victim from a free list,
 > v_usecount is incremented, vp is cleaned and then we check that we're
 > the only user of the vp. Shouldn't vclean() be done only in the
 > (vp->v_usecount == 1) case ?

That sounds like a good bet, yes.

 > Second is vrelel(). Because of locks being dropped and reaquired at various
 > places (one notable point is VOP_INACTIVE()), 

Randomly dropping and acquiring locks is not correct and inevitably
leads to random problems. This should be cleaned up.

I have been meaning to look into this, but you know what my available
time plot looks like. :(

 > Shouldn't vget check for VI_INACTNOW in addition to (VI_XLOCK |
 > VI_FREEING) ?

My offhand guess is that VI_FREEING should be abolished entirely. The
locking should be set up to not expose vnodes in an intermediate state
at all. Getting to that point might be a problem though.

Relatedly to all this, is there any reason vget returns vnodes locked?
It should serve no purpose if the refcounting is working properly, and
creates a problem for rename.

 > Also, what's the point of VI_INACTREDO ? It's set of cleared on multiple
 > places but never tested ?

It's tested in one place in the midle of vrelel(). It also looks like
another piece of mess that ought to be done in some cleaner way... :(

-- 
David A. Holland
dholland%netbsd.org@localhost