Re: [PATCH v3 2/2] fuse: allow parallel direct writes in passthrough write_iter

[Amir Goldstein]

On Wed, Jun 17, 2026 at 1:13 AM Russ Fellows <russ.fellows@xxxxxxxxx> wrote:
>
> fuse_passthrough_write_iter() unconditionally called fuse_dio_lock()
> from file.c, which required those helpers to be exported.  That coupling
> is unnecessary and the exported symbols are undesirable.
>
> Replace the fuse_dio_lock()/fuse_dio_unlock() calls with a
> passthrough-specific pair, fuse_passthrough_lock() and
> fuse_passthrough_unlock(), that is self-contained in passthrough.c.
>
> The new fuse_passthrough_lock() allows shared inode locking only when
> all of the following are true:
>
>   - the open carries FOPEN_PARALLEL_DIRECT_WRITES
>   - the write is direct I/O (IOCB_DIRECT)
>   - the write is not append (IOCB_APPEND absent)
>   - the write does not extend past EOF
>
> The past-EOF check is made once before the lock is taken (fast path
> to choose lock type), and then re-checked after taking the shared lock.
> This re-check closes the TOCTOU window: a concurrent writer could extend
> EOF between the initial check and the lock acquisition; without the
> re-check a shared-lock writer could concurrently update i_size.
>
> Passthrough files are always in uncached iomode (established at open
> time via fuse_file_uncached_io_open()), so the fuse_inode_uncached_io_start()
> guard from fuse_dio_lock() is not needed here.
>
> Restore fuse_dio_lock() and fuse_dio_unlock() to file-private (static).
> Remove their declarations from fuse_i.h.
>
> Signed-off-by: Russ Fellows <russ.fellows@xxxxxxxxx>
> ---
>  fs/fuse/file.c        |  6 ++--
>  fs/fuse/fuse_i.h      |  2 --
>  fs/fuse/passthrough.c | 74 +++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 75 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 7cba331d0..f8651a195 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1366,8 +1366,8 @@ static bool fuse_dio_wr_exclusive_lock(struct kiocb *iocb, struct iov_iter *from
>         return false;
>  }
>
> -void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
> -                  bool *exclusive)
> +static void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
> +                         bool *exclusive)
>  {
>         struct inode *inode = file_inode(iocb->ki_filp);
>         struct fuse_inode *fi = get_fuse_inode(inode);
> @@ -1393,7 +1393,7 @@ void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
>         }
>  }
>
> -void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
> +static void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
>  {
>         struct inode *inode = file_inode(iocb->ki_filp);
>         struct fuse_inode *fi = get_fuse_inode(inode);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 8d05c7c52..cc428d04b 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1507,8 +1507,6 @@ int fuse_file_io_open(struct file *file, struct inode *inode);
>  void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
>
>  /* file.c */
> -void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from, bool *exclusive);
> -void fuse_dio_unlock(struct kiocb *iocb, bool exclusive);
>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>                                  unsigned int open_flags, bool isdir);
>  void fuse_file_release(struct inode *inode, struct fuse_file *ff,
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index ee822a983..11c16de4d 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -25,6 +25,76 @@ static void fuse_passthrough_end_write(struct kiocb *iocb, ssize_t ret)
>         fuse_write_update_attr(inode, iocb->ki_pos, ret);
>  }
>
> +static bool fuse_passthrough_io_past_eof(struct kiocb *iocb,
> +                                        struct iov_iter *iter)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       return iocb->ki_pos + iov_iter_count(iter) > i_size_read(inode);
> +}
> +
> +/*
> + * Decide whether an exclusive inode lock is required for a passthrough write
> + * before the lock is taken.  Returns true (exclusive needed) unless all of:
> + *   - server advertised FOPEN_PARALLEL_DIRECT_WRITES
> + *   - write is direct I/O (not buffered)
> + *   - write is not append
> + *   - write does not appear to extend past EOF (re-checked after lock below)
> + */
> +static bool fuse_passthrough_write_needs_exclusive(struct kiocb *iocb,
> +                                                  struct iov_iter *iter)
> +{
> +       struct fuse_file *ff = iocb->ki_filp->private_data;
> +
> +       if (!(ff->open_flags & FOPEN_PARALLEL_DIRECT_WRITES))
> +               return true;
> +
> +       if (!(iocb->ki_flags & IOCB_DIRECT))
> +               return true;
> +       if (iocb->ki_flags & IOCB_APPEND)
> +               return true;
> +       if (fuse_passthrough_io_past_eof(iocb, iter))
> +               return true;
> +
> +       return false;
> +}
> +
> +static void fuse_passthrough_lock(struct kiocb *iocb, struct iov_iter *iter,
> +                                 bool *exclusive)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       *exclusive = fuse_passthrough_write_needs_exclusive(iocb, iter);
> +       if (*exclusive) {
> +               inode_lock(inode);
> +       } else {
> +               inode_lock_shared(inode);
> +               /*
> +                * The past-EOF check in fuse_passthrough_write_needs_exclusive()
> +                * was made without holding the inode lock and may have raced
> +                * with a concurrent EOF-extending write.  Re-check under the
> +                * shared lock and upgrade to exclusive if the write now reaches
> +                * past EOF, to ensure i_size is never updated without exclusive
> +                * serialisation.
> +                */
> +               if (fuse_passthrough_io_past_eof(iocb, iter)) {
> +                       inode_unlock_shared(inode);
> +                       inode_lock(inode);
> +                       *exclusive = true;
> +               }
> +       }
> +}
> +
> +static void fuse_passthrough_unlock(struct kiocb *iocb, bool exclusive)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       if (exclusive)
> +               inode_unlock(inode);
> +       else
> +               inode_unlock_shared(inode);
> +}
> +

I'll take the blame for phrasing my review comment
"Either you duplicate fuse_dio_lock() -> fuse_passthrough_lock()
or you find a way to have the two share common helper (prefered)"

I should have phrased it stronger.
Unless there is a very good reason not to share a common helper
we need to use a common helper and not duplicate 99% identical code.

See my untested patch trying to do that below.

Thanks,
Amir.

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index f94f3dc082c6b..6909b4bbba685 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1397,19 +1397,24 @@ static bool fuse_io_past_eof(struct kiocb
*iocb, struct iov_iter *iter)
 }

 /*
- * @return true if an exclusive lock for direct IO writes is needed
+ * @return true if an exclusive lock for direct IO or passthrough
writes is needed
  */
-static bool fuse_dio_wr_exclusive_lock(struct kiocb *iocb, struct
iov_iter *from)
+static bool fuse_io_wr_exclusive_lock(struct kiocb *iocb, struct
iov_iter *from)
 {
        struct file *file = iocb->ki_filp;
        struct fuse_file *ff = file->private_data;
        struct inode *inode = file_inode(iocb->ki_filp);
        struct fuse_inode *fi = get_fuse_inode(inode);
+       bool fopen_direct_io = ff->open_flags & FOPEN_DIRECT_IO;

        /* Server side has to advise that it supports parallel dio writes. */
        if (!(ff->open_flags & FOPEN_PARALLEL_DIRECT_WRITES))
                return true;

+       /* Passthrough mode supports parallel IOCB_DIRECT writes */
+       if (!fopen_direct_io && !(iocb->ki_flags & IOCB_DIRECT))
+               return true;
+
        /*
         * Append will need to know the eventual EOF - always needs an
         * exclusive lock.
@@ -1428,13 +1433,33 @@ static bool fuse_dio_wr_exclusive_lock(struct
kiocb *iocb, struct iov_iter *from
        return false;
 }

 static void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
-                         bool *exclusive)
+static int fuse_parallel_dio_start(struct fuse_file *ff, struct fuse_inode *fi)
+{
+       /* Permanent uncached mode from passthrough open */
+       if (ff->iomode == IOM_UNCACHED)
+               return 0;
+
+       return fuse_inode_uncached_io_start(fi, NULL);
+}
+
+static void fuse_parallel_dio_end(struct fuse_file *ff, struct fuse_inode *fi)
+{
+       /* Permanent uncached mode from passthrough open */
+       if (ff->iomode == IOM_UNCACHED)
+               return;
+
+       /* Allow opens in caching mode after last parallel dio end */
+       fuse_inode_uncached_io_end(fi);
+}
+
+/* Take shared/exclusive lock for direct IO or passthrough write */
+void fuse_io_wr_lock(struct kiocb *iocb, struct iov_iter *from, bool
*exclusive)
 {
+       struct fuse_file *ff = iocb->ki_filp->private_data;
        struct inode *inode = file_inode(iocb->ki_filp);
        struct fuse_inode *fi = get_fuse_inode(inode);

-       *exclusive = fuse_dio_wr_exclusive_lock(iocb, from);
+       *exclusive = fuse_io_wr_exclusive_lock(iocb, from);
        if (*exclusive) {
                inode_lock(inode);
        } else {
@@ -1447,7 +1472,7 @@ static void fuse_dio_lock(struct kiocb *iocb,
struct iov_iter *from,
                 * have raced, so check it again.
                 */
                if (fuse_io_past_eof(iocb, from) ||
-                   fuse_inode_uncached_io_start(fi, NULL) != 0) {
+                   fuse_parallel_dio_start(ff, fi) != 0) {
                        inode_unlock_shared(inode);
                        inode_lock(inode);
                        *exclusive = true;
@@ -1455,16 +1480,17 @@ static void fuse_dio_lock(struct kiocb *iocb,
struct iov_iter *from,
        }
 }

-static void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
+/* Release shared/exclusive lock taken for direct IO or passthrough write */
+void fuse_io_wr_unlock(struct kiocb *iocb, bool exclusive)
 {
+       struct fuse_file *ff = iocb->ki_filp->private_data;
        struct inode *inode = file_inode(iocb->ki_filp);
        struct fuse_inode *fi = get_fuse_inode(inode);

        if (exclusive) {
                inode_unlock(inode);
        } else {
-               /* Allow opens in caching mode after last parallel dio end */
-               fuse_inode_uncached_io_end(fi);
+               fuse_parallel_dio_end(ff, fi);
                inode_unlock_shared(inode);
        }
 }
@@ -1793,7 +1819,7 @@ static ssize_t fuse_direct_write_iter(struct
kiocb *iocb, struct iov_iter *from)
        ssize_t res;
        bool exclusive;

-       fuse_dio_lock(iocb, from, &exclusive);
+       fuse_io_wr_lock(iocb, from, &exclusive);
        res = generic_write_checks(iocb, from);
        if (res > 0) {
                task_io_account_write(res);
@@ -1807,7 +1833,7 @@ static ssize_t fuse_direct_write_iter(struct
kiocb *iocb, struct iov_iter *from)
                        fuse_write_update_attr(inode, iocb->ki_pos, res);
                }
        }
-       fuse_dio_unlock(iocb, exclusive);
+       fuse_io_wr_unlock(iocb, exclusive);

        return res;
 }