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

On 8/14/2026 6:28 PM, [email protected] wrote:
> 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?

Yeah, this is right. In ext4_iomap_sync_zeroed_eof(), we should
replace the filemap_fdatawrite_range() +
ext4_iomap_wait_disksize_pending() calls with just
filemap_write_and_wait_range().

Thanks,
Yi.

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