Re: [PATCH v4 10/11] fs/9p: writeback mode fixes

Christian Schoenebeck
2023-02-18T19:58:30Z

On Saturday, February 18, 2023 11:01:22 AM CET asmadeus@codewreck.org wrote:
> Eric Van Hensbergen wrote on Sat, Feb 18, 2023 at 12:33:22AM +0000:
> > This fixes several detected problems from preivous
> > patches when running with writeback mode. In
> > particular this fixes issues with files which are opened
> > as write only and getattr on files which dirty caches.
> >
> > This patch makes sure that cache behavior for an open file is stored in
> > the client copy of fid->mode. This allows us to reflect cache behavior
> > from mount flags, open mode, and information from the server to
> > inform readahead and writeback behavior.
> >
> > This includes adding support for a 9p semantic that qid.version==0
> > is used to mark a file as non-cachable which is important for
> > synthetic files. This may have a side-effect of not supporting
> > caching on certain legacy file servers that do not properly set
> > qid.version. There is also now a mount flag which can disable
> > the qid.version behavior.
> >
> > Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
>
> Didn't have time to review it all thoroughly, sending what I have
> anyway...
>
> > diff --git a/Documentation/filesystems/9p.rst b/Documentation/filesystems/9p.rst
> > index 0e800b8f73cc..0c2c7a181d85 100644
> > --- a/Documentation/filesystems/9p.rst
> > +++ b/Documentation/filesystems/9p.rst
> > @@ -79,18 +79,14 @@ Options
> >
> > cache=mode specifies a caching policy. By default, no caches are used.
> >
> > - none
> > - default no cache policy, metadata and data
> > - alike are synchronous.
> > - loose
> > - no attempts are made at consistency,
> > - intended for exclusive, read-only mounts
> > - fscache
> > - use FS-Cache for a persistent, read-only
> > - cache backend.
> > - mmap
> > - minimal cache that is only used for read-write
> > - mmap. Northing else is cached, like cache=none
> > + ========= =============================================
> > + none no cache of file or metadata
> > + readahead readahead caching of files
> > + writeback delayed writeback of files
> > + mmap support mmap operations read/write with cache
> > + loose meta-data and file cache with no coherency
> > + fscache use FS-Cache for a persistent cache backend
> > + ========= =============================================
>
> perhaps a word saying the caches are incremental, only one can be used,
> and listing them in order?
> e.g. it's not clear from this that writeback also enables readahead,
> and as a user I'd try to use cache=readahead,cache=writeback and wonder
> why that doesn't work (well, I guess it would in that order...)

+1 on docs

The question was also whether to make these true separate options before being
merged.

I give these patches a spin tomorrow.