On Fri, Jun 27, 2025 at 09:02:43AM +0200, Christoph Hellwig wrote:
> The iomap_folio_ops are only used for buffered writes, including
> the zero and unshare variants. Rename them to iomap_write_ops
> to better describe the usage, and pass them through the callchain
> like the other operation specific methods instead of through the
> iomap.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> Documentation/filesystems/iomap/design.rst | 3 -
> .../filesystems/iomap/operations.rst | 8 +-
> block/fops.c | 3 +-
> fs/gfs2/bmap.c | 21 ++---
> fs/gfs2/bmap.h | 1 +
> fs/gfs2/file.c | 3 +-
> fs/iomap/buffered-io.c | 79 +++++++++++--------
> fs/xfs/xfs_file.c | 6 +-
> fs/xfs/xfs_iomap.c | 12 ++-
> fs/xfs/xfs_iomap.h | 1 +
> fs/xfs/xfs_reflink.c | 3 +-
> fs/zonefs/file.c | 3 +-
> include/linux/iomap.h | 22 +++---
> 13 files changed, 89 insertions(+), 76 deletions(-)
>
...
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index ff05e6b1b0bb..2e94a9435002 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -79,6 +79,9 @@ xfs_iomap_valid(
> {
> struct xfs_inode *ip = XFS_I(inode);
>
> + if (iomap->type == IOMAP_HOLE)
> + return true;
> +
Is this to handle the xfs_hole_to_iomap() case? I.e., no validity cookie
and no folio_ops set..? If so, I think a small comment would be helpful.
Otherwise LGTM:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> if (iomap->validity_cookie !=
> xfs_iomap_inode_sequence(ip, iomap->flags)) {
> trace_xfs_iomap_invalid(ip, iomap);
> @@ -89,7 +92,7 @@ xfs_iomap_valid(
> return true;
> }
>
> -static const struct iomap_folio_ops xfs_iomap_folio_ops = {
> +const struct iomap_write_ops xfs_iomap_write_ops = {
> .iomap_valid = xfs_iomap_valid,
> };
>
> @@ -151,7 +154,6 @@ xfs_bmbt_to_iomap(
> iomap->flags |= IOMAP_F_DIRTY;
>
> iomap->validity_cookie = sequence_cookie;
> - iomap->folio_ops = &xfs_iomap_folio_ops;
> return 0;
> }
>
> @@ -2198,7 +2200,8 @@ xfs_zero_range(
> return dax_zero_range(inode, pos, len, did_zero,
> &xfs_dax_write_iomap_ops);
> return iomap_zero_range(inode, pos, len, did_zero,
> - &xfs_buffered_write_iomap_ops, ac);
> + &xfs_buffered_write_iomap_ops, &xfs_iomap_write_ops,
> + ac);
> }
>
> int
> @@ -2214,5 +2217,6 @@ xfs_truncate_page(
> return dax_truncate_page(inode, pos, did_zero,
> &xfs_dax_write_iomap_ops);
> return iomap_truncate_page(inode, pos, did_zero,
> - &xfs_buffered_write_iomap_ops, ac);
> + &xfs_buffered_write_iomap_ops, &xfs_iomap_write_ops,
> + ac);
> }
> diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
> index 674f8ac1b9bd..ebcce7d49446 100644
> --- a/fs/xfs/xfs_iomap.h
> +++ b/fs/xfs/xfs_iomap.h
> @@ -57,5 +57,6 @@ extern const struct iomap_ops xfs_seek_iomap_ops;
> extern const struct iomap_ops xfs_xattr_iomap_ops;
> extern const struct iomap_ops xfs_dax_write_iomap_ops;
> extern const struct iomap_ops xfs_atomic_write_cow_iomap_ops;
> +extern const struct iomap_write_ops xfs_iomap_write_ops;
>
> #endif /* __XFS_IOMAP_H__*/
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index ad3bcb76d805..3f177b4ec131 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1881,7 +1881,8 @@ xfs_reflink_unshare(
> &xfs_dax_write_iomap_ops);
> else
> error = iomap_file_unshare(inode, offset, len,
> - &xfs_buffered_write_iomap_ops);
> + &xfs_buffered_write_iomap_ops,
> + &xfs_iomap_write_ops);
> if (error)
> goto out;
>
> diff --git a/fs/zonefs/file.c b/fs/zonefs/file.c
> index a0ce6c97b9e5..88cb7df2709f 100644
> --- a/fs/zonefs/file.c
> +++ b/fs/zonefs/file.c
> @@ -572,7 +572,8 @@ static ssize_t zonefs_file_buffered_write(struct kiocb *iocb,
> if (ret <= 0)
> goto inode_unlock;
>
> - ret = iomap_file_buffered_write(iocb, from, &zonefs_write_iomap_ops, NULL);
> + ret = iomap_file_buffered_write(iocb, from, &zonefs_write_iomap_ops,
> + NULL, NULL);
> if (ret == -EIO)
> zonefs_io_error(inode, true);
>
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 568a246f949b..482787013ff7 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -101,8 +101,6 @@ struct vm_fault;
> */
> #define IOMAP_NULL_ADDR -1ULL /* addr is not valid */
>
> -struct iomap_folio_ops;
> -
> struct iomap {
> u64 addr; /* disk offset of mapping, bytes */
> loff_t offset; /* file offset of mapping, bytes */
> @@ -113,7 +111,6 @@ struct iomap {
> struct dax_device *dax_dev; /* dax_dev for dax operations */
> void *inline_data;
> void *private; /* filesystem private */
> - const struct iomap_folio_ops *folio_ops;
> u64 validity_cookie; /* used with .iomap_valid() */
> };
>
> @@ -143,16 +140,11 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap)
> }
>
> /*
> - * When a filesystem sets folio_ops in an iomap mapping it returns, get_folio
> - * and put_folio will be called for each folio written to. This only applies
> - * to buffered writes as unbuffered writes will not typically have folios
> - * associated with them.
> - *
> * When get_folio succeeds, put_folio will always be called to do any
> * cleanup work necessary. put_folio is responsible for unlocking and putting
> * @folio.
> */
> -struct iomap_folio_ops {
> +struct iomap_write_ops {
> struct folio *(*get_folio)(struct iomap_iter *iter, loff_t pos,
> unsigned len);
> void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied,
> @@ -335,7 +327,8 @@ static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> }
>
> ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
> - const struct iomap_ops *ops, void *private);
> + const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops, void *private);
> int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops);
> void iomap_readahead(struct readahead_control *, const struct iomap_ops *ops);
> bool iomap_is_partially_uptodate(struct folio *, size_t from, size_t count);
> @@ -344,11 +337,14 @@ bool iomap_release_folio(struct folio *folio, gfp_t gfp_flags);
> void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len);
> bool iomap_dirty_folio(struct address_space *mapping, struct folio *folio);
> int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len,
> - const struct iomap_ops *ops);
> + const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops);
> int iomap_zero_range(struct inode *inode, loff_t pos, loff_t len,
> - bool *did_zero, const struct iomap_ops *ops, void *private);
> + bool *did_zero, const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops, void *private);
> int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
> - const struct iomap_ops *ops, void *private);
> + const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops, void *private);
> vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops,
> void *private);
> typedef void (*iomap_punch_t)(struct inode *inode, loff_t offset, loff_t length,
> --
> 2.47.2
>
>
