Re: Vnode API change: add global vnode cache

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

On 07 Apr 2014, at 18:02, Taylor R Campbell 
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:

>   Date: Mon, 7 Apr 2014 15:51:00 +0200
>   From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
> 
>   On 07 Apr 2014, at 03:22, Taylor R Campbell 
> <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
>> Is the plan to nix getnewvnode and ungetnewvnode?  It would be good to
>> avoid long-term duplication of its body.  But I suspect we'll want to
>> keep those for, e.g., tmpfs, in which case this should use getnewvnode
>> and ungetnewvnode instead of vnalloc/setup/teardown/vnfree.
> 
>   Not sure if we have to keep getnewvnode() in the long term.
> 
>   It cannot be used here as we don't want the partially initialised
>   vnode (before VFS_LOAD_NODE) on the mnt_vnodelist.
> 
>   If we end up replacing getnewvnode() with a two-stage allocator that
>   initialises in the first stage and finalises in the second stage we
>   may use it here.
> 
> In that case, could you set the VI_CHANGING bit in vp, rather than
> node->vn_vnode = NULL in the vcache node, to indicate that the vnode
> is being initialized in vcache_intern?  That way,
> 
> (a) it is safe to put the vnode on the mount list -- anyone trying to
> use it from there must use vget first --;
> 
> (b) there is only one place waits happen, namely vget (vwait), so no
> need for vcache.cv or some of the complex loop logic in vcache_intern;
> 
> (c) tmpfs can mirror the vcache_intern logic without more duplication
> of code from getnewvnode; and
> 
> (d) as a tiny added bonus, omitting vcache.cv will make the vcache
> struct fit in a single cache line on, e.g., amd64.
> 
> Generally, we should have only one mechanism for weak references to
> vnodes, and currently that's vget.  If we do want to take dholland's
> idea of a separate structure for weak references to vnodes, then the
> vnode mount list and anything else should use that too.  But I think
> vget is good enough for now.
> 
> Also, it occurs to me that we're going to have not two but multiple
> copies of initialization code that is currently in getnewvnode,
> because in your API, some is copied in vcache_intern and some must be
> copied in VFS_LOAD_NODE.
> 
> So I'm inclined to say that vcache_intern should itself look a little
> more like getnewvnode, and perhaps VFS_LOAD_NODE should be VOP_LOAD
> instead.  See attached file for a sketch this, and a tmpfs version.
> 
> Except for the VI_CHANGING part, the vcache_intern I attached uses
> nothing internal to the vnode abstraction.  To address VI_CHANGING, we
> could make getnewvnode set it by default and require callers to use
> some new routine, say vready, to clear it.  Then vcache_intern, or
> clones of it like tmpfs will want, can live entirely outside the vnode
> API.

Sorry for being late -- back from the openssl disaster now ...

There is no need to do this VI_CHANGING etc. here.  Primary goal of
the vnode cache is to always initialise vnode / fs node pairs before
they become visible or appear on any lists.

There is no need to make the vcache API look like getnewvnode().  But
getnewvnode() should change to a two-pass allocator that initialises
in the first pass and finalises, adds to mount list in the second pass.

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