Re: Vnode API change: add global vnode cache

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

On 07 Apr 2014, at 19:28, Chuck Silvers <chuq%chuq.com@localhost> wrote:

> On Sun, Apr 06, 2014 at 12:14:24PM +0200, J. Hannken-Illjes wrote:
>> Currently all file systems have to implement their own cache of
>> vnode / fs node pairs.  Most file systems use a copy and pasted
>> version of ufs_ihash.
>> 
>> So add a global vnode cache with lookup and remove:
> [...]
> 
> hi,
> 
> the principle of this is good, but I have a couple concerns about the details:
> 
> - why use an rb tree instead of a hash table?
>   other people are saying that the lock contention is the same,
>   but there are two differences:
>    - updating an rb tree touches a lot more memory, so the lock is held for
>      longer.
>    - different parts of a hash table can easily be protected by
>      different locks, but that doesn't work for a tree.

The underlying data structure may change.  Once at least ufs, layerfs and nfs
use this cache it will be easier to do some measurements.

> - it looks like this increases the effective size of a vnode by a net 40 bytes
>   (7 pointers added, 2 removed).  it would be better to do this without using
>   more memory than we do now.  is there any real reason to have a separate
>   structure for this rather than just putting the fields directly in
>   struct vnode?

Not all file systems will use the cache (tmpfs for example doesn't need it)
so to me it looks better to use an extra struct when needed.

> to avoid needing fields for the "key" pointer and key length,
>   each fs could supply its own comparison function.

Going this route (more or less making the current ufs_ihash general)
makes it impossible to serialise node creation.

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