vnode locking oddities, again

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

Hi,
while looking at the vnode code, I've found a what I think is an issue
with deleted files, and also with getcleanvnode().

If I get it right, what guarantees a vnode user that it's vnode
won't be dramatically changed under it (unless umount -f or a removable
device is gone) is that he holds a reference to the vnode (i.e. it
increased v_usecount by one).

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 ?

Second is vrelel(). Because of locks being dropped and reaquired at various
places (one notable point is VOP_INACTIVE()), the vnode may get another
reference while vrelel() is already in the process of cleaning it.
This is a problem with VOP_INACTIVE() and deleted files: we can gain
a reference to a unlinked but not yet freed file (unlikely when the
access is local, but can easily happen with NFS), yet VOP_INACTIVE()
will free the inode and associated blocks. I guess bad things may happen
after that, like writing to a free block/free inode or such.

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

I guess theses could explain Matthias's panic on VI_TEXT ("panic due to lost
vnode flag" on tech-kern@ today), and maybe some corruption I'm seeing
on a NFS server.

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

-- 
Manuel Bouyer, LIP6, Universite Paris VI.           
Manuel.Bouyer%lip6.fr@localhost
     NetBSD: 26 ans d'experience feront toujours la difference
--