Re: Vnode API change: VI_CLEAN and VI_XLOCK

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

   Date: Fri, 21 Mar 2014 11:10:54 +0100
   From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>

   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?