MIME-Version: 1.0
References: <20230124023834.106339-1-ericvh@kernel.org> <20230218003323.2322580-11-ericvh@kernel.org>
 <Y/Ch8o/6HVS8Iyeh@codewreck.org> <1983433.kCcYWV5373@silver>
 <CAFkjPT=xhEEedeYcyn1FFcngqOJf_+8ynz4zeLbsXPOGoY6aqw@mail.gmail.com> <Y/FiBbMQcEblQ/XR@codewreck.org>
In-Reply-To: <Y/FiBbMQcEblQ/XR@codewreck.org>
From:   Eric Van Hensbergen <ericvh@gmail.com>
Date:   Sat, 18 Feb 2023 17:52:15 -0600
Message-ID: <CAFkjPTk5GYk4mDR2+OcU1qsaW8Z7c-H8PQpJ0NrfgZRkx-4udA@mail.gmail.com>
Subject: Re: [PATCH v4 10/11] fs/9p: writeback mode fixes
To:     asmadeus@codewreck.org
Cc:     Christian Schoenebeck <linux_oss@crudebyte.com>,
        Eric Van Hensbergen <ericvh@kernel.org>,
        v9fs-developer@lists.sourceforge.net, rminnich@gmail.com,
        lucho@ionkov.net, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Feb 18, 2023 at 5:41 PM <asmadeus@codewreck.org> wrote:
>
> For clarity though I'd use writeback in the documentation, and keep mmap
> as a legacy mapping just for mount opts parsing; the behaviour is
> slightly different than it used to be (normal read/writes were sync) so
> it's good to be clear about that.
>

Yeah, that was my intent in the current form -- although it did occur to me
that at some point we might want something subtly different for mmap,
however, I couldn't ever work out in my head if there would be any way to
be consistent if we had mmap but no transient caching -- I guess one form
would be to only cache/buffer while file is mmapped which we should be
able to track and in that way it could be distinct from the others.

>
> Separating the two makes sense implementation-wise as well, I like this
> idea.
> What would the difference be between file_cache=writeback and loose?
> Do you plan some form of revalidation with writeback, e.g. using qid
> version that loose wouldn't do? (sorry if it's already done, I don't
> recall seeing that)
>

It would mostly have to do with when we validate the cache.  Loose
essentially means NEVER validate the cache and assume the cache
is always correct.  But in the file_cache= case we would only do it
for file contents
and not directory.

> fscache is currently a cache option but it's pretty much unrelated, we
> can have it as a separate option and alias cache=fscache to
> `file_cache=writeback(loose),dir_cache=loose,fscache=on`
> but on its own it ought to work with any level of file_cache and no
> dir_cache...
> The test matrix will be fun, though :|
>

Yeah - feels like fscache should just be seperate, but then it can follow
the consistency policies of file and/or dir as to when to revalidate with
server.

Test matrix is already a nightmare :). Right now I have a simple one
with multiple fstabs for various options (which I feed in with cpu), but
I'm gonna add this into my python notebook script so I can explore
all options (and all config options for 9p in the kernel configs) -- but
probably keep a smoke test "quick" regression as well.

> > It struck me as well with xattr enabled we may want to have separate
> > caches for xattr caching since it generates a load of traffic with
> > security on.
>
> xattr caching currently isn't done at all afaik, and it'd definitely
> make sense with any kind of dir_cache... That'd likely halve the
> requests for some find-like workloads.
> Probably another new option as well..

Yeah, I was going to tackle this after the dir cache stuff is fixed up.

      -eric