Re: [PATCH v3 00/11] Performance fixes for 9p filesystem

[Christian Schoenebeck]

On Sunday, February 5, 2023 5:37:53 PM CET Eric Van Hensbergen wrote:
> I was so focused on the bugs that I forgot to respond to the
> performance concerns -- just to be clear, readahead and writeback
> aren't meant to be more performant than loose, they are meant to have
> stronger guarantees of consistency with the server file system.  Loose
> is inclusive of readahead and writeback, and it keeps the caches
> around for longer, and it does some dir caching as well -- so its
> always going to win, but it does so with risk of being more
> inconsistent with the server file system and should only be done when
> the guest/client has exclusive access or the filesystem itself is
> read-only.

Okay, that's surprising to me indeed. My expecation was that "loose" would 
still retain its previous behaviour, i.e. loose consistency cache but without 
any readahead or writeback. I already wondered about the transitivity you used 
in code for cache selection with direct `<=` comparison of user's cache 
option.

Having said that, I wonder whether it would make sense to handle these as 
options independent of each other (e.g. cache=loose,readahead), but not sure, 
maybe it would overcomplicate things unnecessarily.

> I've a design for a "tight" cache, which will also not be
> as performant as loose but will add consistent dir-caching on top of
> readahead and writeback -- once we've properly vetted that it should
> likely be the default cache option and any fscache should be built on
> top of it.  I was also thinking of augmenting "tight" and "loose" with
> a "temporal" cache that works more like NFS and bounds consistency to
> a particular time quanta.  Loose was always a bit of a "hack" for some
> particular use cases and has always been a bit problematic in my mind.

Or we could add notifications on file changes from server side, because that's 
what this is actually about, right?

> So, to make sure we are on the same page, was your performance
> uplifts/penalties versus cache=none or versus legacy cache=loose?

I have not tested cache=none at all, because in the scenario of 9p being a 
root fs, you need at least cache=mmap, otherwise you won't even be able to 
boot a minimum system.

I compared:

  * master(cache=loose) vs. this(cache=loose)

  * master(cache=loose) vs. this(cache=readahead)

  * master(cache=loose) vs. this(cache=writeback)

> The 10x perf improvement in the patch series was in streaming reads over
> cache=none.

OK, that's an important information to mention in the first place. Because 
when say you measured a performance plus of x times, I would assume you 
compared it to at least a somewhat similar setup. I mean cache=loose was 
always much faster than cache=none before.

> I'll add the cache=loose datapoints to my performance
> notebook (on github) for the future as points of reference, but I'd
> always expect cache=loose to be the upper bound (although I have seen
> some things in the code to do with directory reads/etc. that could be
> improved there and should benefit from some of the changes I have
> planned once I get to the dir caching).
> 
>           -eric