Re: [PATCH v2 09/21] fuse: handle partial io passthrough for read/write, splice, and mmap

On Sat, May 16, 2026 at 9:57 AM Amir Goldstein <[email protected]> wrote:
>
> On Sat, May 16, 2026 at 2:52 AM Joanne Koong <[email protected]> wrote:
> >
> > Servers can now pass through only reads or writes instead of being
> > required to pass through both. When the read or write operation is not
> > passed through, fall back to direct io for handling it. This avoids
> > cache coherency issues from mixing the backing file's page cache with
> > the fuse inode's page cache.
> >
> > Reject mmap when passthrough is partial or not set for read/writes since
> > mmap requires both read and write access to the backing page cache.
> >
> > The FOPEN_DIRECT_IO check in the read/write fallback is not strictly
> > needed as the caller already handles it, but including it here makes
> > the fallback logic self-contained.
> >
> > Signed-off-by: Joanne Koong <[email protected]>
> > ---
> >  fs/fuse/file.c        |  7 ++++---
> >  fs/fuse/fuse_i.h      |  3 +++
> >  fs/fuse/passthrough.c | 18 ++++++++++++++++++
> >  3 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 1173811b2ea7..5b70d8272700 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1781,7 +1781,7 @@ static ssize_t __fuse_direct_read(struct fuse_io_priv *io,
> >
> >  static ssize_t fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
> >
> > -static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > +ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >  {
> >         ssize_t res;
> >
> > @@ -1796,7 +1796,7 @@ static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >         return res;
> >  }
> >
> > -static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > +ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
> >  {
> >         struct inode *inode = file_inode(iocb->ki_filp);
> >         ssize_t res;
> > @@ -1884,7 +1884,8 @@ static ssize_t fuse_splice_write(struct pipe_inode_info *pipe, struct file *out,
> >         struct fuse_file *ff = out->private_data;
> >
> >         /* FOPEN_DIRECT_IO overrides FOPEN_PASSTHROUGH */
> > -       if (fuse_file_passthrough(ff) && !(ff->open_flags & FOPEN_DIRECT_IO))
> > +       if (fuse_passthrough_op(file_inode(out), FUSE_WRITE) &&
>
> Removing fuse_file_passthrough(ff) makes me uncomfortable.
> It's true that if server sets PASSTHROUGH_OP_WRITE on lookup
> the file has to be opened with FOPEN_PASSTHROUGH, but this
> not easily understood by passing by reviewers.
>
> It's be nicer if we keep this logic contained in a helper where it can be
> properly documented, something like this (following the changes to
> FUSE_PASSTHROUGH_OP() discussed earlier):
>
> static inline u64 fuse_passthrough_op(struct inode *inode, u64 op_mask)
> {
>         struct fuse_inode *fi = get_fuse_inode(inode);
>         struct fuse_backing *fb = fuse_inode_passthrough(fi);
>
>         return !fb ? 0 : fb->ops_mask & op_mask;
> }
>
> static inline struct file *fuse_file_passthrough_op(struct file *file,
>                                                     u64 op_mask)
> {
>         struct fuse_file *ff = file->private_data;
>
>         /*
>          * FOPEN_DIRECT_IO can be used to opt-out of passthrough per file and
>          * ops_mask can be used to opt-out of passthrough per operation type.
>          */
>         return fuse_file_passthrough(ff) &&
>                 !(ff->open_flags & FOPEN_DIRECT_IO) &&
>                 fuse_passthrough_op(file_inode(file), op_mask) == op_mask;
> }
>
> ...
>
> static ssize_t fuse_splice_write(struct pipe_inode_info *pipe, struct file *out,
>                                  loff_t *ppos, size_t len, unsigned int flags)
> {
>         if (fuse_file_passthrough_op(out, FUSE_PASSTHROUGH_OP_WRITE))
>                 return fuse_passthrough_splice_write(pipe, out, ppos,
> len, flags);
>         else
>                 return iter_file_splice_write(pipe, out, ppos, len, flags);
> }
>

I really like all your suggestions, Amir. I agree that this looks much
nicer. I'll incorporate this into v3.

>
> > +           !(ff->open_flags & FOPEN_DIRECT_IO))
> >                 return fuse_passthrough_splice_write(pipe, out, ppos, len, flags);
> >         else
> >                 return iter_file_splice_write(pipe, out, ppos, len, flags);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 9e5142a94a09..a1034533ce60 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1261,6 +1261,9 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> >  void fuse_file_release(struct inode *inode, struct fuse_file *ff,
> >                        unsigned int open_flags, fl_owner_t id, bool isdir);
> >
> > +ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to);
> > +ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from);
> > +
> >  /* passthrough.c */
> >
> >  /* READ/WRITE are implied by FOPEN_PASSTHROUGH, but defined for completeness */
> > diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> > index f2025772c9c1..d62a1c751157 100644
> > --- a/fs/fuse/passthrough.c
> > +++ b/fs/fuse/passthrough.c
> > @@ -44,6 +44,10 @@ ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> >         if (!count)
> >                 return 0;
> >
> > +       if ((ff->open_flags & FOPEN_DIRECT_IO) ||
> > +           !fuse_passthrough_op(file_inode(file), FUSE_READ))
> > +               return fuse_direct_read_iter(iocb, iter);
> > +
> >         ret = backing_file_read_iter(backing_file, iter, iocb, iocb->ki_flags,
> >                                      &ctx);
> >
> > @@ -70,6 +74,10 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb,
> >         if (!count)
> >                 return 0;
> >
> > +       if ((ff->open_flags & FOPEN_DIRECT_IO) ||
> > +           !fuse_passthrough_op(inode, FUSE_WRITE))
> > +               return fuse_direct_write_iter(iocb, iter);
> > +
> >         inode_lock(inode);
> >         ret = backing_file_write_iter(backing_file, iter, iocb, iocb->ki_flags,
> >                                       &ctx);
> > @@ -91,6 +99,9 @@ ssize_t fuse_passthrough_splice_read(struct file *in, loff_t *ppos,
> >         struct kiocb iocb;
> >         ssize_t ret;
> >
> > +       if (!fuse_passthrough_op(file_inode(in), FUSE_READ))
> > +               return copy_splice_read(in, ppos, pipe, len, flags);
> > +
> >         pr_debug("%s: backing_file=0x%p, pos=%lld, len=%zu, flags=0x%x\n", __func__,
> >                  backing_file, *ppos, len, flags);
> >
>
> I understand why you brought this "jump back to direct" into the passthrough
> helpers to reduce complexity in the call sites, but I think that if you use
> the fuse_file_passthrough_op() helper in the call sites code will look
> better and will not need to "jump back to direct io" in passthrough.c.
>
> > @@ -133,6 +144,9 @@ ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma)
> >  {
> >         struct fuse_file *ff = file->private_data;
> >         struct file *backing_file = fuse_file_passthrough(ff);
> > +       struct inode *inode = file_inode(file);
> > +       struct fuse_inode *fi = get_fuse_inode(inode);
> > +       struct fuse_backing *fb = fuse_inode_backing(fi);
> >         struct backing_file_ctx ctx = {
> >                 .cred = ff->cred,
> >                 .accessed = fuse_file_accessed,
> > @@ -141,6 +155,10 @@ ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma)
> >         pr_debug("%s: backing_file=0x%p, start=%lu, end=%lu\n", __func__,
> >                  backing_file, vma->vm_start, vma->vm_end);
> >
> > +       /* mmap requires both read and write passthrough */
> > +       if ((fb->ops_mask & FUSE_PASSTHROUGH_RW_OPS) != FUSE_PASSTHROUGH_RW_OPS)
> > +               return -ENODEV;
> > +
>
> Changing the helper in call site in nicer and already hits the
> existing ENODEV case:
>
>         if (fuse_file_passthrough_op(file, FUSE_PASSTHROUGH_RW_OPS))
>                 return fuse_passthrough_mmap(file, vma);
>         else if (fuse_inode_backing(get_fuse_inode(inode)))
>                 return -ENODEV;

Nice! This looks a lot cleaner.

Thanks,
Joanne
>
> Thanks,
> Amir.