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: <20220608104202.19461-1-zhangjiachen.jaycee@bytedance.com> <YrnHeqckLknFleud@redhat.com>
In-Reply-To: <YrnHeqckLknFleud@redhat.com>
From:   Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Date:   Tue, 28 Jun 2022 13:14:21 +0800
Message-ID: <CAFQAk7jgV1EkLK120tPkcG0MvupRMLQcvLNj8aP-=45nymMnQw@mail.gmail.com>
Subject: Re: Re: [PATCH] fuse: add FOPEN_INVAL_ATTR
To:     Vivek Goyal <vgoyal@redhat.com>
Cc:     Miklos Szeredi <miklos@szeredi.hu>, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jun 27, 2022 at 11:06 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Wed, Jun 08, 2022 at 06:42:02PM +0800, Jiachen Zhang wrote:
> > So that the fuse daemon can ask kernel to invalidate the attr cache on file
> > open.
>
> Will be great if there was a proper context around this. Without any
> explanation, how one is supposed to understand how this is useful.
>
> By going through other email threads, looks like, with writeback
> cache enabled, you want to invalidate attr cache and data cache
> when file is opened next time. Right?


Hi Vivek,

Yes, exactly. This patch is supposed to be a supplement to the
writeback_cache consistency enhancement patch [1]. Sorry for the lack
of explanation. I think I will send this and the enhancement patch as
a patchset later after there is more comments and discussions.

>
> IOW, when file is closed, its changes will be flushed out. And when
> file is reopened, server somehow is supposed to determine if file
> has changed (on server by another client) and based on that determine
> whether to invalidate attr and data cache on next open?

Yes, to achieve this so-called close-to-open consistency, this patch
gives a knod to FUSE server handler to invalidate attributes cache on
file open.

>
> Even without that, on next open, it probably makes sense to being
> invalidate attr cache. We have notion to invalidate data cache. So
> it will be kind of odd that we can invalidate data but not attrs
> on next open. Am I understanding it right?

Yes, Exactly. It could also be used for immediate attr invalidation in
write-through mode.

[1] https://lore.kernel.org/linux-fsdevel/20220624055825.29183-1-zhangjiachen.jaycee@bytedance.com/

Thanks,
Jiachen

>
> Thanks
> Vivek
>
> >
> > Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
> > ---
> >  fs/fuse/file.c            | 4 ++++
> >  include/uapi/linux/fuse.h | 2 ++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index fdcec3aa7830..9609d13ec351 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -213,6 +213,10 @@ void fuse_finish_open(struct inode *inode, struct file *file)
> >               file_update_time(file);
> >               fuse_invalidate_attr_mask(inode, FUSE_STATX_MODSIZE);
> >       }
> > +
> > +     if (ff->open_flags & FOPEN_INVAL_ATTR)
> > +             fuse_invalidate_attr(inode);
> > +
> >       if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
> >               fuse_link_write_file(file);
> >  }
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index d6ccee961891..0b0b7d308ddb 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -301,6 +301,7 @@ struct fuse_file_lock {
> >   * FOPEN_CACHE_DIR: allow caching this directory
> >   * FOPEN_STREAM: the file is stream-like (no file position at all)
> >   * FOPEN_NOFLUSH: don't flush data cache on close (unless FUSE_WRITEBACK_CACHE)
> > + * FOPEN_INVAL_ATTR: invalidate the attr cache on open
> >   */
> >  #define FOPEN_DIRECT_IO              (1 << 0)
> >  #define FOPEN_KEEP_CACHE     (1 << 1)
> > @@ -308,6 +309,7 @@ struct fuse_file_lock {
> >  #define FOPEN_CACHE_DIR              (1 << 3)
> >  #define FOPEN_STREAM         (1 << 4)
> >  #define FOPEN_NOFLUSH                (1 << 5)
> > +#define FOPEN_INVAL_ATTR     (1 << 6)
> >
> >  /**
> >   * INIT request/reply flags
> > --
> > 2.20.1
> >
>