On Mar 21, 2014, at 3:07 PM, Taylor R Campbell <campbell+netbsd-tech-k...@mumble.net> wrote:
> Date: Fri, 21 Mar 2014 11:10:54 +0100 > From: "J. Hannken-Illjes" <hann...@eis.cs.tu-bs.de> > > The vnode flags VI_CLEAN and VI_XLOCK are used in file systems to > check for reclaimed or reclaiming but still active vnodes, so > make the vnode flags VI_XLOCK, VI_CLEAN (and VI_LOCKSHARE) private to > the files kern/vfs_* and add a function to check the vnode state: > > int vdead_check(struct vnode *vp, bool may_sleep) > > will return one of: > > EBUSY: vnode is becoming dead, with "may_sleep == false" only. > ENOENT: vnode is dead. > 0: otherwise. > > Is this about vnodes that have been *revoked* or vnodes that the > system has chosen to *destroy*? (`Dead' currently means both, but I > believe that joint meaning is a mistake and I'd like to either kill > that term or assign it one meaning or the other.) > > If it's vnodes that have been revoked, OK -- I'm not sure there's much > real use for this outside the lock routines (lfs looks sketchy, as > always) and I think it would be better to make that distinction in > vn_lock rather than in all the VOP_LOCKs, but OK. > > But if it's vnodes that the system has chosen to destroy, then it > seems wrong -- it shouldn't be possible to use this routine without > first having called vget. It is vnodes that either are revoking (at a point of no return) or that have been revoked. And yes -- in the future (after lfs and specfs got fixed) it should assert "v_usecount > 0". > For example, the logic in spec_node_lookup_by_dev looks wrong to me, > before and after your patch -- it's not clear to me why it doesn't > just first call vget (and then perhaps call vget again for > vp->v_specnode->sn_dev->sd_bdevvp). Yes -- this was wrong and still is. But not with this commit. > Other minor notes: > > Does this also make vwait private Yes -- taken. > I'd rather use a named constant flag than a boolean here. I can never > keep straight what true or false means and whether true means it or > false means it, but vdead_check(vp, VDEAD_NOWAIT) or vdead_check(vp, > VDEAD_WAITOK) is clear enough. Ok -- will use VDEAD_NOWAIT. > Also, what about EINTR/ERESTART? It is (still) not possible to interrupt vwait() ... New diff at http://www.netbsd.org/~hannken/vnode-pass5-2.diff -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)