On Thu Jul 05 2007 at 10:43:56 -0700, Bill Stouder-Studenmund wrote:
> We have VOP_REVOKE() so that when someone logs out of a tty, any existing 
> process w/ that device (that instance of the vnode) open gets connected to 
> a black hole. That way your cat > special_secrets & process can't read my 
> passwords, and I can't accidentally see the output spew of the employee 
> performance analysis tool you have running the "layoffs" criteria set.
> 
> So VOP_REVOKE() is supposed to blow away the node in that it's now a 
> deadfs vnode. However the vnode is still supposed to be usable, to the 
> extent that all the callers get errors then eventually release references, 
> and the vnode dies.
>
> From what we're saying, that won't work. That's bad.

Yes.  But the only reason it explodes seems to be the lock pointer
exported from the vnode.  I, however, do maintain that you should revoke
all vnodes down from the upmost you revoked.  You just need to make the
one above know that it's a deadfs situation.  Actually, that's what the
code does now.  It just doesn't revoke any layerfs vnodes.

> > How do I not kill the lock?  The vnode is reclaimed.  It won't be
> > re-reclaimed after this.  If it's the lower layer, it doesn't even know
> > about the upper one, right?  If vnode locks were separate and had separate
> > reference counts, then maybe, but ...
> 
> I think we need to look at our reclaim processing.
> 
> The lower layer DOES know something's going on. It knows the vnode has a 
> use count that's not zero.

Ah, right.

> So there are two cases where we go through reclaim processing. In one, we 
> have a vnode that's on the "free" list, meaning it's got no active 
> references. As an aside, it's not really free as it's still a live vnode 
> for whatever it initially was, it's just up for grabs. In this case, we 
> need to scrub the old usage of the vnode away, and make it ready for a new 
> user. Implicitly we know that it was at the head of the free list (well, 
> the first one up for grabs).

Well, except that the vnode is really free effectively, as it has the
irrevokable VXLOCK bit set and nobody can grab hold of that particular
instance any more.  Not that that makes any particular difference in
this discussion.

> The other case is where we're revoking a node. For a leaf file system, if
> the use count is greater than one, there are other users and so we can't
> just wipe the vnode now. Unhook it from the fs, yes. Turn it into a deafs
> node, yes. Make it so future uses will explode, no. Likewise, if the
> revoke came to us via a layered file system, we shouldn't do the 
> super-destructive reclaim.
> 
> I haven't looked at the code in a while, but assuming the code will 
> correctly handle the "someone still has the tty open" case, we just need 
> to get the layer case to trigger the same processing. Adding an explicit 
> parameter to VOP_REVOKE() which sys_revoke() sets to 0 and layer_revoke 
> sets to 1 would do it.

I don't see how the forced unmount situation is any different from a
revoke and therefore why revoke would need a special flag.

The call sequence is approximately the following:

VOP_REVOKE
VOP_UNLOCK (vclean() doesn't call inactive because the vnode is still active)
VOP_RECLAIM
v_op set to deadfs
upper layer decides this one is not worth holding on to
vrele()
VOP_INACTIVE (for deadfs)
new start for the vnode

Where do you want to free the lock structure?  We will never get another
VOP_RECLAIM for the original file system method.  Do we want to do it in
deadfs VOP_INACTIVE?  That's ugly, but it's the only place I see where to
do it.  It also needs an accompanying bit to say "hey, we already nuked
the lock in the file system reclaim op" in case the vnode is NOT revoked.

I'd prefer another approach where the whole layering lock export system
was redone instead of redoing reclaim to fix the layer lock exporting.
But I don't know how to fix layerfs locking ...

> If we won't correctly handle the case of a device being open when someone 
> else doe the revoke, well, we need to fix that issue first. :-)

That's not a problem.  It just reverts to deadfs.  And with one layer
there are no exported locks.

-- 
Antti Kantee <pooka@iki.fi>                     Of course he runs NetBSD
http://www.iki.fi/pooka/                          http://www.NetBSD.org/
    "la qualité la plus indispensable du cuisinier est l'exactitude"