Re: [fuse-devel] [PATCH] fuse: add a new flag to allow shared mmap in FOPEN_DIRECT_IO mode

[Miklos Szeredi <miklos@szeredi.hu>]

On Thu, 8 Jun 2023 at 23:29, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 6/8/23 09:17, Hao Xu wrote:
> > ping...
> >
> > On 5/5/23 16:16, Hao Xu wrote:
> >> From: Hao Xu <howeyxu@tencent.com>
> >>
> >> FOPEN_DIRECT_IO is usually set by fuse daemon to indicate need of strong
> >> coherency, e.g. network filesystems. Thus shared mmap is disabled since
> >> it leverages page cache and may write to it, which may cause
> >> inconsistence. But FOPEN_DIRECT_IO can be used not for coherency but to
> >> reduce memory footprint as well, e.g. reduce guest memory usage with
> >> virtiofs. Therefore, add a new flag FOPEN_DIRECT_IO_SHARED_MMAP to allow
> >> shared mmap for these cases.
> >>
> >> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> >> ---
> >>   fs/fuse/file.c            | 11 ++++++++---
> >>   include/uapi/linux/fuse.h |  2 ++
> >>   2 files changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >> index 89d97f6188e0..655896bdb0d5 100644
> >> --- a/fs/fuse/file.c
> >> +++ b/fs/fuse/file.c
> >> @@ -161,7 +161,8 @@ struct fuse_file *fuse_file_open(struct fuse_mount
> >> *fm, u64 nodeid,
> >>       }
> >>       if (isdir)
> >> -        ff->open_flags &= ~FOPEN_DIRECT_IO;
> >> +        ff->open_flags &=
> >> +            ~(FOPEN_DIRECT_IO | FOPEN_DIRECT_IO_SHARED_MMAP);
> >>       ff->nodeid = nodeid;
> >> @@ -2509,8 +2510,12 @@ static int fuse_file_mmap(struct file *file,
> >> struct vm_area_struct *vma)
> >>           return fuse_dax_mmap(file, vma);
> >>       if (ff->open_flags & FOPEN_DIRECT_IO) {
> >> -        /* Can't provide the coherency needed for MAP_SHARED */
> >> -        if (vma->vm_flags & VM_MAYSHARE)
> >> +        /* Can't provide the coherency needed for MAP_SHARED.
> >> +         * So disable it if FOPEN_DIRECT_IO_SHARED_MMAP is not
> >> +         * set, which means we do need strong coherency.
> >> +         */
> >> +        if (!(ff->open_flags & FOPEN_DIRECT_IO_SHARED_MMAP) &&
> >> +            vma->vm_flags & VM_MAYSHARE)
> >>               return -ENODEV;
> >>           invalidate_inode_pages2(file->f_mapping);
> >> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> >> index 1b9d0dfae72d..003dcf42e8c2 100644
> >> --- a/include/uapi/linux/fuse.h
> >> +++ b/include/uapi/linux/fuse.h
> >> @@ -314,6 +314,7 @@ struct fuse_file_lock {
> >>    * 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_PARALLEL_DIRECT_WRITES: Allow concurrent direct writes on
> >> the same inode
> >> + * FOPEN_DIRECT_IO_SHARED_MMAP: allow shared mmap when
> >> FOPEN_DIRECT_IO is set
> >>    */
> >>   #define FOPEN_DIRECT_IO        (1 << 0)
> >>   #define FOPEN_KEEP_CACHE    (1 << 1)
> >> @@ -322,6 +323,7 @@ struct fuse_file_lock {
> >>   #define FOPEN_STREAM        (1 << 4)
> >>   #define FOPEN_NOFLUSH        (1 << 5)
> >>   #define FOPEN_PARALLEL_DIRECT_WRITES    (1 << 6)
> >> +#define FOPEN_DIRECT_IO_SHARED_MMAP    (1 << 7)
> >>   /**
> >>    * INIT request/reply flags
> >
> >
> >
>
> Sorry, currently get distracted by non-fuse work :/
>
> I think see the reply from Miklos on the initial question, which is on
> fuse-devel. Ah, I see you replied to it.
>
> https://sourceforge.net/p/fuse/mailman/message/37849170/
>
>
> I think what Miklos asks for, is to add a new FUSE_INIT reply flag and
> then to set something like fc->dio_shared_mmap. That way it doesn't need
> to be set for each open, but only once in the server init handler.
> @Miklos, please correct me if I'm wrong.

Yes, that's exactly what I suggested.

Thanks,
Miklos