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

[Christoph Hellwig]

On Mon, Jul 27, 2026 at 02:17:58PM +0800, Heming Zhao wrote:
> This patch migrates the DIO read path from the legacy buffer_head

Please never start commit messages with "This patch", that context
is implied already.

> The rw cluster-lock level is not stashed in iocb->private: iomap DIO owns

Explaining what you don't do is always a bit odd, I'd start with what
you do, and only really mention what not later if really needed.

> that field (it stores a bio there for polled I/O and unconditionally
> clears it before calling ->end_io), so any side-channel packed into
> iocb->private is corrupted and would trip the end_io lock-state check.
> Instead the unlock level is encoded by which iomap_dio_ops is passed -
> ocfs2_iomap_dio_ops_pr (PRMODE) or ocfs2_iomap_dio_ops_ex (EXMODE) - and
> 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).

.. and a lot of this just seems to be about the locking, but very little
about the iomap changes.

> +	if (flags & IOMAP_WRITE) {
> +		/* todo */

should this be a WARN_ON for now?

> +	if (direct_io && ocfs2_should_use_dio(iocb, to, inode)) {
> +		struct iomap_dio *dio;
> +
> +		dio = __iomap_dio_rw(iocb, to, &ocfs2_iomap_ops,
> +				&ocfs2_iomap_dio_ops_r_pr, 0, NULL, 0);
> +		if (dio == NULL) {
> +			/* No I/O issued; rw_lock still held. */
> +			ret = 0;
> +		} else if (IS_ERR(dio)) {
> +			ret = PTR_ERR(dio);
> +			if (ret == -EIOCBQUEUED)
> +				rw_level = -1;
> +		} else {
> +			ret = iomap_dio_complete(dio);
> +			if (ret != 0)
> +				rw_level = -1;
> +		}

What information is missing if you use plain iomap_dio_rw here?

> +		/*
> +		 * A 0 result means the mapping bounced us back to buffered I/O
> +		 * (e.g. inline data); the rw_lock is still held. Clear
> +		 * IOCB_DIRECT so generic_file_read_iter() takes the buffered
> +		 * path rather than re-entering direct I/O.
> +		 */
> +		if (ret == 0) {
> +			iocb->ki_flags &= ~IOCB_DIRECT;
> +			ret = generic_file_read_iter(iocb, to);
> +		}
> +	} else {
> +		iocb->ki_flags &= ~IOCB_DIRECT;
> +		ret = generic_file_read_iter(iocb, to);
> +	}

Can you share this code somehow?