On Thu, May 29, 2025 at 3:15 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> On Wed, May 21, 2025 at 5:03 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> >
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Implement functions to enable upcalling of iomap_begin and iomap_end to
> > userspace fuse servers.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > ---
> > fs/fuse/fuse_i.h | 38 ++++++
> > fs/fuse/fuse_trace.h | 258 +++++++++++++++++++++++++++++++++++++++++
> > include/uapi/linux/fuse.h | 87 ++++++++++++++
> > fs/fuse/Kconfig | 23 ++++
> > fs/fuse/Makefile | 1
> > fs/fuse/file_iomap.c | 280 +++++++++++++++++++++++++++++++++++++++++++++
> > fs/fuse/inode.c | 5 +
> > 7 files changed, 691 insertions(+), 1 deletion(-)
> > create mode 100644 fs/fuse/file_iomap.c
> >
> >
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index d56d4fd956db99..aa51f25856697d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -895,6 +895,9 @@ struct fuse_conn {
> > /* Is link not implemented by fs? */
> > unsigned int no_link:1;
> >
> > + /* Use fs/iomap for FIEMAP and SEEK_{DATA,HOLE} file operations */
> > + unsigned int iomap:1;
> > +
> > /* Use io_uring for communication */
> > unsigned int io_uring;
> >
> > @@ -1017,6 +1020,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;
> > +}
> > +
>
> Instead of adding this new helper (and the ones below), what about
> modifying the existing (non-const) versions of these helpers to take
> in const * input args, eg
>
> -static inline struct fuse_mount *get_fuse_mount_super(struct super_block *sb)
> +static inline struct fuse_mount *get_fuse_mount_super(const struct
> super_block *sb)
> {
> return sb->s_fs_info;
> }
>
> Then, doing something like "const struct fuse_mount *mt =
> get_fuse_mount(inode);" would enforce the same guarantees as "const
> struct fuse_mount *mt = get_fuse_mount_c(inode);" and we wouldn't need
> 2 sets of helpers that pretty much do the same thing.
>
> > static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
> > {
> > return get_fuse_mount_super(sb)->fc;
> > @@ -1027,16 +1035,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);
> > +}
> > +
> > static inline u64 get_node_id(struct inode *inode)
> > {
> > return get_fuse_inode(inode)->nodeid;
> > @@ -1577,4 +1600,19 @@ extern void fuse_sysctl_unregister(void);
> > #define fuse_sysctl_unregister() do { } while (0)
> > #endif /* CONFIG_SYSCTL */
> >
> > +#if IS_ENABLED(CONFIG_FUSE_IOMAP)
> > +# include <linux/fiemap.h>
> > +# include <linux/iomap.h>
> > +
> > +bool fuse_iomap_enabled(void);
> > +
> > +static inline bool fuse_has_iomap(const struct inode *inode)
> > +{
> > + return get_fuse_conn_c(inode)->iomap;
> > +}
> > +#else
> > +# define fuse_iomap_enabled(...) (false)
> > +# define fuse_has_iomap(...) (false)
> > +#endif
> > +
> > #endif /* _FS_FUSE_I_H */
> > diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> > index ca215a3cba3e31..fc7c5bf1cef52d 100644
> > --- a/fs/fuse/Kconfig
> > +++ b/fs/fuse/Kconfig
> > @@ -64,6 +64,29 @@ config FUSE_PASSTHROUGH
> >
> > If you want to allow passthrough operations, answer Y.
> >
> > +config FUSE_IOMAP
> > + bool "FUSE file IO over iomap"
> > + default y
> > + depends on FUSE_FS
> > + select FS_IOMAP
> > + help
> > + For supported fuseblk servers, this allows the file IO path to run
> > + through the kernel.
>
> I have config FUSE_FS select FS_IOMAP in my patchset (not yet
> submitted) that changes fuse buffered writes / writeback handling to
> use iomap. Could we just have config FUSE_FS automatically opt into
> FS_IOMAP here or do you see a reason that this needs to be a separate
> config?
Thinking about it some more, the iomap stuff you're adding also
requires a "depends on BLOCK", so this will need to be a separate
config anyways regardless of whether the FUSE_FS will always "select
FS_IOMAP"
Thanks,
Joanne
>
>
> Thanks,
> Joanne
> > +
> > +config FUSE_IOMAP_BY_DEFAULT
> > + bool "FUSE file I/O over iomap by default"
> > + default n
> > + depends on FUSE_IOMAP
> > + help
> > + Enable sending FUSE file I/O over iomap by default.
> > +
> > +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.
> > +
> > config FUSE_IO_URING
> > bool "FUSE communication over io-uring"
> > default y
