Re: [RFC PATCH v5 2/4] ocfs2: switch DIO read path from buffer_head to iomap

[Christoph Hellwig <hch@lst.de>]

On Thu, Nov 05, 2026 at 03:19:26PM +0800, Heming Zhao wrote:
> Migrates the DIO read path from the legacy buffer_head infrastructure
> to the modern and more efficient iomap framework.
> 
> Under the current code, ocfs2 holds a cluster lock during DIO via
> ocfs2_iocb_set_rw_locked(), and releases the cluster lock in
> ocfs2_dio_end_io() via ocfs2_iocb_clear_rw_locked(). The lock level is
> stashed in iocb->private.
> 
> However, iomap DIO owns that field (it stores a bio there for polled I/O
> and unconditionally clears it before calling ->end_io). Any side-channel
> state packed into iocb->private is corrupted, which trips the end_io
> lock-state check.
> 
> Instead, encode the unlock level by passing distinct iomap_dio_ops methods
> (ocfs2_iomap_dio_ops_pr for PRMODE or ocfs2_iomap_dio_ops_ex for EXMODE).
> Additionally, the *_iter() functions use the split __iomap_dio_rw() +
> iomap_dio_complete() API so they can determine unambiguously, from the
> return value, whether the completion handler already dropped the lock
> (real I/O, sync or -EIOCBQUEUED) or whether the caller must drop it
> (no I/O issued, or buffered fallback).
> 
> The new function ocfs2_iomap_begin() performs a similar role to
> ocfs2_get_block(). While ocfs2_get_block() retrieves only one block per
> call, ocfs2_iomap_begin() can handle multiple blocks at once. Limit the
> map offset and length in ocfs2_iomap_begin() to OCFS2_MAX_LOGICAL_BLOCK
> to avoid overflow issues.
> 
> Co-developed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/Kconfig    |   1 +
>  fs/ocfs2/aops.c     | 111 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/ocfs2/aops.h     |   4 ++
>  fs/ocfs2/file.c     |  62 +++++++++++++++++++++----
>  fs/ocfs2/ocfs2_fs.h |   3 ++
>  5 files changed, 172 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/Kconfig b/fs/ocfs2/Kconfig
> index 2514d36cbe01..bf1678a5eb01 100644
> --- a/fs/ocfs2/Kconfig
> +++ b/fs/ocfs2/Kconfig
> @@ -7,6 +7,7 @@ config OCFS2_FS
>  	select CRC32
>  	select QUOTA
>  	select QUOTA_TREE
> +	select FS_IOMAP
>  	select FS_POSIX_ACL
>  	select LEGACY_DIRECT_IO
>  	help
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 0f7502de9b8d..8e4d31cb85db 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/fs.h>
> +#include <linux/iomap.h>
>  #include <linux/slab.h>
>  #include <linux/highmem.h>
>  #include <linux/pagemap.h>
> @@ -2534,6 +2535,116 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  				    ocfs2_dio_end_io, 0);
>  }
>  
> +static int ocfs2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> +		unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
> +{
> +	int ret;
> +	struct ocfs2_map_block map;
> +	struct ocfs2_inode_info *oi = OCFS2_I(inode);
> +	u8 blkbits = inode->i_blkbits;
> +
> +	if ((offset >> blkbits) > OCFS2_MAX_LOGICAL_BLOCK)
> +		return -EINVAL;
> +
> +	/*
> +	 * Calculate the first and last logical blocks respectively.
> +	 */
> +	map.lblk = offset >> blkbits;
> +	map.len = min_t(loff_t, (offset + length - 1) >> blkbits,
> +			OCFS2_MAX_LOGICAL_BLOCK) - map.lblk + 1;
> +	map.flags = 0;
> +
> +	if (flags & IOMAP_WRITE) {
> +		return -EOPNOTSUPP;
> +	} else {

No need for an else after a return..

> +	/*
> +	 * Before returning to iomap, let's ensure the allocated mapping
> +	 * covers the entire requested length for atomic writes.
> +	 */
> +	if (flags & IOMAP_ATOMIC) {

IOMAP_ATOMIC is never set for reads.