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

[Ojaswin Mujoo]

On Fri, Oct 09, 2026 at 06:30:28PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> 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@xxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
>  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 :) 

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
>