Re: [PATCH v7 27/31] ext4: set DISKSIZE_GROW_PENDING after zeroing unaligned EOF block

[Zhang Yi <yizhang089@gmail.com>]

On 10/10/2026 9:49 PM, Ojaswin Mujoo wrote:
> On Sat, Oct 10, 2026 at 05:50:24PM +0800, Zhang Yi wrote:
>> On 10/10/2026 5:00 PM, Ojaswin Mujoo wrote:
>>> On Fri, Oct 09, 2026 at 06:30:28PM +0800, Zhang Yi wrote:
>>>> From: Zhang Yi <yi.zhang@huawei.com>
>>>>
>>>> In the iomap buffered I/O path, data=ordered mode is not used, so the
>>>> zeroed EOF block has no implicit ordering with later i_disksize updates.
>>>> Without the pending state being set, i_disksize can be advanced past the
>>>> zeroed block before writeback completes, exposing stale data after a
>>>> crash.
>>>>
>>>> Previous patches added the consumer side of the
>>>> disksize-grow-pending mechanism: the state bit, clear and wait helpers,
>>>> and ioend tagging. Now add ext4_iomap_mark_disksize_pending() and call
>>>> it from ext4_block_zero_eof() after zeroing the tail of the block that
>>>> straddles i_disksize.
>>>>
>>>> The helper locks the folio, waits for any in-flight writeback on it to
>>>> complete, then sets EXT4_STATE_DISKSIZE_GROW_PENDING only if the folio
>>>> is still dirty. Waiting for writeback prevents folio_test_dirty() from
>>>> returning false mid-writeback, which would cause us to skip the pending
>>>> state while zeroed data is still in flight. The dirty check then avoids
>>>> setting the bit when the data has already been written back.
>>>>
>>>> Suggested-by: Jan Kara <jack@suse.cz>
>>>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>>>> ---
>>>>    fs/ext4/inode.c | 106 ++++++++++++++++++++++++++++++++++++++++++------
>>>>    1 file changed, 94 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>>>> index 9b6916609000c..3bdb5f3b119ab 100644
>>>> --- a/fs/ext4/inode.c
>>>> +++ b/fs/ext4/inode.c
>>>> @@ -4847,6 +4847,89 @@ static int ext4_block_zero_range(struct inode *inode,
>>>>    					zero_written);
>>>>    }
>>>> +/*
>>>> + * Inodes using the iomap buffered I/O path do not use data=ordered mode.
>>>> + * Therefore, we mark the inode as disksize-grow-pending after zeroing the
>>>> + * EOF block. The zeroed block will be submitted before any subsequent
>>>> + * data.
>>>> + *
>>>> + * In the I/O completion path, ext4_iomap_wb_disksize_pending_wait() will
>>>> + * wait for I/O completion before advancing i_disksize if the write
>>>> + * extends beyond the zeroed boundary.
>>>> + *
>>>> + * When zeroed I/O is in progress, operations that extend i_disksize are
>>>> + * handled as follows:
>>>> + *
>>>> + *  - Truncate up, append fallocate and zero_range:
>>>> + *    Defer the update. The file size will be updated to i_size by the
>>>> + *    end_io handler once the ongoing pending I/O completes.
>>>> + *
>>>> + *  - Insert range and collapse range operations:
>>>> + *    Wait synchronously for the relevant I/O to complete before updating
>>>> + *    i_disksize.
>>>> + */
>>>> +static int ext4_iomap_mark_disksize_pending(struct inode *inode, loff_t from)
>>>> +{
>>>> +	struct folio *folio;
>>>> +
>>>> +	folio = filemap_lock_folio(inode->i_mapping, from >> PAGE_SHIFT);
>>>> +	if (IS_ERR(folio))
>>>> +		/* Already in writeback and cleared? */
>>>> +		return PTR_ERR(folio) == -ENOENT ? 0 : PTR_ERR(folio);
>>>> +
>>>> +	/*
>>>> +	 * Ensure that in-flight writeback, possibly started after
>>>> +	 * iomap_zero_range() unlocked the folio, has completed. Without
>>>> +	 * this wait folio_test_dirty() below may miss the zeroed data
>>>> +	 * (writeback clears PG_dirty), causing us to skip the
>>>> +	 * disksize-grow-pending tracking and potentially expose stale
>>>> +	 * on-disk data.
>>>> +	 */
>>>> +	folio_wait_writeback(folio);
>>>> +	WARN_ON_ONCE(folio_test_writeback(folio));
>>>> +
>>>> +	/*
>>>> +	 * If the zeroed range does not overlap the existing on-disk tail
>>>> +	 * block, the zeroed data lies beyond the currently on-disk data
>>>> +	 * and will be written back before i_disksize is advanced past it,
>>>> +	 * so no stale data can be exposed.
>>>> +	 *
>>>> +	 * Checking i_disksize here (after folio_wait_writeback()) is
>>>> +	 * necessary and safe. If a delalloc writeback of this folio was
>>>> +	 * in-flight, it could be raced by a concurrent mmap write which
>>>> +	 * corrupts the tail block beyond i_size but the i_disksize is not
>>>> +	 * advanced. folio_wait_writeback() has waited for its completion
>>>> +	 * and the ioend has advanced i_disksize accordingly. If writeback
>>>> +	 * had not started, we don't need to mark any pending state because
>>>> +	 * any future writeback will carry the pagecache content that now
>>>> +	 * includes the zeroed data, so no stale data can appear on disk
>>>> +	 * even without the pending tracking.
>>>> +	 */
>>>
>>> Hey Zhang,
>>>
>>> So in continuation for our discussion at [1], thanks for the info and
>>> yes I think the race mentioned there cannot happen. But I was still
>>> trying to look at this path for my own understanding and there's another
>>> sequence of events I'd like to discuss with you. Mostly the same from
>>> last but what if the writeback moves after
>>> ext4_iomap_makr_zero_pending():
>>>
>>>       Initial state: i_size = i_disksize = 2k
>>>
>>>       1. pwrite(4k,6k)
>>>         - ext4_block_zero_eof(2k,4k)
>>>             set inode state DISK_SIZE_GROW_PENDING
>>>         - i_size=6k, i_disksize=2k
>>>       2. writeback(2k,4k)
>>>         - submits a GROW_IO ioend for 0,4k
>>>         - i_size=6k, i_disksize=2k (unchanged)
>>>       3. pwrite(8k,10k) - part 1
>>>         - ext4_block_zero_eof(6k,8k)
>>>         - zeroes 6k,8k
>>>         - ext4_iomap_mark_zero_pending(from=6k)
>>>           - folio is unlocked and not under writeback yet
>>>           - keeps DISK_SIZE_GROW_PENDING set
>>>         - iomap_write_iter not called yet...
>>>       4. mmap write at (6k,8k)
>>>         - I dont see anything stopping this?
>>>         - i_size is still 6k so this is eof write.
>>>       5. writeback(4k,6k)
>>>         - submits IO for 4k,8k
>>>         - i_size=6k, i_disksize=2k (unchanged)
>>>       6. ioend for 2. (2k,4k)
>>>         - clears DISKSIZE_GROW_PENDING
>>>         - GROW_IO so i_disksize updated
>>>         - i_size=6k, i_disksize=6k
>>>       7. ioend for 5. (4k,8k)
>>>         - DISKSIZE_GROW_PENDING is cleared
>>>         - i_disksize=6k,i_size=6k - no updates
>>>         - 4k,8k converted written - data beyond EOF from 4.
>>>       8. 3. continues
>>>         - enter iomap_write_iter
>>>         - pagecache_i_size_extended()
>>>           - page_mkwrite(6k folio) and mark dirty
>>>           - zero 6k,8k
>>>         - i_size=10k, i_disksize=6k
>>>       9. writeback(8k,10k)
>>>         - DISKSIZE_GROW_PENDING cleared so just submit (8k,10k)
>>>       10. ioend for 8.
>>>         - DISKSIZE_GROW_PENDING cleared so no waiting
>>>         - i_size=10k, i_disksize=10k - updated
>>>         - if we crash now, 6k,8k non-zero data exposed
>>>
>>> I looked at the code surrounding ext4_iomap_mkwrite() and
>>> ext4_buffered_write_iter() and I don't think there's any sort of
>>> serialization against this sequence right? Or am I missing something
>>> again :)
>>>
>>
>> Hi Ojaswin,
>>
>> If I understand correctly, your scenario is essentially a concurrent
>> mmap write and append write, that is, a post-EOF mmap write is performed
>> after the append write does the EOF zeroing but before i_size is
>> advanced, right?
>>
>> The scenario can be simplified as follows:
>>
>>    Initial state: i_size = i_disksize = 2k
>>
>>    1. pwrite (4k,6k)
>>      - ext4_block_zero_eof(2k,4k), zero 2k,4k and dirty folio
>>      - iomap_write_iter not called yet... so i_size is still 2k
>>    2. mmap write at (2k,4k)
>>      - i_size is still 2k (so this is still an EOF write?)
>>    3. writeback (0,4k) since it's dirty
>>      - submits I/O for (0,4k), data is written to disk
>>    4. 1 continue and then writeback
>>      - i_size=6k
>>      - submits I/O for (4,8k)
>>      - i_disksize=6k, 2k,4k non-zero data exposed
> 
> Yep, looks right.
>>
>> Actually, I think we don't need to consider this kind of concurrent
>> scenario, since it is undefined behavior in itself. Even if we
>> don't write this part of the mmap data back to disk, we can still
> 
> It will be zeroed again when i_size grows in iomap_write_iter()
> -> pagecache_isize_extended() but yeah a mapped reader might be able to
> read it before that happens.

In fact, for our current scenario, pagecache_isize_extended() is a
no-op, since blocksize equals PAGE_SIZE, so it will no longer zero out
the post-EOF data written by mmap. What pagecache_isize_extended()
zeroes is the data in the folio that doesn't belong to the block
containing the current EOF when blocksize is smaller than folio size,
whereas ext4_block_zero_eof() zeroes the post-EOF part inside the tail
block. Their responsibilities are not the same.

So in the scenario you describe, the data written by mmap will remain in
memory for a long time, and this is not a small window. Since we don't
care about this concurrent scenario, puting ext4_block_zero_eof() before
the write is safe.

Cheers,
Yi.

> 
>> read it from memory, right? What ext4_block_zero_eof() needs to do
>> is zero out the post-EOF stale data written by the mmap before the
>> append write.
> 
> Okay got it so we care more about not exposing data that was
> previously on disk rather than a race with mmap. We do have some
> xfstests that seem to care about the later as well (like generic/363)
> but I get that the race is a very small window here and needs us to
> crash at a specific time. Plus its a parallel mmap written data rather
> than stale data so hopefully it's harmless even if a bit of it is
> exposed after crash.
> 
> Thanks,
> ojaswin
> 
>>
>> Thanks,
>> Yi
>>
>>
>>> Regards,
>>> ojaswin
>>>
>>> [1] https://lore.kernel.org/linux-ext4/179153611724.860602.5266334742364847927.b4-ty@b4/T/#m72e1710d172cf7699b6f05e29a270e45951800b0
>>>> +	if (from >= round_up(READ_ONCE(EXT4_I(inode)->i_disksize),
>>>> +			     i_blocksize(inode)))
>>>> +		goto out;
>>>> +
>>>> +	/*
>>>> +	 * Mark the inode as disksize-grow-pending. The zeroed block will
>>>> +	 * be written out by the generic writepages cycle or any other
>>>> +	 * syncing operation.
>>>> +	 *
>>>> +	 * Multiple overlapping unaligned EOF writes should not happen,
>>>> +	 * because we only mark the pending state after zeroing the on-disk
>>>> +	 * EOF block, and i_disksize can only be updated after the previous
>>>> +	 * zeroed pending block has been written back or the dirty folio
>>>> +	 * has been discarded.
>>>> +	 */
>>>> +	if (likely(folio_test_dirty(folio) &&
>>>> +		   !ext4_test_inode_state(inode,
>>>> +					  EXT4_STATE_DISKSIZE_GROW_PENDING)))
>>>> +		ext4_set_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING);
>>>> +out:
>>>> +	folio_unlock(folio);
>>>> +	folio_put(folio);
>>>> +	return 0;
>>>> +}
>>>> +
>>>>    /*
>>>>     * Submit and wait for the pending zeroed EOF block range to complete
>>>>     * if the given range [@offset, @end) fully covers it.  Must be called
>>>> @@ -4923,21 +5006,20 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
>>>>    	 * truncating up or performing an append write, because there might be
>>>>    	 * exposing stale on-disk data which may caused by concurrent post-EOF
>>>>    	 * mmap write during folio writeback.
>>>> -	 *
>>>> -	 * 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 (ext4_should_order_data(inode) &&
>>>> -	    did_zero && zero_written && !IS_DAX(inode)) {
>>>> -		handle_t *handle;
>>>> +	if (did_zero && zero_written && !IS_DAX(inode)) {
>>>> +		if (ext4_should_order_data(inode)) {
>>>> +			handle_t *handle;
>>>> -		handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
>>>> -		if (IS_ERR(handle))
>>>> -			return PTR_ERR(handle);
>>>> +			handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
>>>> +			if (IS_ERR(handle))
>>>> +				return PTR_ERR(handle);
>>>> -		err = ext4_jbd2_inode_add_write(handle, inode, from, length);
>>>> -		ext4_journal_stop(handle);
>>>> +			err = ext4_jbd2_inode_add_write(handle, inode, from,
>>>> +							length);
>>>> +			ext4_journal_stop(handle);
>>>> +		} else if (ext4_inode_buffered_iomap(inode))
>>>> +			err = ext4_iomap_mark_disksize_pending(inode, from);
>>>>    		if (err)
>>>>    			return err;
>>>>    	}
>>>> -- 
>>>> 2.52.0
>>>>
>>