On Thu, Oct 1, 2026 at 5:09 PM Miklos Szeredi <[email protected]> wrote:
>
> Conditions are triggered by a buggy fuse server will result in -EIO which
> is hard to interpret without any context.
>
> Add helpers that print a short message to the kernel log as well as
> returning the error value. This serves a dual purpose:
>
> - documents the error condition inside the code
>
> - allows the implementor of the fuse server to get the context for the
> error
>
> Some debug messages are removed in favor of this.
>
> This also changes the mmap return value from -ETXTBSY to -ENODEV if a
> passthrough open raced with the prior check. This makes both cases return
> -ENODEV if the file is in passthrough mode.
>
> Additionally change the return value of fuse_inode_uncached_io_start() and
> fuse_file_cached_io_open() from an error value to a bool (true on success)
> as the error value is translated anyway.
>
> Currently these use the pr_notice_once() variant. Possibly should be
> changed to a ratelimit of e.g. once per minute.
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> ---
> fs/fuse/file.c | 8 ++---
> fs/fuse/fuse_i.h | 6 ++--
> fs/fuse/iomode.c | 74 ++++++++++++++++---------------------------
> fs/fuse/passthrough.c | 19 ++++-------
> 4 files changed, 42 insertions(+), 65 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 6d707f2b3bff..92976906ab05 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1478,7 +1478,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_inode_uncached_io_start(fi, NULL)) {
> inode_unlock_shared(inode);
> inode_lock(inode);
> *exclusive = true;
> @@ -2415,7 +2415,6 @@ static int fuse_file_mmap(struct file *file, struct
> vm_area_struct *vma)
> struct fuse_file *ff = file->private_data;
> struct fuse_conn *fc = ff->fm->fc;
> struct inode *inode = file_inode(file);
> - int rc;
>
> /* DAX mmap is superior to direct_io mmap */
> if (FUSE_IS_VDAX(inode))
> @@ -2457,9 +2456,8 @@ static int fuse_file_mmap(struct file *file, struct
> vm_area_struct *vma)
> * After first mmap, the inode stays in caching io mode until
> * the direct_io file release.
> */
> - rc = fuse_file_cached_io_open(inode, ff);
> - if (rc)
> - return rc;
> + if (!fuse_file_cached_io_open(inode, ff))
> + return -ENODEV;
> }
>
> if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 8546855386b5..3dd4dff24c50 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -11,6 +11,8 @@
> # define pr_fmt(fmt) "fuse: " fmt
> #endif
>
> +#define fuse_EIO(fmt, ...) (pr_notice_once("%s: " fmt "\n", __func__,
> ##__VA_ARGS__), -EIO)
> +
> #include "args.h"
> #include <linux/fuse.h>
> #include <linux/fs.h>
> @@ -1251,8 +1253,8 @@ int fuse_fileattr_set(struct mnt_idmap *idmap,
> struct dentry *dentry, struct file_kattr *fa);
>
> /* iomode.c */
> -int fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff);
> -int fuse_inode_uncached_io_start(struct fuse_inode *fi,
> +bool fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff);
> +bool fuse_inode_uncached_io_start(struct fuse_inode *fi,
> struct fuse_backing *fb);
> void fuse_inode_uncached_io_end(struct fuse_inode *fi);
>
> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> index 79637c09e883..1a10bc65fb31 100644
> --- a/fs/fuse/iomode.c
> +++ b/fs/fuse/iomode.c
> @@ -27,15 +27,15 @@ static inline bool fuse_is_io_cache_wait(struct
> fuse_inode *fi)
> * Blocks new parallel dio writes and waits for the in-progress parallel dio
> * writes to complete.
> */
> -int fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff)
> +bool fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff)
> {
> struct fuse_inode *fi = get_fuse_inode(inode);
>
> /* There are no io modes if server does not implement open */
> if (!ff->args)
> - return 0;
> + return true;
>
> - spin_lock(&fi->lock);
> + guard(spinlock)(&fi->lock);
> /*
> * Setting the bit advises new direct-io writes to use an exclusive
> * lock - without it the wait below might be forever.
> @@ -53,8 +53,7 @@ int fuse_file_cached_io_open(struct inode *inode, struct
> fuse_file *ff)
> */
> if (fuse_inode_backing(fi)) {
> clear_bit(FUSE_I_CACHE_IO_MODE, &fi->state);
> - spin_unlock(&fi->lock);
> - return -ETXTBSY;
> + return false;
> }
>
> WARN_ON(ff->iomode == IOM_UNCACHED);
> @@ -64,8 +63,7 @@ int fuse_file_cached_io_open(struct inode *inode, struct
> fuse_file *ff)
> set_bit(FUSE_I_CACHE_IO_MODE, &fi->state);
> fi->iocachectr++;
> }
> - spin_unlock(&fi->lock);
> - return 0;
> + return true;
> }
>
> static void fuse_file_cached_io_release(struct fuse_file *ff,
> @@ -82,22 +80,19 @@ static void fuse_file_cached_io_release(struct fuse_file
> *ff,
> }
>
> /* Start strictly uncached io mode where cache access is not allowed */
> -int fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing
> *fb)
> +bool fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing
> *fb)
> {
> struct fuse_backing *oldfb;
> - int err = 0;
>
> - spin_lock(&fi->lock);
> + guard(spinlock)(&fi->lock);
> /* deny conflicting backing files on same fuse inode */
> oldfb = fuse_inode_backing(fi);
> - if (fb && oldfb && oldfb != fb) {
> - err = -EBUSY;
> - goto unlock;
> - }
> - if (fi->iocachectr > 0) {
> - err = -ETXTBSY;
> - goto unlock;
> - }
> + if (fb && oldfb && oldfb != fb)
> + return false;
> +
> + if (fi->iocachectr > 0)
> + return false;
> +
> fi->iocachectr--;
>
> /* fuse inode holds a single refcount of backing file */
> @@ -107,9 +102,7 @@ int fuse_inode_uncached_io_start(struct fuse_inode *fi,
> struct fuse_backing *fb)
> } else {
> fuse_backing_put(fb);
> }
> -unlock:
> - spin_unlock(&fi->lock);
> - return err;
> + return true;
> }
>
> /* Takes uncached_io inode mode reference to be dropped on file release */
> @@ -118,11 +111,9 @@ static int fuse_file_uncached_io_open(struct inode
> *inode,
> struct fuse_backing *fb)
> {
> struct fuse_inode *fi = get_fuse_inode(inode);
> - int err;
>
> - err = fuse_inode_uncached_io_start(fi, fb);
> - if (err)
> - return err;
> + if (!fuse_inode_uncached_io_start(fi, fb))
> + return fuse_EIO("failed to start uncached I/O");
Why lose the internal error code?
If server implementation is wrong that is a good hint of what went wrong.
ETXTBSY is trying to mix cache mode and passthrough mode.
EBUSY is trying to passthrough to two different backing inodes.
I am not claiming this is a good API but at least we (or LLM) could do
something useful with this lost information.
>
> WARN_ON(ff->iomode != IOM_NONE);
> ff->iomode = IOM_UNCACHED;
> @@ -173,9 +164,11 @@ static int fuse_file_passthrough_open(struct inode
> *inode, struct file *file)
> int err;
>
> /* Check allowed conditions for file open in passthrough mode */
> - if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough ||
> - (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK))
> - return -EINVAL;
> + if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough)
> + return fuse_EIO("passthrough not enabled");
> +
> + if (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK)
> + return fuse_EIO("conflicting open flags");
>
> fb = fuse_passthrough_open(file, ff->args->open_outarg.backing_id);
> if (IS_ERR(fb))
> @@ -208,11 +201,12 @@ int fuse_file_io_open(struct file *file, struct inode
> *inode)
>
> /*
> * Server is expected to use FOPEN_PASSTHROUGH for all opens of an
> inode
> - * which is already open for passthrough.
> + * which is already open for passthrough. Using incorrect open mode
> is
> + * a server mistake, which results in user visible failure of open()
> + * with EIO error.
> */
> - err = -EINVAL;
> if (fuse_inode_backing(fi) && !(ff->open_flags & FOPEN_PASSTHROUGH))
> - goto fail;
> + return fuse_EIO("FOPEN_PASSTHROUGH expected");
>
> /*
> * FOPEN_PARALLEL_DIRECT_WRITES requires FOPEN_DIRECT_IO.
> @@ -234,22 +228,10 @@ int fuse_file_io_open(struct file *file, struct inode
> *inode)
>
> if (ff->open_flags & FOPEN_PASSTHROUGH)
> err = fuse_file_passthrough_open(inode, file);
> - else
> - err = fuse_file_cached_io_open(inode, ff);
> - if (err)
> - goto fail;
> + else if (!fuse_file_cached_io_open(inode, ff))
> + err = fuse_EIO("conflicting passthrough open");
Same here.
If we can keep this bit of information and print it - it is helpful IMO,
even if we don't go all the way to annotate every error case with
a pretty log.
Thanks,
Amir.
