Re: [PATCH 10/31] fuse: implement basic iomap reporting such as FIEMAP and SEEK_{DATA,HOLE}

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 21, 2026 at 06:07:12PM -0800, Joanne Koong wrote:
> On Tue, Oct 28, 2025 at 5:47 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Implement the basic file mapping reporting functions like FIEMAP, BMAP,
> > and SEEK_DATA/HOLE.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  fs/fuse/fuse_i.h     |    8 ++++++
> >  fs/fuse/dir.c        |    1 +
> >  fs/fuse/file.c       |   13 ++++++++++
> >  fs/fuse/file_iomap.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  4 files changed, 89 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index c7aeb324fe599e..6fe8aa1845b98d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1730,6 +1730,11 @@ static inline bool fuse_inode_has_iomap(const struct inode *inode)
> >
> >         return test_bit(FUSE_I_IOMAP, &fi->state);
> >  }
> > +
> > +int fuse_iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > +                     u64 start, u64 length);
> > +loff_t fuse_iomap_lseek(struct file *file, loff_t offset, int whence);
> > +sector_t fuse_iomap_bmap(struct address_space *mapping, sector_t block);
> >  #else
> >  # define fuse_iomap_enabled(...)               (false)
> >  # define fuse_has_iomap(...)                   (false)
> > @@ -1739,6 +1744,9 @@ static inline bool fuse_inode_has_iomap(const struct inode *inode)
> >  # define fuse_iomap_init_nonreg_inode(...)     ((void)0)
> >  # define fuse_iomap_evict_inode(...)           ((void)0)
> >  # define fuse_inode_has_iomap(...)             (false)
> > +# define fuse_iomap_fiemap                     NULL
> > +# define fuse_iomap_lseek(...)                 (-ENOSYS)
> > +# define fuse_iomap_bmap(...)                  (-ENOSYS)
> >  #endif
> >
> >  #endif /* _FS_FUSE_I_H */
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index 18eb1bb192bb58..bafc386f2f4d3a 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -2296,6 +2296,7 @@ static const struct inode_operations fuse_common_inode_operations = {
> >         .set_acl        = fuse_set_acl,
> >         .fileattr_get   = fuse_fileattr_get,
> >         .fileattr_set   = fuse_fileattr_set,
> > +       .fiemap         = fuse_iomap_fiemap,
> >  };
> >
> >  static const struct inode_operations fuse_symlink_inode_operations = {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index bd9c208a46c78d..8a981f41b1dbd0 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -2512,6 +2512,12 @@ static sector_t fuse_bmap(struct address_space *mapping, sector_t block)
> >         struct fuse_bmap_out outarg;
> >         int err;
> >
> > +       if (fuse_inode_has_iomap(inode)) {
> > +               sector_t alt_sec = fuse_iomap_bmap(mapping, block);
> > +               if (alt_sec > 0)
> > +                       return alt_sec;
> > +       }
> > +
> >         if (!inode->i_sb->s_bdev || fm->fc->no_bmap)
> >                 return 0;
> >
> > @@ -2547,6 +2553,13 @@ static loff_t fuse_lseek(struct file *file, loff_t offset, int whence)
> >         struct fuse_lseek_out outarg;
> >         int err;
> >
> > +       if (fuse_inode_has_iomap(inode)) {
> > +               loff_t alt_pos = fuse_iomap_lseek(file, offset, whence);
> > +
> > +               if (alt_pos >= 0 || (alt_pos < 0 && alt_pos != -ENOSYS))
> 
> I don't think you technically need the "alt_pos < 0" part here since
> the  "alt_pos >= 0 ||" part already accounts for that

alt_pos is loff_t, which is a signed type.

But I think this could be more concise:

		alt_pos = fuse_iomap_lseek(...);
		if (alt_pos != -ENOSYS)
			return alt_pos;

> > +                       return alt_pos;
> > +       }
> > +
> >         if (fm->fc->no_lseek)
> >                 goto fallback;
> >
> > diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> > index 66a7b8faa31ac2..ce64e7c4860ef8 100644
> > --- a/fs/fuse/file_iomap.c
> > +++ b/fs/fuse/file_iomap.c
> > @@ -4,6 +4,7 @@
> >   * Author: Darrick J. Wong <djwong@kernel.org>
> >   */
> >  #include <linux/iomap.h>
> > +#include <linux/fiemap.h>
> >  #include "fuse_i.h"
> >  #include "fuse_trace.h"
> >  #include "iomap_i.h"
> > @@ -561,7 +562,7 @@ static int fuse_iomap_end(struct inode *inode, loff_t pos, loff_t count,
> >         return err;
> >  }
> >
> > -const struct iomap_ops fuse_iomap_ops = {
> > +static const struct iomap_ops fuse_iomap_ops = {
> >         .iomap_begin            = fuse_iomap_begin,
> >         .iomap_end              = fuse_iomap_end,
> >  };
> > @@ -690,3 +691,68 @@ void fuse_iomap_evict_inode(struct inode *inode)
> >         if (conn->iomap && fuse_inode_is_exclusive(inode))
> >                 clear_bit(FUSE_I_EXCLUSIVE, &fi->state);
> >  }
> > +
> > +int fuse_iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > +                     u64 start, u64 count)
> > +{
> > +       struct fuse_conn *fc = get_fuse_conn(inode);
> > +       int error;
> > +
> > +       /*
> > +        * We are called directly from the vfs so we need to check per-inode
> > +        * support here explicitly.
> > +        */
> > +       if (!fuse_inode_has_iomap(inode))
> > +               return -EOPNOTSUPP;
> > +
> > +       if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR)
> 
> I don't see where FIEMAP_FLAG_SYNC and FIEMAP_FLAG_CACHE are supported
> either, should these return -EOPNOTSUPP if they're set as well?

The vfs implements FIEMAP_FLAG_SYNC for us in fiemap_prep, which is
called by iomap_fiemap.

I'm not sure what FIEMAP_FLAG_CACHE means in this context.  Its comment
says "request caching of the extents" which doesn't sound like doing
anything is mandatory.

> > +               return -EOPNOTSUPP;
> > +
> > +       if (fuse_is_bad(inode))
> > +               return -EIO;
> > +
> > +       if (!fuse_allow_current_process(fc))
> > +               return -EACCES;
> > +
> > +       inode_lock_shared(inode);
> > +       error = iomap_fiemap(inode, fieinfo, start, count, &fuse_iomap_ops);
> > +       inode_unlock_shared(inode);
> > +
> > +       return error;
> > +}
> > +
> > +sector_t fuse_iomap_bmap(struct address_space *mapping, sector_t block)
> > +{
> > +       ASSERT(fuse_inode_has_iomap(mapping->host));
> > +
> > +       return iomap_bmap(mapping, block, &fuse_iomap_ops);
> > +}
> > +
> > +loff_t fuse_iomap_lseek(struct file *file, loff_t offset, int whence)
> > +{
> > +       struct inode *inode = file->f_mapping->host;
> > +       struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > +       ASSERT(fuse_inode_has_iomap(inode));
> > +
> > +       if (fuse_is_bad(inode))
> > +               return -EIO;
> > +
> > +       if (!fuse_allow_current_process(fc))
> > +               return -EACCES;
> > +
> > +       switch (whence) {
> > +       case SEEK_HOLE:
> > +               offset = iomap_seek_hole(inode, offset, &fuse_iomap_ops);
> > +               break;
> > +       case SEEK_DATA:
> > +               offset = iomap_seek_data(inode, offset, &fuse_iomap_ops);
> > +               break;
> > +       default:
> 
> Does it make sense to have the default case just call generic_file_llseek()?

Yes.  Thanks for spotting that bug!

--D

> Thanks,
> Joanne
> 
> > +               return -ENOSYS;
> > +       }
> > +
> > +       if (offset < 0)
> > +               return offset;
> > +       return vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
> > +}
> >
>