Subject: Re: CVS commit: syssrc/sys/miscfs/nullfs
To: enami tsugutomo <enami@sm.sony.co.jp>
From: Bill Studenmund <wrstuden@wasabisystems.com>
List: tech-kern
Date: 03/18/2002 10:25:23
On 18 Mar 2002, enami tsugutomo wrote:
[in another mail]
> BTW, probably it is time to move to tech-kern, instead of staying
> source-changes :-).
Agreed. :-)
> Bill Studenmund <wrstuden@wasabisystems.com> writes:
>
> > I'm sorry, I misspoke. What I meant to suggest was that when we reclaim
> > the layered node, we vrecycle() the lower one (if we were the last
> > reference). That way both nodes die at about the same time.
>
> No, no. For example, if the lower vnode is once used directly in the
> lower file system while it is cached in layer file system, the lower
> vnode should be reclaimed much later than the layer file system node
> (instead of same time).
True. I think that trying to ensure that things are reused in the exact
order they were last used gets messy with layered file systems. So my vote
is (now) to just release the lower vnode when the upper one dies, and let
the lower one die as it dies.
> > > > But why? Why release and grab every time we go on or off the free list,
> > > > when the times it matters (deletion below for instance) are rare, and
> > > > well-defined?
> > >
> > > Since, IMHO, keeping reference when it isn't used actually is abuse.
> >
> > Abuse of what? You feel strongly about this, but I'm still not clear on
> > what excatly you feel strongly about and why.
>
> As we (or only me?) saw, rest of kernel expects node is released when
> it is no longer used.
Ok. What does that have to do with the upper vnode keeping a reference to
the lower one? Yes, there is a problem from how we don't propogate
information up when the lower vnode has been fully unlinked. But we can
fix that without unreferencing the lower vnode when we inactivate the
upper one.
> > > Put vnode back to freelist when it isn't used anymore and reactivating
> > > it when it becomes necessary later is what we do for other
> > > filesystems. Why we need to handle layerfs specially?
> >
> > That is what we do for the layer vnode. But why should we do it for the
> > lower vnode too? Releasing and reactivating the lower node each time we
> > release and reactivate the upper one helps us catch a problem that happens
> > infrequently, but involves adding extra steps to the common code path.
>
> So, the lower filesystem or lookup code also implements own cache to
> increase performance.
I assumed it would. But we still have to take the lower vnode on and off
the free list each time the upper one comes on and off.
Hmm.. I just thought of a strong reason why we NEED the upper vnode to
reference the lower one. The lock for the upper one is the same as the
lower one - they use the same struct lock, which is located in the lower
vnode. As long as that is the case, the upper really needs to hang onto
the lower one.
Also, I think you've made an implicit assumption in what you describe, and
that it will (well can) get us into trouble. You've suggested (as I
understand it) when we put the upper vnode on the free list, we release
its reference on the lower vnode. Then in layer_node_find, if we find the
upper vnode doesn't have its reference to the lower one, we make sure it's
still the same lower vnode (v_id) and if so readd the reference.
The assumption is that the only way that vnode can come back into use
involves layer_node_find(). While that may be the only way in practice,
strictly speaking, anything can vget the upper vnode off of the free list
and start using it. And there's no call to the file system when that
happens. So something can vget the layered vnode off of the free list and
start using it when it holds no reference to the lower vnode. That would
be bad.
Take care,
Bill