linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>,
	Eric Van Hensbergen <ericvh@gmail.com>
Cc: v9fs-developer@lists.sourceforge.net,
	Eric Van Hensbergen <ericvh@kernel.org>,
	asmadeus@codewreck.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs/9p: Add new options to Documentation
Date: Tue, 04 Apr 2023 14:26:42 -0400	[thread overview]
Message-ID: <a7458f6fdfcf902e620fefb7f44a7e4700f761ae.camel@kernel.org> (raw)
In-Reply-To: <5898218.pUKYPoVZaQ@silver>

On Sun, 2023-04-02 at 16:07 +0200, Christian Schoenebeck 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?
> 
> > 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?
> 
> > The ultimate goal is to simplify the options based on expected use models:
> > 
> > - cache=[ none, file, all ] (none is currently default)
> 
> dir?
> 
> > - 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?

See the acregmin/acregmax/acdirmin/acdirmax settings in nfs(5). What
you're talking about is basically adding an actimeo= option.

If you're revising options for this stuff, then consider following NFS's
naming. Not that they are better in any sense, but they are at least
familiar to administrators.

As far as the sliding window goes, the way it tracks that is a bit
arcane, but in include/linux/nfs_fs.h:


        /*
         * read_cache_jiffies is when we started read-caching this inode.
         * attrtimeo is for how long the cached information is assumed
         * to be valid. A successful attribute revalidation doubles
         * attrtimeo (up to acregmax/acdirmax), a failure resets it to
         * acregmin/acdirmin.
         *
         * We need to revalidate the cached attrs for this inode if
         *
         *      jiffies - read_cache_jiffies >= attrtimeo
         *
         * Please note the comparison is greater than or equal
         * so that zero timeout values can be specified.
         */


That's probably what I'd aim for here.

> > - 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.
> 
> > - 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.
> 
> > So, I think this is more clear from a documentation standpoint, but
> > unfortuantely I haven't reduced the test matrix much - in fact I've
> > probably made it worse. I expect the common cases to basically be:
> > - cache=none
> > - new default? (cache=all, write_policy=writeback, cache_validate=open)
> > - fscache w/(cache=all, write_policy=writeback, cache_validate=5)
> > 
> > Which would give us 3 configurations to test against versus 25
> > (assuming testing for one time value for cache-validate=x). Important
> > to remember that this is just cache mode tests, the other mount
> > options act as multipliers.
> > 
> > Thoughts?  Alternatives?
> > 
> >         -eric
> > 
> > On Mon, Mar 27, 2023 at 10:38 AM Christian Schoenebeck
> > <linux_oss@crudebyte.com> wrote:
> > > 
> > > On Monday, March 27, 2023 5:05:52 AM CEST Eric Van Hensbergen wrote:
> > > > Need to update the documentation for new mount flags
> > > > and cache modes.
> > > > 
> > > > Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
> > > > ---
> > > >  Documentation/filesystems/9p.rst | 29 ++++++++++++++++-------------
> > > >  1 file changed, 16 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/Documentation/filesystems/9p.rst b/Documentation/filesystems/9p.rst
> > > > index 0e800b8f73cc..6d257854a02a 100644
> > > > --- a/Documentation/filesystems/9p.rst
> > > > +++ b/Documentation/filesystems/9p.rst
> > > > @@ -78,19 +78,18 @@ Options
> > > >               offering several exported file systems.
> > > > 
> > > >    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
> > > > +             Modes are progressive and inclusive.  For example, specifying fscache
> > > > +             will use loose caches, writeback, and readahead.  Due to their
> > > > +             inclusive nature, only one cache mode can be specified per mount.
> > > 
> > > I would highly recommend to rather specify below for each option "this option
> > > implies writeback, readahead ..." etc., as it is not obvious otherwise which
> > > option would exactly imply what. It is worth those extra few lines IMO to
> > > avoid confusion.
> > > 
> > > > +
> > > > +                     =========       =============================================
> > > > +                     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
> > > > +                     =========       =============================================
> > > > 
> > > >    debug=n    specifies debug level.  The debug level is a bitmask.
> > > > 
> > > > @@ -137,6 +136,10 @@ Options
> > > >               This can be used to share devices/named pipes/sockets between
> > > >               hosts.  This functionality will be expanded in later versions.
> > > > 
> > > > +  directio   bypass page cache on all read/write operations
> > > > +
> > > > +  ignoreqv   ignore qid.version==0 as a marker to ignore cache
> > > > +
> > > >    noxattr    do not offer xattr functions on this mount.
> > > > 
> > > >    access     there are four access modes.
> > > > 
> > > 
> > > 
> > > 
> > > 
> > 
> 
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

      parent reply	other threads:[~2023-04-04 18:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27  3:05 [PATCH] fs/9p: Add new options to Documentation Eric Van Hensbergen
2023-03-27 15:38 ` Christian Schoenebeck
2023-03-28 15:51   ` Eric Van Hensbergen
2023-04-02 14:07     ` Christian Schoenebeck
2023-04-02 18:22       ` asmadeus
2023-04-02 18:43       ` Eric Van Hensbergen
2023-04-04 18:26       ` Jeff Layton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a7458f6fdfcf902e620fefb7f44a7e4700f761ae.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@gmail.com \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=v9fs-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).