Re: [PATCH 01/28] fuse: implement the basic iomap mechanisms

Joanne Koong
2025-09-19T22:37:03Z

On Mon, Sep 15, 2025 at 5:28 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> From: Darrick J. Wong <djwong@kernel.org>
>
> Implement functions to enable upcalling of iomap_begin and iomap_end to
> userspace fuse servers.
>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/fuse/fuse_i.h | 35 ++++
> fs/fuse/iomap_priv.h | 36 ++++
> include/uapi/linux/fuse.h | 90 +++++++++
> fs/fuse/Kconfig | 32 +++
> fs/fuse/Makefile | 1
> fs/fuse/file_iomap.c | 434 +++++++++++++++++++++++++++++++++++++++++++++
> fs/fuse/inode.c | 9 +
> 7 files changed, 636 insertions(+), 1 deletion(-)
> create mode 100644 fs/fuse/iomap_priv.h
> create mode 100644 fs/fuse/file_iomap.c
>
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 4560687d619d76..f0d408a6e12c32 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -923,6 +923,9 @@ struct fuse_conn {
> /* Is synchronous FUSE_INIT allowed? */
> unsigned int sync_init:1;
>
> + /* Enable fs/iomap for file operations */
> + unsigned int iomap:1;
> +
> /* Use io_uring for communication */
> unsigned int io_uring;
>
> @@ -1047,6 +1050,11 @@ static inline struct fuse_mount *get_fuse_mount_super(struct super_block *sb)
> return sb->s_fs_info;
> }
>
> +static inline const struct fuse_mount *get_fuse_mount_super_c(const struct super_block *sb)
> +{
> + return sb->s_fs_info;
> +}
> +
> static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
> {
> return get_fuse_mount_super(sb)->fc;
> @@ -1057,16 +1065,31 @@ static inline struct fuse_mount *get_fuse_mount(struct inode *inode)
> return get_fuse_mount_super(inode->i_sb);
> }
>
> +static inline const struct fuse_mount *get_fuse_mount_c(const struct inode *inode)
> +{
> + return get_fuse_mount_super_c(inode->i_sb);
> +}
> +
> static inline struct fuse_conn *get_fuse_conn(struct inode *inode)
> {
> return get_fuse_mount_super(inode->i_sb)->fc;
> }
>
> +static inline const struct fuse_conn *get_fuse_conn_c(const struct inode *inode)
> +{
> + return get_fuse_mount_super_c(inode->i_sb)->fc;
> +}
> +
> static inline struct fuse_inode *get_fuse_inode(struct inode *inode)
> {
> return container_of(inode, struct fuse_inode, inode);
> }
>
> +static inline const struct fuse_inode *get_fuse_inode_c(const struct inode *inode)
> +{
> + return container_of(inode, struct fuse_inode, inode);
> +}

Do we need these new set of helpers? AFAICT it does two things: a)
guarantee constness of the arg passed in b) guarantee constness of the
pointer returned

But it seems like for a) we could get that by modifying the existing
functions to take in a const arg, eg

-static inline struct fuse_inode *get_fuse_inode(struct inode *inode)
+static inline struct fuse_inode *get_fuse_inode(const struct inode *inode)
{
return container_of(inode, struct fuse_inode, inode);
}

and for b) it seems to me like the caller enforces the constness of
the pointer returned whether the actual function returns a const
pointer or not,

eg
const struct fuse_inode *fi = get_fuse_inode{_c}(inode);

Maybe I'm missing something here but it seems to me like we don't need
these new helpers?

> +
> diff --git a/fs/fuse/iomap_priv.h b/fs/fuse/iomap_priv.h

btw, i think the general convention is to use "_i.h" suffixing for
private internal files, eg fuse_i.h, fuse_dev_i.h, dev_uring_i.h

> new file mode 100644
> index 00000000000000..243d92cb625095
> --- /dev/null
> +++ b/fs/fuse/iomap_priv.h
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Oracle. All Rights Reserved.
> + * Author: Darrick J. Wong <djwong@kernel.org>
> + */
> +#ifndef _FS_FUSE_IOMAP_PRIV_H
> +#define _FS_FUSE_IOMAP_PRIV_H
> +
...
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 31b80f93211b81..3634cbe602cd9c 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -235,6 +235,9 @@
> *
> * 7.44
> * - add FUSE_NOTIFY_INC_EPOCH
> + *
> + * 7.99

Just curious, where did you get the .99 from?

> + * - add FUSE_IOMAP and iomap_{begin,end,ioend} for regular file operations
> */
>
> #ifndef _LINUX_FUSE_H
> @@ -270,7 +273,7 @@
> #define FUSE_KERNEL_VERSION 7
> diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> index 9563fa5387a241..67dfe300bf2e07 100644
> --- a/fs/fuse/Kconfig
> +++ b/fs/fuse/Kconfig
> @@ -69,6 +69,38 @@ config FUSE_PASSTHROUGH
> +config FUSE_IOMAP_DEBUG
> + bool "Debug FUSE file IO over iomap"
> + default n
> + depends on FUSE_IOMAP
> + help
> + Enable debugging assertions for the fuse iomap code paths and logging
> + of bad iomap file mapping data being sent to the kernel.
> +

I wonder if we should have a general FUSE_DEBUG that this would fall
under instead of creating one that's iomap_debug specific

> config FUSE_IO_URING
> bool "FUSE communication over io-uring"
> default y
> diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> index 46041228e5be2c..27be39317701d6 100644
> --- a/fs/fuse/Makefile
> +++ b/fs/fuse/Makefile
> @@ -18,5 +18,6 @@ fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o
> fuse-$(CONFIG_FUSE_BACKING) += backing.o
> fuse-$(CONFIG_SYSCTL) += sysctl.o
> fuse-$(CONFIG_FUSE_IO_URING) += dev_uring.o
> +fuse-$(CONFIG_FUSE_IOMAP) += file_iomap.o
>
> virtiofs-y := virtio_fs.o
> diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> new file mode 100644
> index 00000000000000..dda757768d3ea6
> --- /dev/null
> +++ b/fs/fuse/file_iomap.c
> @@ -0,0 +1,434 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Oracle. All Rights Reserved.
> + * Author: Darrick J. Wong <djwong@kernel.org>
> + */
> +#include <linux/iomap.h>
> +#include "fuse_i.h"
> +#include "fuse_trace.h"
> +#include "iomap_priv.h"
> +
> +/* Validate FUSE_IOMAP_TYPE_* */
> +static inline bool fuse_iomap_check_type(uint16_t fuse_type)
> +{
> + switch (fuse_type) {
> + case FUSE_IOMAP_TYPE_HOLE:
> + case FUSE_IOMAP_TYPE_DELALLOC:
> + case FUSE_IOMAP_TYPE_MAPPED:
> + case FUSE_IOMAP_TYPE_UNWRITTEN:
> + case FUSE_IOMAP_TYPE_INLINE:
> + case FUSE_IOMAP_TYPE_PURE_OVERWRITE:
> + return true;
> + }
> +
> + return false;
> +}

Maybe faster to check by using a bitmask instead?

> +
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 1e7298b2b89b58..32f4b7c9a20a8a 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1448,6 +1448,13 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>
> if (flags & FUSE_REQUEST_TIMEOUT)
> timeout = arg->request_timeout;
> +
> + if ((flags & FUSE_IOMAP) && fuse_iomap_enabled()) {
> + fc->local_fs = 1;
> + fc->iomap = 1;
> + printk(KERN_WARNING
> + "fuse: EXPERIMENTAL iomap feature enabled. Use at your own risk!");
> + }

pr_warn() seems to be the convention elsewhere in the fuse code

Thanks,
Joanne
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> @@ -1516,6 +1523,8 @@ static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
> */
> if (fuse_uring_enabled())
> flags |= FUSE_OVER_IO_URING;
> + if (fuse_iomap_enabled())
> + flags |= FUSE_IOMAP;
>
> ia->in.flags = flags;
> ia->in.flags2 = flags >> 32;
>