Re: Vnode API change: add global vnode cache

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)