Re: [PATCH -next v5 27/32] ext4: clear DISKSIZE_GROW_PENDING on truncate or error

On 8/14/2026 6:31 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The EXT4_STATE_DISKSIZE_GROW_PENDING inode state bit is never set anywhere in the codebase. As a result, the entire deferred disksize update mechanism introduced in this patch series is non-functional.
> - [Medium] The DISKSIZE_GROW_PENDING state is not cleared for linked inodes (i_nlink > 0) during eviction, leading to a WARN_ON_ONCE in ext4_destroy_inode() on forced shutdown.
> --
> 
> --- Patch [27]: [PATCH -next v5 27/32] ext4: clear DISKSIZE_GROW_PENDING on truncate or error ---
> commit a9d36fc1a5b406105120b9e8fd8d91b4a9fe12f0
> Author: Zhang Yi <[email protected]>
> 
> ext4: clear DISKSIZE_GROW_PENDING on truncate or error
> 
> The disksize-grow-pending state is set when a zeroed EOF block is queued
> for writeback and cleared by the ioend completion path once writeback
> finishes. This patch clears the disksize-grow-pending state on discard,
> inode eviction, filesystem emergency state, or truncate to avoid blocking
> writebacks and preventing warnings in ext4_destroy_inode().
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 056937e278594..a1dfb70127ca8 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -273,6 +273,8 @@ void ext4_evict_inode(struct inode *inode)
>>   
>>   	if (ext4_should_order_data(inode))
>>   		ext4_begin_ordered_truncate(inode, 0);
>> +	if (ext4_inode_buffered_iomap(inode))
>> +		ext4_iomap_clear_disksize_pending(inode);
> 
> [Severity: Medium]
> If a filesystem is forcibly shut down (e.g., via EXT4_IOC_SHUTDOWN without
> logging an error that sets EXT4_ERROR_FS) and then unmounted, dirty linked
> inodes will be evicted.
> 
> Looking earlier in ext4_evict_inode(), if inode->i_nlink is > 0, the code
> jumps to the no_delete label, completely bypassing this cleanup:
> 
> 	if (inode->i_nlink) {
> 		...
> 		goto no_delete;
> 	}
> 
> Will this cause the newly added WARN_ON_ONCE in ext4_destroy_inode() to
> trigger for linked inodes since they bypass this state clearing?
> 

Yes, this is right. A linked inode evicted after a forced shutdown
bypasses it and would hit the WARN_ON_ONCE in ext4_destroy_inode(). So
I'd tightened the WARN_ON_ONCE guard in ext4_destroy_inode() to skip the
check when the filesystem is in an error or emergency state, since the
pending block may not have been written back in those cases. Besides,
drop the emergency-state cleanup added to ext4_iomap_writepages()
because it becomes unnecessary.

Thanks,
Yi.


>>   	truncate_inode_pages_final(&inode->i_data);
>>   
>>   	/*
> 
> [ ... ]
> 
>> @@ -6741,7 +6759,18 @@ static int ext4_truncate_down(struct inode *inode, loff_t oldsize,
>>   	start_lblk = newsize > 0 ? (newsize - 1) >> inode->i_blkbits : 0;
>>   	ext4_fc_track_range(handle, inode, start_lblk, EXT_MAX_BLOCKS - 1);
>>   
>> -	ext4_set_inode_size(inode, newsize);
>> +	down_write(&EXT4_I(inode)->i_data_sem);
>> +	/*
>> +	 * Truncate the zeroed EOF block invalidates the pending disksize
>> +	 * update, so clear the disksize-grow-pending state.
>> +	 */
>> +	if (ext4_test_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING) &&
>> +	    (newsize <= EXT4_I(inode)->i_disksize))
>> +		ext4_iomap_clear_disksize_pending(inode);
> 
> [Severity: High]
> Is the EXT4_STATE_DISKSIZE_GROW_PENDING bit ever set anywhere in the
> codebase?
> 
> A search of the tree indicates the literal EXT4_STATE_DISKSIZE_GROW_PENDING
> is only ever read or cleared, never set via ext4_set_inode_state() or
> similar.
> 
> There is a TODO comment in ext4_block_zero_eof() indicating that this state
> should be set:
> 
> 	/*
> 	 * TODO: In the iomap path, handle this by tracking the ordered
> 	 * range and updating i_disksize to i_size after the zeroed data
> 	 * has been written back.
> 	 */
> 
> If the bit is never actually set, won't this entire deferred disksize update
> mechanism be non-functional, leaving the filesystem vulnerable to the stale
> data exposure this patchset was intended to fix?
> 
>> +
>> +	i_size_write(inode, newsize);
>> +	__ext4_set_i_disksize(inode, newsize);
>> +	up_write(&EXT4_I(inode)->i_data_sem);
>>   
>>   	ret = ext4_mark_inode_dirty(handle, inode);
>>   	ext4_journal_stop(handle);
>