On Sun, Apr 2, 2023 at 9:07 AM Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx> wrote: > > +CC: Jeff for experience on this caching issue with NFS ... > > On Tuesday, March 28, 2023 5:51:51 PM CEST Eric Van Hensbergen wrote: > > As I work through the documentation rework and to some extent the > > testing matrix -- I am reconsidering some choices and wanted to open > > up the discussion here. > > > > TLDR; I'm thinking of reworking the cache options before the merge > > window to keep things simple while setting up for some of the future > > options. > > Yeah, revising the 9p cache options highly makes sense! > > So what is the plan on this now? I saw you sent a new patch with the "old" > options today? So is this one here deferred? > Yes - I decided to just bite the bullet and try and have a new approach to things that was less confusing and try and reduce the test matrix. Unfortunately, I was also switching over to trying to use b4 to manage these things and didn't see a way to relate the patch to the original one. I figured the increase in scope justified a new thread. > > While we have a bunch of new options, in practice I expect users to > > probably consolidate around three models: no caching, tight caches, > > and expiring caches with fscache being an orthogonal add-on to the > > last two. > > Actually as of today I don't know why somebody would want to use fscache > instead of loose. Does it actually make sense to keep fscache and if yes why? > Persistent caches make sense in certain scenarios, it basically gives a way to lazy populate a local cache of the remote server. I wouldn't use it in qemu or a VM, but I'd definitely use it in cluster scenarios. Its also potentially useful for unstable connections -- but this is more challenging to deal with in 9p although solutions have been proposed in the past. > > The ultimate goal is to simplify the options based on expected use models: > > > > - cache=[ none, file, all ] (none is currently default) > > dir? > That was what I was originally going to do, but 'all' makes the inclusivity of file explicit. The more I thought about it, the less I saw a use case for caching meta data and not file data. Of course, in the new patch series I have a bit for meta-data (which is a bit more accurate than just saying dir) at least in the current implementation. However, if I did a shortcut, it probably would be all and not dir (there isn't one yet in the new patch as I'm holding off until I fix consistency for meta-data -- which is in progress and I think imperfect but closer than loose). And actually once we are convinced this is working, the shortcut for all may be better represented as 'default' (hopefully). > > - write_policy = [ *writethrough, writeback ] (writethrough would be default) > > - cache_validate = [ never, *open, x (seconds) ] (cache_validate > > would default to open) > > Jeff came up with the point that NFS uses a slicing time window for NFS. So > the question is, would it make sense to add an option x seconds that might be > dropped soon anyway? > I had thought about that, and in fact in the new patch series I haven't implemented either yet. In the bitmask version I could used one of the reserved bits to specify adaptive timeouts like Jeff said NFS does and then the high bits to specify the base timeout or 0 if you never want to validate (current loose). Loose mode itself specifies look at the base timeout for how much temporal consistency to apply. Looking ahead at implementing this there is going to be ripple effects through the code, probably changing the way we use the cache_validity field in inode. But seems straightforward enough. > > - fscache > > > > So, mapping of existing (deprecated) legacy modes: > > - none (obvious) write_policy=writethrough > > - *readahead -> cache=file cache_validate_open write_policy=writethrough > > - mmap -> cache=file cache_validate=open write_policy=writeback > > Mmm, why is that "file"? To me "file" sounds like any access to files is > cached, whereas cache=mmap just uses the cache if mmap() was called, not for > any other file access. > Well, not technically true -- because if the file is mmaped and it is accessed another way other than the mmap then we need to basically treat it as cached or we'll get inconsistencies between the mmap version and the uncached read/write version. So the code ends up using open-to-close file caching for everything. In any case, mmap was there for compatibility reasons before we had a non-loose caching option. With tight caches, its better to just incorporate mmap with it (given the above concerns on behavior). Otherwise we'd have to enforce a non-posix semantic of the only way to access an mmaped file is with mmap and force invalidations of others that might already have a file open when someonebody else mmaps it...would be messy. > > - loose -> cache=all cache_validate=never write_policy=writeback > > - fscache -> cache=all cache_validate=never write_policy=writeback & > > fscache enabled > > > > Some things I'm less certain of: cache_validation is probably an > > imperfect term as is using 'open' as one of the options, in this case > > I'm envisioning 'open' to mean open-to-close coherency for file > > caching (cache is only validated on open) and validation on lookup for > > dir-cache coherency (using qid.version). Specifying a number here > > expires existing caches and requires validation after a certain number > > of seconds (is that the right granularity)? > > Personally I would then really call it open-to-close or opentoclose and waste > some more characters in favour of clarity. > I had open2close in the new version and then reverted it to the inverse of loose because it made checking the bit pattern much easier. But the plan is for loose to cover no-consistency (when cache-timeout==0, and temporal consistency when cache-timeout > 0, and then augment that with Jeff's suggestion of an adaptive bit). Perhaps the new loose shortcut will cover NFS-like semantics with a base-timeout of 5s and adaptivity bit set. -eric