Re: [PATCH -next v5 25/32] ext4: defer i_disksize update while DISKSIZE_GROW_PENDING is set

On 8/14/2026 6:31 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Missing State Initialization / Dead Code: EXT4_STATE_DISKSIZE_GROW_PENDING is never set.
> - [High] State Leak / Data Loss: Discarding the zeroed EOF folio without writeback permanently leaks the pending bit.

These are false positive. EXT4_STATE_DISKSIZE_GROW_PENDING is set
in patch 28. This patch is just preparing for that functionality.

> - [High] Durability Loss: O_SYNC extensions via fallocate/zero_range silently persist a stale i_disksize.

This is a false positive. O_SYNC in fallocate operations is handled
in patch 26.

Thanks,
Yi.

> --
> 
> --- Patch [25]: [PATCH -next v5 25/32] ext4: defer i_disksize update while DISKSIZE_GROW_PENDING is set ---
> commit 5fa612fe6ec599be935a3aa55b36845e34d3180e
> Author: Zhang Yi <[email protected]>
> 
> ext4: defer i_disksize update while DISKSIZE_GROW_PENDING is set
> 
> Operations like append allocate, zero range, and truncate update
> i_disksize directly. If the new i_disksize exceeds the original value
> while the zeroed EOF block is still awaiting writeback, metadata may be
> persisted before the zeroed data, exposing stale data on crash.
> 
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -3605,30 +3605,67 @@ do {								\
> 
> [ ... ]
> 
>> +static inline void __ext4_set_i_disksize(struct inode *inode, loff_t newsize)
>> +{
>> +	WARN_ON_ONCE(!rwsem_is_locked(&EXT4_I(inode)->i_data_sem));
>> +
>> +	if (newsize < EXT4_I(inode)->i_disksize ||
>> +	    !ext4_test_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING))
> 
> [Severity: High]
> Could O_SYNC extensions via ext4_do_fallocate() or ext4_zero_range() silently
> persist a stale i_disksize here?
> 
> If the size is extended with an unaligned old EOF, the i_disksize update is
> deferred. Since O_SYNC immediately commits the journal without waiting for
> the pending zeroing writeback to complete, it seems it will commit the old,
> deferred i_disksize to disk.
> 
> Would this cause the file size to revert on crash despite a successful fsync,
> discarding the size extension?
> 
>> +		WRITE_ONCE(EXT4_I(inode)->i_disksize, newsize);
>> +}
> 
> [ ... ]
> 
>>   static inline int ext4_update_inode_size(struct inode *inode, loff_t newsize)
>>   {
>>   	int changed = 0;
>>   
>> +	if (newsize <= inode->i_size && newsize <= EXT4_I(inode)->i_disksize)
>> +		return 0;
>> +
>> +	down_write(&EXT4_I(inode)->i_data_sem);
>>   	if (newsize > inode->i_size) {
>>   		i_size_write(inode, newsize);
>>   		changed = 1;
>>   	}
>> -	if (newsize > EXT4_I(inode)->i_disksize) {
>> -		ext4_update_i_disksize(inode, newsize);
>> +	if (newsize > EXT4_I(inode)->i_disksize &&
>> +	    !ext4_test_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING)) {
> 
> [Severity: High]
> Are we setting EXT4_STATE_DISKSIZE_GROW_PENDING anywhere in the codebase?
> 
> Looking at this patch series, it appears the bit is checked here but never
> actually initialized or set. Does this mean the deferral condition is always
> bypassed and i_disksize is always updated immediately?
> 
> [Severity: High]
> If the EXT4_STATE_DISKSIZE_GROW_PENDING bit is properly set, could it leak
> if the dirty folio is discarded before writeback?
> 
> For example, if a file is truncated down before writeback completes, it
> seems the bit might never be cleared since the writeback completion function
> wouldn't be called for that folio.
> 
> Would this leak cause all subsequent appends to skip updating i_disksize,
> leading to silent data loss on crash for future writes to the file?
> 
>> +		WRITE_ONCE(EXT4_I(inode)->i_disksize, newsize);
>>   		changed |= 2;
>>   	}
>> +	up_write(&EXT4_I(inode)->i_data_sem);
>>   	return changed;
>>   }
>