Re: [PATCH -next v5 26/32] ext4: submit and wait for disksize-grow I/O in fallocate paths

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In `ext4_do_fallocate` with `O_SYNC`, the fast commit can execute before the workqueue updates `i_disksize`, resulting in lost file extension on crash.
--

--- Patch [26]: [PATCH -next v5 26/32] ext4: submit and wait for disksize-grow I/O in fallocate paths ---
commit 918bb376bb323b629f0c4b2e5c84c055d7c4c754
Author: Zhang Yi <[email protected]>

ext4: submit and wait for disksize-grow I/O in fallocate paths

Collapse range and insert range update i_disksize directly under
i_data_sem. If the operation runs while the zeroed EOF block is still
awaiting writeback, i_disksize could advance past the zeroed boundary
before the zeroed data is persisted, exposing stale data on crash.

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index fc5aa2dbefcff..dda6d50e96e30 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4928,10 +4938,20 @@ static long ext4_do_fallocate(struct file *file, loff_t offset,
>  	if (ret)
>  		goto out;
>  
> -	if (((file->f_flags & O_SYNC) || IS_SYNC(inode)) &&
> -	    EXT4_SB(inode->i_sb)->s_journal) {
> -		ret = ext4_fc_commit(EXT4_SB(inode->i_sb)->s_journal,
> -					EXT4_I(inode)->i_sync_tid);
> +	if ((file->f_flags & O_SYNC) || IS_SYNC(inode)) {
> +		/*
> +		 * Sync the pending zeroed EOF block to ensure the
> +		 * i_disksize update is persisted.
> +		 */
> +		if (new_size) {
> +			ret = ext4_iomap_sync_zeroed_eof(inode, 0, LLONG_MAX);
> +			if (ret)
> +				goto out;
> +		}
> +		if (EXT4_SB(inode->i_sb)->s_journal) {
> +			ret = ext4_fc_commit(EXT4_SB(inode->i_sb)->s_journal,
> +						EXT4_I(inode)->i_sync_tid);
> +		}

[Severity: High]
Can this fast commit execute before the workqueue updates i_disksize,
potentially losing the file extension on a crash?

Looking at the bio completion path in fs/ext4/page-io.c:ext4_iomap_end_bio():

	if (io_mode == EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO)
		ext4_iomap_clear_disksize_pending(ioend->io_inode);
	
	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
	if (list_empty(&ei->i_rsv_conversion_list))
		queue_work(EXT4_SB(ioend->io_inode->i_sb)->rsv_conversion_wq,
		           ...);

It appears the EXT4_STATE_DISKSIZE_GROW_PENDING flag is cleared before the
workqueue is actually dispatched and executed.

If ext4_iomap_sync_zeroed_eof() wakes up immediately after the flag is
cleared, could it proceed to ext4_fc_commit() before the workqueue gets a
chance to update i_disksize?

If a crash happens right after the fast commit in this scenario, would it
persist the inode with the old i_disksize, causing a regression by
violating the requested O_SYNC guarantees?

>  	}
>  out:
>  	trace_ext4_fallocate_exit(inode, offset,

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