Re: [PATCH v6 13/31] ext4: do not use data=ordered mode for inodes using buffered iomap path

[Ojaswin Mujoo]

On Thu, Sep 03, 2026 at 08:35:25PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> The data=ordered mode introduces two fundamental conflicts with the
> iomap buffered write path, leading to potential deadlocks.
> 
> 1) Lock ordering conflict
>    In the iomap writeback path, each folio is processed sequentially:
>    the folio lock is acquired first, followed by starting a transaction
>    to create block mappings.
> 
>    In data=ordered mode, writeback triggered by the journal commit
>    process may attempt to acquire a folio lock that is already held by
>    iomap background writeback process. Meanwhile, iomap, under that same
>    folio lock, may start a new transaction to map other blocks on this
>    folio and wait for the currently committing transaction to finish,
>    resulting in a deadlock.
> 
>    Trans N commit                    background writeback(via iomap)
> 
>    journal_submit_data_buffers()
>     ext4_journal_submit_inode_data_buffers()
>      iomap_writepages()
>                                      iomap_writepages()
>                                      folio_lock()
>       folio_lock()  -- wait
>                                       iomap_writeback_folio()
>                                        iomap_writeback_range()
>                                         ext4_journal_start()
>                                          start new transaction
>                                           -- wait for trans N commit,
> 					     DEADLOCK
>                                          ext4_map_blocks()
> 
>    Currently, in the buffer_head writeback path, this is handled by
>    starting the transaction before taking any folio locks for writeback.
> 
> 2) Partial folio submission not supported
>    When block size < folio size, a folio may contain both mapped and
>    unmapped blocks. In data=ordered mode, a deadlock can occur if the
>    journal waits (pure JI_WAIT_DATA) for such a folio to be written back
>    while background writeback has already started on it (with the
>    writeback flag set). The problem is that mapping the remaining
>    delalloc blocks can deadlock because the writeback flag is not
>    cleared until the entire folio is processed and committed.
> 
>    T0: Assume we have a folio contains four blocks, from front to back,
>        they are A, B, C, D. The block B and C are holes, and the last
>        block D is written in delalloc mode (the block is not allocated
>        yet).
>    T1: The background writeback process starts to write back data, set
>        writeback flag on the folio, allocates block D, and adds it to
>        transaction N's order list of jbd2 in pure JI_WAIT_DATA mode.
>    T2: This folio completes the writeback and clears the writeback flag.
>    T3: Before transaction N commit, we buffered write block A to C.
>    T4: Transaction N commit and folio writeback are running
>        concurrently.
> 
>    Trans N commit                   background writeback(via iomap)
> 
>                                     iomap_writeback_folio()
>                                      folio_start_writeback()
>                                       -- set writeback flag
>    jbd2_journal_finish_inode_data_buffers()
>     __filemap_fdatawait_range()
>      -- wait writeback flag to clear
>                                       iomap_writeback_range()
>                                        ext4_journal_start()
>                                         start new transaction
>                                          -- wait for trans N commit,
>                                             DEADLOCK
>                                        ext4_map_block() (B, C)
> 
>    Currently, in the buffer_head writeback path, this is handled by:
> 
>    1. Partial folio submission — already-allocated buffers can be
>       submitted first. The writeback flag is cleared after I/O
>       completes, preventing block allocation while the writeback flag is
>       set.
>    2. Allocation order — the transaction is started first, then blocks
>       are allocated, the writeback flag is set, and finally the
>       allocated buffers submission begins.
> 
> To support data=ordered mode, the iomap core would need two invasive
> changes:
>  - Acquire the transaction handle before locking any folio for
>    writeback.
>  - Support partial folio submission.
> 
> Both changes are complicated and risk performance regressions.
> Therefore, we must avoid using data=ordered mode when converting to the
> iomap path.
> 
> Currently, data=ordered mode is used in three scenarios:
>  - Append write
>  - Post-EOF partial block truncate-up followed by append write
>  - Online defragmentation
> 
> We can address the first two without data=ordered mode:
>  - For append write: always allocate unwritten blocks (i.e. always
>    enable dioread_nolock), preserving the behavior of current
>    extent-type inodes.
>  - For post-EOF truncate-up + append write: postpone updating i_disksize
>    until after the zeroed partial block has been written back.
> 
> Online defragmentation does not yet support iomap; this can be resolved
> separately in the future.
> 
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> Reviewed-by: Jan Kara <jack@xxxxxxx>

Thanks for adding the extra information in the commit.

Feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Regards,
ojaswin

> ---
>  fs/ext4/ext4_jbd2.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 2fbf48b3dfe2..be54e93bde0b 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -379,7 +379,12 @@ static inline int ext4_should_journal_data(struct inode *inode)
>  
>  static inline int ext4_should_order_data(struct inode *inode)
>  {
> -	return ext4_inode_journal_mode(inode) & EXT4_INODE_ORDERED_DATA_MODE;
> +	/*
> +	 * inodes using the iomap buffered I/O path do not use the
> +	 * data=ordered mode.
> +	 */
> +	return !ext4_inode_buffered_iomap(inode) &&
> +		(ext4_inode_journal_mode(inode) & EXT4_INODE_ORDERED_DATA_MODE);
>  }
>  
>  static inline int ext4_should_writeback_data(struct inode *inode)
> -- 
> 2.52.0
>