Re: [PATCH v6 15/31] ext4: implement writeback path using iomap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Journal Credits Exhaustion / Panic in `ext4_iomap_finish_ioend()` due to arbitrarily large merged ioends.
- [High] Infinite loop in `ext4_iomap_map_writeback_range` due to open-coded ENOSPC retry logic without limits.
--

commit e7cc5164579a940550c93afb2dfec1f186e079ed
Author: Zhang Yi <yi.zhang@huawei.com>
ext4: implement writeback path using iomap

This commit adds the iomap writeback path for ext4 buffered I/O, introducing
new writeback operations and end I/O handlers. It maps and allocates blocks
as unwritten extents during writeback to prevent stale data exposure.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1f18854b9cdd3..f7d7610197a1c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c

[ ... ]

> +static int ext4_iomap_map_writeback_range(struct iomap_writepage_ctx *wpc,
> +					  loff_t offset, unsigned int dirty_len)
> +{

[ ... ]

> +retry:
> +	map.m_lblk = index;
> +	map.m_len = min_t(unsigned int, MAX_WRITEPAGES_EXTENT_LEN, blk_len);
> +	ret = ext4_map_blocks(NULL, inode, &map,
> +			      EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT |
> +			      EXT4_GET_BLOCKS_METADATA_NOFAIL |
> +			      EXT4_GET_BLOCKS_IO_SUBMIT |
> +			      EXT4_EX_NOCACHE);
> +	if (ret < 0) {
> +		if (ext4_emergency_state(sb))
> +			return ret;
> +
> +		/*
> +		 * Retry transient ENOSPC errors, if
> +		 * ext4_count_free_blocks() is non-zero, a commit
> +		 * should free up blocks.
> +		 */
> +		if (ret == -ENOSPC && journal && ext4_count_free_clusters(sb)) {
> +			jbd2_journal_force_commit_nested(journal);
> +			goto retry;
> +		}

[Severity: High]
Could this open-coded retry loop result in an infinite hang under certain
conditions?

If a non-root user fills the disk up to the threshold of root-reserved
blocks, ext4_map_blocks() will return -ENOSPC because the block allocator
denies access to reserved blocks for non-root users.

However, ext4_count_free_clusters() returns the total free space including
those root-reserved blocks. Because the count evaluates as non-zero, the code
will force a journal commit and goto retry indefinitely, without any loop
limits.

Should this retry logic use a limit like ext4_should_retry_alloc(), or
specifically account for root-reserved blocks to prevent a soft lockup in
the writeback worker thread?

> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 0236b6b9785a2..9b0e12b5463cf 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c

[ ... ]

> +static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
> +{

[ ... ]

> +	/*
> +	 * We may need to convert one extent, update the i_disksize and
> +	 * dirty the inode.
> +	 */
> +	credits = ext4_chunk_trans_blocks(inode,
> +			EXT4_MAX_BLOCKS(size, pos, inode->i_blkbits));
> +	handle = ext4_journal_start(inode, EXT4_HT_EXT_CONVERT, credits);
> +	if (IS_ERR(handle)) {
> +		ret = PTR_ERR(handle);
> +		goto out_err;
> +	}

[ ... ]

> +void ext4_iomap_end_io(struct work_struct *work)
> +{
> +	struct ext4_inode_info *ei = container_of(work, struct ext4_inode_info,
> +						  i_rsv_conversion_work);
> +	struct iomap_ioend *ioend;
> +	struct list_head ioend_list;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> +	list_replace_init(&ei->i_rsv_conversion_list, &ioend_list);
> +	spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
> +
> +	iomap_sort_ioends(&ioend_list);
> +	while (!list_empty(&ioend_list)) {
> +		ioend = list_entry(ioend_list.next, struct iomap_ioend, io_list);
> +		list_del_init(&ioend->io_list);
> +		iomap_ioend_try_merge(ioend, &ioend_list);
> +		ext4_iomap_finish_ioend(ioend);
> +	}
> +}

[Severity: Critical]
Is there a risk of exhausting journal credits and panicking JBD2 when ioends
are merged?

In ext4_iomap_end_io(), iomap_ioend_try_merge() merges contiguous completions
into an arbitrarily large ioend. When this merged ioend is later passed to
ext4_iomap_finish_ioend(), it requests credits using ext4_chunk_trans_blocks().

However, ext4_chunk_trans_blocks() internally hardcodes the extent count to 1,
allocating credits sufficient for modifying only a single extent on disk.

If a user writes a very large contiguous file (e.g., larger than 128MB), the
merged ioend will exceed the maximum size of a single unwritten extent in ext4
(which is 32,768 blocks). This will cause ext4_convert_unwritten_extents() to
loop over multiple extents using the same transaction handle.

Could this exhaust the single-extent credits provided by the transaction
handle, resulting in a JBD2 panic during writeback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903123543.2302999-1-yi.zhang@huaweicloud.com?part=15