Re: CVS commit: src/sys/ufs/ufs

[Antti Kantee <pooka%cs.hut.fi@localhost>]

On Tue Sep 22 2009 at 21:04:14 +0200, Manuel Bouyer wrote:
> that's not an issue with the reference count. It's an issue with vclean()
> calling VOP_RECLAIM() even if the refcount is greater than 1, and
> vrelel() calling vclean() even if the refcount is greater than 1,
> when the file has been unlink(2)ed. There's a comment about this in
> vrelel(), look for variable "recycle".

Ah, ic, probably would've been easier if I read the PR first ;)

So basically someone can vget the vnode (via fhtovp, since it's gone
from the directory namespace) between VOP_INACTIVE and clean.

Your fix doesn't really fix this problem, since depending on timings
the inode might still be recycled after you check it's "valid".

Hmm, ok, so things which might happen:

1: VOP_REMOVE, vnode is locked, vrele is called
2: fhtovp before inode is reclaimed, blocks on vn_lock
1: VOP_INACTIVE releases vnlock, returns signal to recycle vnode
2: gets lock, does check that it's still the same inode
1: reclaims vnode
2: boom

Since vget takes the interlock, a dirty & cheap trick might be to check
that the reference count is still one before trying to clean the vnode in
vrelel(), otherwise punting and letting the next release-to-0 reclaim it?

Blah, I didn't even want to think about this migrane-inducer now.
Maybe people who have recently worked on vnode reclaiming could instead
be the ones to comment?