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

[Joseph Qi]

On 7/27/26 2:17 PM, Heming Zhao wrote:
> This patch converts OCFS2's DIO write path from the legacy
> buffer_head infrastructure to the modern iomap framework.
> 
> Key modifications and designs are as follows:
> 
> 1. Dynamic Context Allocation:
>    Refactor 'struct ocfs2_write_ctxt' to use a flexible array 'w_desc[]'
>    instead of a fixed-size array. Dynamically allocate the context based on
>    the write length ('w_clen') in 'ocfs2_alloc_write_ctxt()'. This prevents
>    static limits overflow and optimizes kernel heap memory utilization.
> 
> 2. Robust Mapping and Limits:
>    Introduce 'ocfs2_dio_wr_map_blocks()' to allocate and map direct write
>    blocks. Implement a 1 MiB cap ('OCFS2_DIO_WR_MAX_MAX_BYTES') per mapping
>    call to restrict allocation granularity, preventing JBD2 transaction
>    credit exhaustion during huge asynchronous sequential writes.
> 
> 3. Reliable Completion Work and Fallback:
>    - Implement 'ocfs2_iomap_dio_end_io_write()' to handle metadata completion.
>      It converts UNWRITTEN extents, updates inode size (EOF), and deletes the
>      inode from the orphan directory if it was appended.
>    - Implement 'ocfs2_dio_write_end_io()' to finalize the dio lifecycle and
>      release cluster locks safely.
>    - Intercept '-ENOTBLK' errors from 'iomap_dio_rw()' caused by page cache
>      invalidation failures (due to mmap/buffered collisions). Gracefully clear
>      the error, strip the IOCB_DIRECT flag, and fall back to buffered write
> 
> 4. Moved ocfs2_add_inode_to_orphan():
>    - moved ocfs2_add_inode_to_orphan() from ocfs2_dio_wr_map_blocks() to
>      ocfs2_file_write_iter().
> 
> 5. Uncertain logic in code
>    For the following code block in ocfs2_dio_wr_map_blocks():
>    ```
>      if (extend) {
>              if (ocfs2_sparse_alloc(osb))
>                      ret = ocfs2_zero_tail(inode, di_bh, pos);
>              else
>                      ret = ocfs2_expand_nonsparse_inode(inode, di_bh, pos,
>                                                          map_len, NULL);
>              if (ret < 0) {
>                      mlog_errno(ret);
>                      goto unlock;
>              }
>      }
>    ```
>    I am not completely certain whether it is called only once per
>    ocfs2_file_write_iter(), but I believe calling it multiple times will
>    not introduce any side effects. Furthermore, testing across various
>    scenarios showed no instances of multiple calls.
> 
> Assisted-by: Gemini:gemini-3.5-flash
> Assisted-by: Claude:claude-sonnet-4-5
> Co-developed-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Heming Zhao <heming.zhao@xxxxxxxx>
> ---
>  fs/ocfs2/aops.c           | 394 ++++++++++++++++++++++++++++++++++++--
>  fs/ocfs2/buffer_head_io.c |   7 +-
>  fs/ocfs2/file.c           |  91 +++++++--
>  fs/ocfs2/ocfs2.h          |   2 +
>  4 files changed, 459 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 12f5f2e3530a..9a079436c9c0 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c

...

> +
> +	ocfs2_free_unwritten_list(inode, &wc->w_unwritten_list);
> +	ret = ocfs2_write_end_nolock(inode->i_mapping, pos, map_len, map_len, wc);
> +	BUG_ON(ret != map_len);

Under memory pressure, folio allocation may fail. In this case,
ocfs2_write_end_nolock() can return a short count.
So we must handle this case gracefully.

> +	ret = 0;
> +
> +unlock:
> +	up_write(&oi->ip_alloc_sem);
> +	ocfs2_inode_unlock(inode, 1);
> +	brelse(di_bh);
> +
> +out:
> +	return ret;
> +}
> +
>  static int ocfs2_dio_end_io_write(struct inode *inode,
>  				  struct ocfs2_dio_write_ctxt *dwc,
>  				  loff_t offset,

...

> +static int ocfs2_iomap_dio_end_io_write(struct inode *inode,
> +				  loff_t offset,
> +				  ssize_t bytes)
> +{
> +	struct ocfs2_cached_dealloc_ctxt dealloc;
> +	struct ocfs2_extent_tree et;
> +	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> +	struct ocfs2_inode_info *oi = OCFS2_I(inode);
> +	struct buffer_head *di_bh = NULL;
> +	struct ocfs2_dinode *di;
> +	struct ocfs2_alloc_context *data_ac = NULL;
> +	struct ocfs2_alloc_context *meta_ac = NULL;
> +	handle_t *handle = NULL;
> +	loff_t end = offset + bytes;
> +	int ret = 0, credits = 0;
> +	struct ocfs2_map_block map;
> +	unsigned int blkbits = inode->i_blkbits;
> +	unsigned int max_blocks;
> +	unsigned int ue_cpos = 0, ue_phys = 0, ue_len = 0;
> +	unsigned int curr_lblk, end_lblk;
> +
> +	map.lblk = offset >> blkbits;
> +	max_blocks = (bytes + offset) >> osb->s_clustersize_bits;

Seems unused.

...

> +	curr_lblk = offset >> blkbits;
> +	/*
> +	 * Round the end up so the final partial block (sub-block direct I/O)
> +	 * is included; otherwise the last, partially-written cluster is left
> +	 * unwritten and reads back as zero.
> +	 */
> +	end_lblk = (offset + bytes + (1 << blkbits) - 1) >> blkbits;
> +	while (ret >= 0 && curr_lblk < end_lblk) {
> +		memset(&map, 0, sizeof(map));
> +		map.lblk += curr_lblk;

Since map is memset just now, so here we can use "map.lblk = curr_lblk"
directly.

> +		map.len = end_lblk - curr_lblk;
> +
> +		ret = ocfs2_assure_trans_credits(handle, credits);
> +		if (ret < 0) {
> +			mlog_errno(ret);
> +			break;
> +		}
> +

...

> +
> +static int ocfs2_dio_write_end_io(struct kiocb *iocb, ssize_t size,
> +			int error, unsigned int flags, int level)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	loff_t offset = iocb->ki_pos;
> +	int ret = 0;
> +
> +	if (error)
> +		mlog_ratelimited(ML_ERROR, "Direct IO failed, bytes = %lld errno:%d",
> +				 (long long)size, error);
> +
> +	if (size && ((flags & IOMAP_DIO_UNWRITTEN) ||
> +		    (offset + size > i_size_read(inode)))) {

Is it safe to do i_size_read() in case async dio completion?

...