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. 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). Other minor notes: Does this also make vwait private? 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. Also, what about EINTR/ERESTART?