Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220325132126.61949-1-zhangjiachen.jaycee@bytedance.com>
 <CAJfpeguESQm1KsQLyoMRTevLttV8N8NTGsb2tRbNS1AQ_pNAww@mail.gmail.com>
 <CAFQAk7ibzCn8OD84-nfg6_AePsKFTu9m7pXuQwcQP5OBp7ZCag@mail.gmail.com>
 <CAJfpegsbaz+RRcukJEOw+H=G3ft43vjDMnJ8A24JiuZFQ24eHA@mail.gmail.com>
 <CAFQAk7hakYNfBaOeMKRmMPTyxFb2xcyUTdugQG1D6uZB_U1zBg@mail.gmail.com>
 <Ymfu8fGbfYi4FxQ4@miu.piliscsaba.redhat.com> <YrnCfISl7Nl8Wk52@redhat.com>
In-Reply-To: <YrnCfISl7Nl8Wk52@redhat.com>
From:   Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Date:   Tue, 28 Jun 2022 18:46:12 +0800
Message-ID: <CAFQAk7iVKxXGyybJ8OB0sUL1zq=aXSO32OEv3te-80sFNgbRMw@mail.gmail.com>
Subject: Re: Re: Re: Re: [RFC PATCH] fuse: support cache revalidation in
 writeback_cache mode
To:     Vivek Goyal <vgoyal@redhat.com>
Cc:     Miklos Szeredi <miklos@szeredi.hu>, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, Xie Yongji <xieyongji@bytedance.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jun 27, 2022 at 10:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Tue, Apr 26, 2022 at 03:09:05PM +0200, Miklos Szeredi wrote:
> > On Mon, Apr 25, 2022 at 09:52:44PM +0800, Jiachen Zhang wrote:
> >
> > > Some users may want both the high performance of writeback mode and a
> > > little bit more consistency among FUSE mounts. In the current
> > > writeback mode implementation, users of one FUSE mount can never see
> > > the file expansion done by other FUSE mounts.
> >
> > Okay.
> >
> > Here's a preliminary patch that you could try.
> >
> > Thanks,
> > Miklos
> >
> > ---
> >  fs/fuse/dir.c             |   35 ++++++++++++++++++++++-------------
> >  fs/fuse/file.c            |   17 +++++++++++++++--
> >  fs/fuse/fuse_i.h          |   14 +++++++++++++-
> >  fs/fuse/inode.c           |   32 +++++++++++++++++++++++++++-----
> >  include/uapi/linux/fuse.h |    5 +++++
> >  5 files changed, 82 insertions(+), 21 deletions(-)
> >
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -194,6 +194,7 @@
> >   *  - add FUSE_SECURITY_CTX init flag
> >   *  - add security context to create, mkdir, symlink, and mknod requests
> >   *  - add FUSE_HAS_INODE_DAX, FUSE_ATTR_DAX
> > + *  - add FUSE_WRITEBACK_CACHE_V2 init flag
> >   */
> >
> >  #ifndef _LINUX_FUSE_H
> > @@ -353,6 +354,9 @@ struct fuse_file_lock {
> >   * FUSE_SECURITY_CTX:        add security context to create, mkdir, symlink, and
> >   *                   mknod
> >   * FUSE_HAS_INODE_DAX:  use per inode DAX
> > + * FUSE_WRITEBACK_CACHE_V2:
> > + *                   - allow time/size to be refreshed if no pending write
> > + *                   - time/size not cached for falocate/copy_file_range
> >   */
> >  #define FUSE_ASYNC_READ              (1 << 0)
> >  #define FUSE_POSIX_LOCKS     (1 << 1)
> > @@ -389,6 +393,7 @@ struct fuse_file_lock {
> >  /* bits 32..63 get shifted down 32 bits into the flags2 field */
> >  #define FUSE_SECURITY_CTX    (1ULL << 32)
> >  #define FUSE_HAS_INODE_DAX   (1ULL << 33)
> > +#define FUSE_WRITEBACK_CACHE_V2      (1ULL << 34)
> >
> >  /**
> >   * CUSE INIT request/reply flags
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -222,19 +222,37 @@ void fuse_change_attributes_common(struc
> >  u32 fuse_get_cache_mask(struct inode *inode)
> >  {
> >       struct fuse_conn *fc = get_fuse_conn(inode);
> > +     struct fuse_inode *fi = get_fuse_inode(inode);
> >
> >       if (!fc->writeback_cache || !S_ISREG(inode->i_mode))
> >               return 0;
> >
> > +     /*
> > +      * In writeback_cache_v2 mode if all the following conditions are met,
> > +      * then allow the attributes to be refreshed:
> > +      *
> > +      * - inode is not dirty (I_DIRTY_INODE)
> > +      * - inode is not in the process of being written (I_SYNC)
> > +      * - inode has no dirty pages (I_DIRTY_PAGES)
> > +      * - inode does not have any page writeback in progress
> > +      *
> > +      * Note: checking PAGECACHE_TAG_WRITEBACK is not sufficient in fuse,
> > +      * since inode can appear to have no PageWriteback pages, yet still have
> > +      * outstanding write request.
> > +      */
>
> Hi,
>
> I started following this thread just now after Jiachen pointed me to
> previous conversations. Without going into too much details.
>
> Based on above description, so we will update mtime/ctime/i_size only
> if inode does not have dirty pages or nothing is in progress. So that
> means sometime we will update it and other times we will ignore it.
>
> Do I understand it correctly. I am wondering how that is useful to
> applications.
>
> I thought that other remote filesystems might have leasing for this so
> that one client can acquire the lease and cache changes and when lease
> is broken, this client pushes out all the changes and other client gets
> the lease.
>
> Given we don't have any lease mechanism, we probably need to define the
> semantics more clearly and we should probably document it as well.
>

Hi Vivek,

I agree we should define or document the semantics properly. For now,
it seems that Miklos' writeback_mode_v2 is making best-effort updating
when pages are not dirty and a set of new attributes are returned from
FUSE server.

Thanks,
Jiachen

> Thanks
> Vivek
>