Re: Vnode API change: add global vnode cache

[J. Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost>]

On 01 May 2014, at 20:02, David Holland <dholland-tech%netbsd.org@localhost> 
wrote:

> On Thu, May 01, 2014 at 06:49:41PM +0200, J. Hannken-Illjes wrote:
>>> ok, in that case I think the only issue with the patch I looked at
>>> Sunday (was it that long ago already?) is that it does duplicate the
>>> getnewvnode logic -- if you've since fixed that I don't think I have
>>> further substantive objections.
>> 
>> What do you mean with "duplicate the getnewvnode logic" here?
>> Vfs_busy()/vfs_unbusy() and vfs_insmntque()?
> 
> ultimately, what Taylor was objecting to -- he convinced me last
> weekend that it was a valid objection, but I don't have time to look
> at the details right now :-/
<snip>
>>> I don't think that's necessary. For vnodes belonging to virtual
>>> objects, there's very little to be gained by counting them under
>>> maxvnodes or cycling them "out of memory". The actual underlying
>>> objects can't go away and the vnode itself is just a fairly small
>>> administrative structure.
>> 
>> Sounds reasonable -- but we are not at a point to make this decision.
> 
> I think we are; we aren't at the point to *do* it, but knowing more or
> less where we want to go I think we can make the design decision.

For now it should be sufficient to know these file systems may either
become users of the vnode cache or will use their own vnodes permanently
attached to their object.

>>>> Agreed.  And we may need vcache_lookup to lookup but not load a
>>>> node too.
>>> 
>>> When would one do that? I suppose there are probably reasons, but I
>>> can't think of any offhand. It's going to inherently be a race, also,
>>> since if it returns NULL or ENOENT or whatever it does if the vnode
>>> isn't loaded, the vnode could get loaded by the time the caller sees
>>> this. Consequently I suspect that most of the uses that someone might
>>> think they want would actually be wrong. :-/
>> 
>> At least union will need something like this.  The key of a union
>> node may change and there must be a way to "test" a key.  The file
>> system will have to serialise to prevent races.
> 
> Ew. It's been a long time since I looked at onionfs and I've blocked
> most of it out. How does this happen? It probably needs to be able to
> look up its own vnodes by the keys or identities of either the upper
> or lower vnodes under it, but I'd think it'd be better served to
> manufacture its own keys and have its own table for the layer
> translation. That or we could have it enter both and allow the vnode
> cache to point to multiple references of the same vnode, but that
> seems like a can of worms.

Currently union cache gets looked up by (upper vnode, lower vnode),
either but not both may be NULL.  When both vnodes are != NULL it
has to lookup (upper, NULL), (NULL, lower) and (upper, lower) and
possibly adjust the key.

> How does onionfs assign its inode numbers?

It takes them from the upper or lower layer.

> On the other hand, as Taylor noted, msdosfs needs to be able to change
> the key of a vnode, so maybe we should just expose such a function. It
> should be relatively straightforward as it doesn't need to do anything
> besides lock and shuffle the table.

If we had this and vcache_lookup union could use it too.  Another
approach is for union to manage its own table of
"(upper, lower) -> key" mappings and use this key for the vcache.

<snip>

--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)