Re: [PATCH -next v5 24/32] ext4: advance i_disksize to i_size upon disksize-grow I/O completion

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The EXT4_STATE_DISKSIZE_GROW_PENDING state bit is never set anywhere in the codebase, rendering the entire disksize-grow logic and safety mechanism inert.
--

--- Patch [24]: [PATCH -next v5 24/32] ext4: advance i_disksize to i_size upon disksize-grow I/O completion ---
commit d834c25602b493451d52cc75b3ae8fd40461bce9
Author: Zhang Yi <[email protected]>

ext4: advance i_disksize to i_size upon disksize-grow I/O completion

This commit updates the iomap writeback path to advance i_disksize to i_size
upon completion of a disksize-grow I/O. It introduces an is_disksize_grow flag
to ext4_iomap_wb_update_disksize() to trigger this update safely.

> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 955ff88045db6..4f1176b9332f2 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
[ ... ]
> @@ -589,9 +589,36 @@ static int ext4_iomap_wb_update_disksize(handle_t *handle, struct inode *inode,
>  	 * i_data_sem.
>  	 */
>  	down_write(&ei->i_data_sem);
> -	new_disksize = min(new_disksize, i_size_read(inode));
> +	i_size = i_size_read(inode);
> +
> +	/*
> +	 * EXT4_STATE_DISKSIZE_GROW_PENDING is cleared when the pending
> +	 * I/O completes. However, another thread may have re-set the bit
> +	 * between that point and here, meaning i_disksize has already
> +	 * been advanced and a new EOF zeroing has been initiated. In that
> +	 * case, do not advance i_disksize to i_size; leave it to the
> +	 * next pending grow ioend.
> +	 */
> +	if (is_disksize_grow &&
> +	    ext4_test_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING))
> +		is_disksize_grow = false;

[Severity: High]
Is EXT4_STATE_DISKSIZE_GROW_PENDING actually set anywhere in the codebase?

It appears this state bit is never set, such as during EOF zeroing in
ext4_block_zero_eof() where there is a TODO comment about tracking the
ordered range.

If the bit is never set, ext4_test_inode_state() will always return false
and this safety check will be bypassed during concurrent fallocate and
truncate operations.

[ ... ]
> @@ -607,6 +634,8 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
>  	loff_t pos = ioend->io_offset;
>  	size_t size = ioend->io_size;
>  	loff_t end = pos + size;
> +	unsigned long io_mode = (unsigned long)ioend->io_private;
> +	bool is_disksize_grow = (io_mode == EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO);

[Severity: High]
Is EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO ever assigned to ioend->io_private?

Because the EXT4_STATE_DISKSIZE_GROW_PENDING bit is never set, it seems
ext4_iomap_get_disksize_pending_range() will always return 0, meaning this
io_mode flag is never assigned.

Does this render the entire disksize-grow logic inert?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=24