mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Zhang Yi <yizhang089@gmail.com>
To: Ojaswin Mujoo <ojaswin@linux.ibm.com>,
	Zhang Yi <yi.zhang@huaweicloud.com>
Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, tytso@mit.edu,
	adilger.kernel@dilger.ca, libaokun@linux.alibaba.com,
	jack@suse.cz, ritesh.list@gmail.com, djwong@kernel.org,
	hch@infradead.org, yi.zhang@huawei.com, chengzhihao1@huawei.com,
	yangerkun@huawei.com, wangkefeng.wang@huawei.com,
	yukuai@fnnas.com
Subject: Re: [PATCH v7 27/31] ext4: set DISKSIZE_GROW_PENDING after zeroing unaligned EOF block
Date: Sat, 10 Oct 2026 17:50:24 +0800	[thread overview]
Message-ID: <2fb2df2f-090c-4b24-9636-d28454a51d1d@gmail.com> (raw)
In-Reply-To: <asn-qfh9uWnabkJb@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com>

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

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
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.

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
>>


  reply	other threads:[~2026-10-10  9:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09 10:30 [PATCH v7 00/31] ext4: use iomap for regular file's buffered I/O path Zhang Yi
2026-10-09 10:30 ` [PATCH v7 01/31] ext4: simplify size updating in ext4_setattr() Zhang Yi
2026-10-09 10:30 ` [PATCH v7 02/31] ext4: factor out ext4_truncate_[up|down]() Zhang Yi
2026-10-09 10:30 ` [PATCH v7 03/31] ext4: set EXT4_MAP_NEW flag for delayed allocated blocks Zhang Yi
2026-10-09 10:30 ` [PATCH v7 04/31] ext4: recheck extent status tree before block allocation Zhang Yi
2026-10-09 10:30 ` [PATCH v7 05/31] ext4: fix orig_mlen initialization in ext4_map_blocks() Zhang Yi
2026-10-09 10:30 ` [PATCH v7 06/31] ext4: allow ext4_map_blocks() to start its own transaction handle Zhang Yi
2026-10-09 10:30 ` [PATCH v7 07/31] ext4: avoid unnecessary transaction in ext4_map_blocks() for unwritten extents Zhang Yi
2026-10-09 10:30 ` [PATCH v7 08/31] ext4: skip block allocation for holes in the data submission path Zhang Yi
2026-10-09 10:30 ` [PATCH v7 09/31] ext4: add iomap address space operations for buffered I/O Zhang Yi
2026-10-09 10:30 ` [PATCH v7 10/31] ext4: implement buffered read path using iomap Zhang Yi
2026-10-09 10:30 ` [PATCH v7 11/31] ext4: pass out extent seq counter when mapping da blocks Zhang Yi
2026-10-09 10:30 ` [PATCH v7 12/31] ext4: do not use data=ordered mode for inodes using buffered iomap path Zhang Yi
2026-10-09 10:30 ` [PATCH v7 13/31] ext4: implement buffered write path using iomap Zhang Yi
2026-10-09 10:30 ` [PATCH v7 14/31] ext4: rework handle credit accounting for unwritten extent conversion Zhang Yi
2026-10-09 10:30 ` [PATCH v7 15/31] ext4: implement writeback path using iomap Zhang Yi
2026-10-09 10:30 ` [PATCH v7 16/31] ext4: implement mmap " Zhang Yi
2026-10-09 10:30 ` [PATCH v7 17/31] ext4: implement partial block zero range " Zhang Yi
2026-10-09 10:30 ` [PATCH v7 18/31] ext4: drain writeback before removing extents on the iomap path Zhang Yi
2026-10-09 10:30 ` [PATCH v7 19/31] ext4: add block mapping tracepoints for iomap buffered I/O path Zhang Yi
2026-10-09 10:30 ` [PATCH v7 20/31] ext4: disable online defrag when inode using " Zhang Yi
2026-10-09 10:30 ` [PATCH v7 21/31] ext4: add EXT4_STATE_DISKSIZE_GROW_PENDING state bit and helpers Zhang Yi
2026-10-09 10:30 ` [PATCH v7 22/31] ext4: submit and wait for pending disksize-grow I/O on writeback Zhang Yi
2026-10-09 10:30 ` [PATCH v7 23/31] ext4: advance i_disksize to i_size upon disksize-grow I/O completion Zhang Yi
2026-10-09 10:30 ` [PATCH v7 24/31] ext4: defer i_disksize update while DISKSIZE_GROW_PENDING is set Zhang Yi
2026-10-09 10:30 ` [PATCH v7 25/31] ext4: submit and wait for disksize-grow I/O in fallocate paths Zhang Yi
2026-10-09 10:30 ` [PATCH v7 26/31] ext4: clear DISKSIZE_GROW_PENDING on truncate or error Zhang Yi
2026-10-09 10:30 ` [PATCH v7 27/31] ext4: set DISKSIZE_GROW_PENDING after zeroing unaligned EOF block Zhang Yi
2026-10-10  9:00   ` Ojaswin Mujoo
2026-10-10  9:50     ` Zhang Yi [this message]
2026-10-10 13:49       ` Ojaswin Mujoo
2026-10-10 15:54         ` Zhang Yi
2026-10-11  9:30           ` Ojaswin Mujoo
2026-10-09 10:30 ` [PATCH v7 28/31] ext4: add tracepoints for DISKSIZE_GROW_PENDING set, clear, and wait Zhang Yi
2026-10-09 10:38 ` [PATCH v7 29/31] ext4: add tracepoints for EOF block zeroing and disksize-grow I/O Zhang Yi
2026-10-09 10:38 ` [PATCH v7 30/31] ext4: partially enable iomap for the buffered I/O path of regular files Zhang Yi
2026-10-09 10:38 ` [PATCH v7 31/31] ext4: introduce a mount option for iomap buffered I/O path Zhang Yi
2026-10-09 17:54 ` [syzbot ci] Re: ext4: use iomap for regular file's " syzbot ci
2026-10-10  8:44   ` Zhang Yi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fb2df2f-090c-4b24-9636-d28454a51d1d@gmail.com \
    --to=yizhang089@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=chengzhihao1@huawei.com \
    --cc=djwong@kernel.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=libaokun@linux.alibaba.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=tytso@mit.edu \
    --cc=wangkefeng.wang@huawei.com \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yi.zhang@huaweicloud.com \
    --cc=yukuai@fnnas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®