* Re: [PATCH 26/41] fs: Drop osync_buffers_list()
From: Christoph Hellwig @ 2026-03-24 5:44 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-67-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 25/41] kvm: Use private inode list instead of i_private_list
From: Christoph Hellwig @ 2026-03-24 5:44 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise, kvm, Paolo Bonzini
In-Reply-To: <20260320134100.20731-66-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 24/41] fs: Remove i_private_data
From: Christoph Hellwig @ 2026-03-24 5:43 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-65-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 23/41] aio: Stop using i_private_data and i_private_lock
From: Christoph Hellwig @ 2026-03-24 5:43 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-64-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 22/41] hugetlbfs: Stop using i_private_data
From: Christoph Hellwig @ 2026-03-24 5:42 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-63-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 21/41] fs: Stop using i_private_data for metadata bh tracking
From: Christoph Hellwig @ 2026-03-24 5:42 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-62-jack@suse.cz>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 20/41] fs: Ignore inode metadata buffers in inode_lru_isolate()
From: Christoph Hellwig @ 2026-03-24 5:42 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-61-jack@suse.cz>
On Fri, Mar 20, 2026 at 02:41:15PM +0100, Jan Kara wrote:
> There are only a few filesystems that use generic tracking of inode
> metadata buffer heads. As such it is mostly pointless to verify such
> attached buffer heads during inode reclaim. Drop the handling from
> inode_lru_isolate().
But the code isn't just verifying (which to me implies debug code),
but doing actual work to remove the buffers. This does look like a
behavior change to me, buf it is not due to previous patches or
because it was dead code, it would help greatly to explain that here.
^ permalink raw reply
* Re: [PATCH 12/41] fs: Drop sync_mapping_buffers() from __generic_file_fsync()
From: Christoph Hellwig @ 2026-03-24 5:40 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-53-jack@suse.cz>
On Fri, Mar 20, 2026 at 02:41:07PM +0100, Jan Kara wrote:
> No filesystem calling __generic_file_fsync() uses metadata bh tracking.
> Drop sync_mapping_buffers() call from __generic_file_fsync() as it's
> pointless now.
Given how much this changed, maybe rename it to simple_fsync now to
provide an obvious breakage for anyone trying to use it? That name
is probably also better as it's not all that generic.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/libfs.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 74134ba2e8d1..548e119668df 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1555,23 +1555,19 @@ int __generic_file_fsync(struct file *file, loff_t start, loff_t end,
> {
> struct inode *inode = file->f_mapping->host;
> int err;
> - int ret;
> + int ret = 0;
>
> err = file_write_and_wait_range(file, start, end);
> if (err)
> return err;
>
> inode_lock(inode);
> - ret = sync_mapping_buffers(inode->i_mapping);
> if (!(inode_state_read_once(inode) & I_DIRTY_ALL))
> goto out;
> if (datasync && !(inode_state_read_once(inode) & I_DIRTY_DATASYNC))
> goto out;
>
> - err = sync_inode_metadata(inode, 1);
> - if (ret == 0)
> - ret = err;
> -
> + ret = sync_inode_metadata(inode, 1);
> out:
> inode_unlock(inode);
> /* check and advance again to catch errors after syncing out buffers */
> --
> 2.51.0
>
>
---end quoted text---
^ permalink raw reply
* Re: [PATCH 08/41] udf: Switch to generic_buffers_fsync()
From: Christoph Hellwig @ 2026-03-24 5:38 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
OGAWA Hirofumi, Muchun Song, Oscar Salvador, David Hildenbrand,
linux-mm, linux-aio, Benjamin LaHaise
In-Reply-To: <20260320134100.20731-49-jack@suse.cz>
On Fri, Mar 20, 2026 at 02:41:03PM +0100, Jan Kara wrote:
> UDF uses metadata bh list attached to inode. Switch it to
> generic_buffers_fsync() instead of generic_file_fsync().
Can you explain this a bit more? Right now the only difference between
generic_file_fsync and generic_buffers_fsync is that the former takes
i_rwsem and the other does not. I'd expect the commit log to explain
why dropping the lock is safe and desirable.
Same for the other similar patches.
^ permalink raw reply
* Re: [PATCH 06/10] ext4: move ordered data handling out of ext4_block_do_zero_range()
From: Zhang Yi @ 2026-03-24 3:17 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089, yangerkun,
yukuai
In-Reply-To: <cfniq2uvcf5al6ltvbw5s46nacv5a2j6vh57npu64rlqbftkrd@hel5yrb6f22s>
On 3/24/2026 12:59 AM, Jan Kara wrote:
> On Tue 10-03-26 09:40:57, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> Remove the handle parameter from ext4_block_do_zero_range() and move the
>> ordered data handling to ext4_block_zero_eof().
>>
>> This is necessary for truncate up and append writes across a range
>> extending beyond EOF. The ordered data must be committed before updating
>> i_disksize to prevent exposing stale on-disk data from concurrent
>> post-EOF mmap writes during previous folio writeback or in case of
>> system crash during append writes.
>>
>> This is unnecessary for partial block hole punching because the entire
>> punch operation does not provide atomicity guarantees and can already
>> expose intermediate results in case of crash.
>
> Also because hole punching can only ever expose data that was there before
> the punch but missed zeroing during append / truncate could expose data
> that was not visible in the file before the operation.
Yeah, right, will add.
Thanks,
Yi.
>
>> Since ordered data handling is no longer performed inside
>> ext4_zero_partial_blocks(), ext4_punch_hole() no longer needs to attach
>> jinode.
>>
>> This is prepared for the conversion to the iomap infrastructure, which
>> does not use ordered data mode while zeroing post-EOF partial blocks.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>
> The patch looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Honza
>
>> ---
>> fs/ext4/inode.c | 58 ++++++++++++++++++++++++-------------------------
>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 86f169df226a..8fea044b3bff 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4076,12 +4076,12 @@ static struct buffer_head *ext4_block_get_zero_range(struct inode *inode,
>> return err ? ERR_PTR(err) : NULL;
>> }
>>
>> -static int ext4_block_do_zero_range(handle_t *handle, struct inode *inode,
>> - loff_t from, loff_t length, bool *did_zero)
>> +static int ext4_block_do_zero_range(struct inode *inode, loff_t from,
>> + loff_t length, bool *did_zero,
>> + bool *zero_written)
>> {
>> struct buffer_head *bh;
>> struct folio *folio;
>> - int err = 0;
>>
>> bh = ext4_block_get_zero_range(inode, from, length);
>> if (IS_ERR_OR_NULL(bh))
>> @@ -4092,19 +4092,14 @@ static int ext4_block_do_zero_range(handle_t *handle, struct inode *inode,
>> BUFFER_TRACE(bh, "zeroed end of block");
>>
>> mark_buffer_dirty(bh);
>> - /*
>> - * Only the written block requires ordered data to prevent exposing
>> - * stale data.
>> - */
>> - if (ext4_should_order_data(inode) &&
>> - !buffer_unwritten(bh) && !buffer_delay(bh))
>> - err = ext4_jbd2_inode_add_write(handle, inode, from, length);
>> - if (!err && did_zero)
>> + if (did_zero)
>> *did_zero = true;
>> + if (zero_written && !buffer_unwritten(bh) && !buffer_delay(bh))
>> + *zero_written = true;
>>
>> folio_unlock(folio);
>> folio_put(folio);
>> - return err;
>> + return 0;
>> }
>>
>> static int ext4_block_journalled_zero_range(handle_t *handle,
>> @@ -4148,7 +4143,8 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
>> * that corresponds to 'from'
>> */
>> static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
>> - loff_t from, loff_t length, bool *did_zero)
>> + loff_t from, loff_t length, bool *did_zero,
>> + bool *zero_written)
>> {
>> unsigned blocksize = inode->i_sb->s_blocksize;
>> unsigned int max = blocksize - (from & (blocksize - 1));
>> @@ -4167,7 +4163,8 @@ static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
>> return ext4_block_journalled_zero_range(handle, inode, from,
>> length, did_zero);
>> }
>> - return ext4_block_do_zero_range(handle, inode, from, length, did_zero);
>> + return ext4_block_do_zero_range(inode, from, length, did_zero,
>> + zero_written);
>> }
>>
>> /*
>> @@ -4184,6 +4181,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
>> unsigned int offset;
>> loff_t length = end - from;
>> bool did_zero = false;
>> + bool zero_written = false;
>> int err;
>>
>> offset = from & (blocksize - 1);
>> @@ -4196,9 +4194,22 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
>> if (length > blocksize - offset)
>> length = blocksize - offset;
>>
>> - err = ext4_block_zero_range(handle, inode, from, length, &did_zero);
>> + err = ext4_block_zero_range(handle, inode, from, length,
>> + &did_zero, &zero_written);
>> if (err)
>> return err;
>> + /*
>> + * It's necessary to order zeroed data before update i_disksize when
>> + * 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.
>> + */
>> + if (ext4_should_order_data(inode) &&
>> + did_zero && zero_written && !IS_DAX(inode)) {
>> + err = ext4_jbd2_inode_add_write(handle, inode, from, length);
>> + if (err)
>> + return err;
>> + }
>>
>> return did_zero ? length : 0;
>> }
>> @@ -4222,13 +4233,13 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
>> if (start == end &&
>> (partial_start || (partial_end != sb->s_blocksize - 1))) {
>> err = ext4_block_zero_range(handle, inode, lstart,
>> - length, NULL);
>> + length, NULL, NULL);
>> return err;
>> }
>> /* Handle partial zero out on the start of the range */
>> if (partial_start) {
>> err = ext4_block_zero_range(handle, inode, lstart,
>> - sb->s_blocksize, NULL);
>> + sb->s_blocksize, NULL, NULL);
>> if (err)
>> return err;
>> }
>> @@ -4236,7 +4247,7 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
>> if (partial_end != sb->s_blocksize - 1)
>> err = ext4_block_zero_range(handle, inode,
>> byte_end - partial_end,
>> - partial_end + 1, NULL);
>> + partial_end + 1, NULL, NULL);
>> return err;
>> }
>>
>> @@ -4411,17 +4422,6 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>> end = max_end;
>> length = end - offset;
>>
>> - /*
>> - * Attach jinode to inode for jbd2 if we do any zeroing of partial
>> - * block.
>> - */
>> - if (!IS_ALIGNED(offset | end, sb->s_blocksize)) {
>> - ret = ext4_inode_attach_jinode(inode);
>> - if (ret < 0)
>> - return ret;
>> - }
>> -
>> -
>> ret = ext4_update_disksize_before_punch(inode, offset, length);
>> if (ret)
>> return ret;
>> --
>> 2.52.0
>>
^ permalink raw reply
* Re: [PATCH 10/10] ext4: zero post-EOF partial block before appending write
From: Zhang Yi @ 2026-03-24 3:29 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089, yangerkun,
yukuai
In-Reply-To: <fcjjsad6v5v6ubtjidrstg3pjeeqn7njjm2qrovufdklnbhhci@ldy4nrlmtoek>
On 3/24/2026 4:31 AM, Jan Kara wrote:
> On Tue 10-03-26 09:41:01, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> In cases of appending write beyond EOF, ext4_zero_partial_blocks() is
>> called within ext4_*_write_end() to zero out the partial block beyond
>> EOF. This prevents exposing stale data that might be written through
>> mmap.
>>
>> However, supporting only the regular buffered write path is
>> insufficient. It is also necessary to support the DAX path as well as
>> the upcoming iomap buffered write path. Therefore, move this operation
>> to ext4_write_checks().
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>
> I'd note that this allows page fault to race in between the zeroing and
> actual write resulting in new possible result - previously for file size 8,
> pwrite('WWWW...', 8, 16) racing with mmap writes of 'MMMMMM...' at offset 8
> into the page you could see either:
>
> DDDDDDDD00000000WWWWWWWW
> or
> DDDDDDDDMMMMMMMMMMMMMMMM
>
>
> now you can see both of the above an also
>
> DDDDDDDMMMMMMMMWWWWWWWWW
>
> But I don't think that's strictly invalid content and userspace that would
> depend on the outcome of such race would be silly. So feel free to add:
Yes exactly. :-)
Thanks,
Yi.
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Honza
>
>> ---
>> fs/ext4/file.c | 14 ++++++++++++++
>> fs/ext4/inode.c | 21 +++++++--------------
>> 2 files changed, 21 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
>> index f1dc5ce791a7..b2e44601ab6a 100644
>> --- a/fs/ext4/file.c
>> +++ b/fs/ext4/file.c
>> @@ -271,6 +271,8 @@ static ssize_t ext4_generic_write_checks(struct kiocb *iocb,
>>
>> static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
>> {
>> + struct inode *inode = file_inode(iocb->ki_filp);
>> + loff_t old_size = i_size_read(inode);
>> ssize_t ret, count;
>>
>> count = ext4_generic_write_checks(iocb, from);
>> @@ -280,6 +282,18 @@ static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
>> ret = file_modified(iocb->ki_filp);
>> if (ret)
>> return ret;
>> +
>> + /*
>> + * If the position is beyond the EOF, it is necessary to zero out the
>> + * partial block that beyond the existing EOF, as it may contains
>> + * stale data written through mmap.
>> + */
>> + if (iocb->ki_pos > old_size && !ext4_verity_in_progress(inode)) {
>> + ret = ext4_block_zero_eof(inode, old_size, iocb->ki_pos);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> return count;
>> }
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 5288d36b0f09..67a4d12fcb4d 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -1456,10 +1456,9 @@ static int ext4_write_end(const struct kiocb *iocb,
>> folio_unlock(folio);
>> folio_put(folio);
>>
>> - if (old_size < pos && !verity) {
>> + if (old_size < pos && !verity)
>> pagecache_isize_extended(inode, old_size, pos);
>> - ext4_block_zero_eof(inode, old_size, pos);
>> - }
>> +
>> /*
>> * Don't mark the inode dirty under folio lock. First, it unnecessarily
>> * makes the holding time of folio lock longer. Second, it forces lock
>> @@ -1574,10 +1573,8 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
>> folio_unlock(folio);
>> folio_put(folio);
>>
>> - if (old_size < pos && !verity) {
>> + if (old_size < pos && !verity)
>> pagecache_isize_extended(inode, old_size, pos);
>> - ext4_block_zero_eof(inode, old_size, pos);
>> - }
>>
>> if (size_changed) {
>> ret2 = ext4_mark_inode_dirty(handle, inode);
>> @@ -3196,7 +3193,7 @@ static int ext4_da_do_write_end(struct address_space *mapping,
>> struct inode *inode = mapping->host;
>> loff_t old_size = inode->i_size;
>> bool disksize_changed = false;
>> - loff_t new_i_size, zero_len = 0;
>> + loff_t new_i_size;
>> handle_t *handle;
>>
>> if (unlikely(!folio_buffers(folio))) {
>> @@ -3240,19 +3237,15 @@ static int ext4_da_do_write_end(struct address_space *mapping,
>> folio_unlock(folio);
>> folio_put(folio);
>>
>> - if (pos > old_size) {
>> + if (pos > old_size)
>> pagecache_isize_extended(inode, old_size, pos);
>> - zero_len = pos - old_size;
>> - }
>>
>> - if (!disksize_changed && !zero_len)
>> + if (!disksize_changed)
>> return copied;
>>
>> - handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
>> + handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
>> if (IS_ERR(handle))
>> return PTR_ERR(handle);
>> - if (zero_len)
>> - ext4_block_zero_eof(inode, old_size, pos);
>> ext4_mark_inode_dirty(handle, inode);
>> ext4_journal_stop(handle);
>>
>> --
>> 2.52.0
>>
^ permalink raw reply
* Re: [PATCH 04/10] ext4: factor out journalled block zeroing range
From: Zhang Yi @ 2026-03-24 3:16 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089, yangerkun,
yukuai
In-Reply-To: <gvsj27lkqxs6xy7rdiyarw5ocjuvqrrn2knx5jo45eoc7tw6no@kow27zlgjy5r>
On 3/24/2026 12:48 AM, Jan Kara wrote:
> On Tue 10-03-26 09:40:55, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> Refactor __ext4_block_zero_page_range() by separating the block zeroing
>> operations for ordered data mode and journal data mode into two distinct
>> functions:
>>
>> - ext4_block_do_zero_range(): handles non-journal data mode with
>> ordered data support
>> - ext4_block_journalled_zero_range(): handles journal data mode
>>
>> Also extract a common helper, ext4_block_get_zero_range(), to handle
>> buffer head and folio retrieval, along with the associated error
>> handling. This prepares for converting the partial block zero range to
>> the iomap infrastructure.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>
> Just one nit below. Otherwise feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 52c6a86ad9f9..d63d455831b9 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4002,13 +4002,12 @@ void ext4_set_aops(struct inode *inode)
>> * ext4_punch_hole, etc) which needs to be properly zeroed out. Otherwise a
>> * racing writeback can come later and flush the stale pagecache to disk.
>> */
>> -static int __ext4_block_zero_page_range(handle_t *handle,
>> - struct address_space *mapping, loff_t from, loff_t length,
>> - bool *did_zero)
>> +static struct buffer_head *ext4_block_get_zero_range(struct inode *inode,
>> + loff_t from, loff_t length)
>
> I'd call this function ext4_load_tail_bh() and AFAICT the 'length' argument
> is unused so it can be dropped.
>
> Honza
Ha, sounds better, will do in my next iteration.
Thanks,
Yi.
^ permalink raw reply
* Re: [PATCH 09/10] ext4: move zero partial block range functions out of active handle
From: Zhang Yi @ 2026-03-24 3:14 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
ojaswin, ritesh.list, libaokun, yizhang089, yangerkun, yukuai,
Zhang Yi, Zhang Yi
In-Reply-To: <7fa8415e-199f-4e31-8b07-08f0f73d5b99@huaweicloud.com>
On 3/24/2026 11:10 AM, Zhang Yi wrote:
> On 3/24/2026 4:17 AM, Jan Kara wrote:
>> On Tue 10-03-26 09:41:00, Zhang Yi wrote:
>>> From: Zhang Yi <yi.zhang@huawei.com>
>>>
>>> Move ext4_block_zero_eof() and ext4_zero_partial_blocks() calls out of
>>> the active handle context, making them independent operations. This is
>>> safe because it still ensures data is updated before metadata for
>>> data=ordered mode and data=journal mode because we still zero data and
>>> ordering data before modifying the metadata.
>>>
>>> This change is required for iomap infrastructure conversion because the
>>> iomap buffered I/O path does not use the same journal infrastructure for
>>> partial block zeroing. The lock ordering of folio lock and starting
>>> transactions is "folio lock -> transaction start", which is opposite of
>>> the current path. Therefore, zeroing partial blocks cannot be performed
>>> under the active handle.
>>>
>>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>>
>> So in this patch you also move ext4_zero_partial_blocks() before the
>> transaction start in ext4_zero_range() and ext4_punch_hole(). However
>> cannot these operations in theory fail with error such as EDQUOT or ENOSPC
>> when they need to grow the extent tree as a result of the operation? In
>> that case we'd return such error but partial blocks are already zeroed out
>> which is a problem...
>
> Thank you for review this patch set!
>
> Let me see, I believe you are referring to the cases where
> ext4_xxx_remove_space() in ext4_punch_hole() and
> ext4_alloc_file_blocks() in ext4_zero_range() return normal error codes
> such as EDQUOT or ENOSPC.
>
> In ext4_punch_hole(), we first zero out the partial blocks at the
> beginning and end of the punch range, and then release the aligned
> blocks in the middle. If errors occur during the middle of the
> hole-punching operation, there will left some data in the middle of the
> punch range, this is weird. Conversely, if the zeroization is performed
> in sequence, the result is zeroization at the front and the retention of
> valid data at the rear, which is acceptable. right? Besides, this
> problem seems not occur in ext4_zero_range() because
> ext4_zero_partial_blocks() is still called after
> ext4_alloc_file_blocks() after this patch.
>
> Regarding this, I found that the result is the same whether
> ext4_alloc_file_blocks() is called inside transaction start or not. The
> partial blocks at the head and tail have already been zeroed out after
> calling ext4_zero_partial_blocks(), am I missing some thing? Or do you
> suggest that we should also move ext4_zero_partial_blocks() to execute
> after ext4_xxx_remove_space() in ext4_punch_hole()?
>
>> OTOH in these cases we don't need to order the data
>> at all so in principle we shouldn't need to move the zeroing earlier
>> here. Am I missing something?
>>
>
> Moving the zero partial block before the start handle is not only
> because they do not need to order the data, but rather to avoid
> deadlock. iomap_zero_range() acquires the folio lock, and if it is
> called after ext4_journal_start(), the locking order would be
> transaction start -> folio lock, which is the opposite of the locking
> order used in the writeback process. If these two operations are
> performed concurrently (), it could potentially result in an ABBA
^^ A
> deadlock.
>
> In the subsequent iomap patch [1], I will add a WARNING and a comment
> in ext4_iomap_block_zero_range(I don't see anything can prevent this)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sorry, this sentence should be placed at position A.
> to ensure it is not called under an active handle.
>
> [1] https://lore.kernel.org/linux-ext4/20260203062523.3869120-18-yi.zhang@huawei.com/
>
> Thanks,
> Yi.
>
>> Honza
>>
>>> @@ -4722,25 +4724,18 @@ static long ext4_zero_range(struct file *file, loff_t offset,
>>> if (IS_ALIGNED(offset | end, blocksize))
>>> return ret;
>>>
>>> - /*
>>> - * In worst case we have to writeout two nonadjacent unwritten
>>> - * blocks and update the inode
>>> - */
>>> - credits = (2 * ext4_ext_index_trans_blocks(inode, 2)) + 1;
>>> - if (ext4_should_journal_data(inode))
>>> - credits += 2;
>>> - handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
>>> + /* Zero out partial block at the edges of the range */
>>> + ret = ext4_zero_partial_blocks(inode, offset, len);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
>>> if (IS_ERR(handle)) {
>>> ret = PTR_ERR(handle);
>>> ext4_std_error(inode->i_sb, ret);
>>> return ret;
>>> }
>>>
>>> - /* Zero out partial block at the edges of the range */
>>> - ret = ext4_zero_partial_blocks(inode, offset, len);
>>> - if (ret)
>>> - goto out_handle;
>>> -
>>> if (new_size)
>>> ext4_update_inode_size(inode, new_size);
>>> ret = ext4_mark_inode_dirty(handle, inode);
>>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>>> index d5b783a7c814..5288d36b0f09 100644
>>> --- a/fs/ext4/inode.c
>>> +++ b/fs/ext4/inode.c
>>> @@ -4443,8 +4443,12 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>>> if (ret)
>>> return ret;
>>>
>>> + ret = ext4_zero_partial_blocks(inode, offset, length);
>>> + if (ret)
>>> + return ret;
>>> +
>>> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
>>> - credits = ext4_chunk_trans_extent(inode, 2);
>>> + credits = ext4_chunk_trans_extent(inode, 0);
>>> else
>>> credits = ext4_blocks_for_truncate(inode);
>>> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
>>> @@ -4454,10 +4458,6 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>>> return ret;
>>> }
>>>
>>> - ret = ext4_zero_partial_blocks(inode, offset, length);
>>> - if (ret)
>>> - goto out_handle;
>>> -
>>> /* If there are blocks to remove, do it */
>>> start_lblk = EXT4_B_TO_LBLK(inode, offset);
>>> end_lblk = end >> inode->i_blkbits;
>
^ permalink raw reply
* Re: [PATCH 09/10] ext4: move zero partial block range functions out of active handle
From: Zhang Yi @ 2026-03-24 3:10 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089, yangerkun,
yukuai
In-Reply-To: <irkaodowvqmpug3moetmd75vssj6czh67qzuxs4v5vxe56qxz7@opkvl3apdpuk>
On 3/24/2026 4:17 AM, Jan Kara wrote:
> On Tue 10-03-26 09:41:00, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> Move ext4_block_zero_eof() and ext4_zero_partial_blocks() calls out of
>> the active handle context, making them independent operations. This is
>> safe because it still ensures data is updated before metadata for
>> data=ordered mode and data=journal mode because we still zero data and
>> ordering data before modifying the metadata.
>>
>> This change is required for iomap infrastructure conversion because the
>> iomap buffered I/O path does not use the same journal infrastructure for
>> partial block zeroing. The lock ordering of folio lock and starting
>> transactions is "folio lock -> transaction start", which is opposite of
>> the current path. Therefore, zeroing partial blocks cannot be performed
>> under the active handle.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>
> So in this patch you also move ext4_zero_partial_blocks() before the
> transaction start in ext4_zero_range() and ext4_punch_hole(). However
> cannot these operations in theory fail with error such as EDQUOT or ENOSPC
> when they need to grow the extent tree as a result of the operation? In
> that case we'd return such error but partial blocks are already zeroed out
> which is a problem...
Thank you for review this patch set!
Let me see, I believe you are referring to the cases where
ext4_xxx_remove_space() in ext4_punch_hole() and
ext4_alloc_file_blocks() in ext4_zero_range() return normal error codes
such as EDQUOT or ENOSPC.
In ext4_punch_hole(), we first zero out the partial blocks at the
beginning and end of the punch range, and then release the aligned
blocks in the middle. If errors occur during the middle of the
hole-punching operation, there will left some data in the middle of the
punch range, this is weird. Conversely, if the zeroization is performed
in sequence, the result is zeroization at the front and the retention of
valid data at the rear, which is acceptable. right? Besides, this
problem seems not occur in ext4_zero_range() because
ext4_zero_partial_blocks() is still called after
ext4_alloc_file_blocks() after this patch.
Regarding this, I found that the result is the same whether
ext4_alloc_file_blocks() is called inside transaction start or not. The
partial blocks at the head and tail have already been zeroed out after
calling ext4_zero_partial_blocks(), am I missing some thing? Or do you
suggest that we should also move ext4_zero_partial_blocks() to execute
after ext4_xxx_remove_space() in ext4_punch_hole()?
> OTOH in these cases we don't need to order the data
> at all so in principle we shouldn't need to move the zeroing earlier
> here. Am I missing something?
>
Moving the zero partial block before the start handle is not only
because they do not need to order the data, but rather to avoid
deadlock. iomap_zero_range() acquires the folio lock, and if it is
called after ext4_journal_start(), the locking order would be
transaction start -> folio lock, which is the opposite of the locking
order used in the writeback process. If these two operations are
performed concurrently (), it could potentially result in an ABBA
deadlock.
In the subsequent iomap patch [1], I will add a WARNING and a comment
in ext4_iomap_block_zero_range(I don't see anything can prevent this)
to ensure it is not called under an active handle.
[1] https://lore.kernel.org/linux-ext4/20260203062523.3869120-18-yi.zhang@huawei.com/
Thanks,
Yi.
> Honza
>
>> @@ -4722,25 +4724,18 @@ static long ext4_zero_range(struct file *file, loff_t offset,
>> if (IS_ALIGNED(offset | end, blocksize))
>> return ret;
>>
>> - /*
>> - * In worst case we have to writeout two nonadjacent unwritten
>> - * blocks and update the inode
>> - */
>> - credits = (2 * ext4_ext_index_trans_blocks(inode, 2)) + 1;
>> - if (ext4_should_journal_data(inode))
>> - credits += 2;
>> - handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
>> + /* Zero out partial block at the edges of the range */
>> + ret = ext4_zero_partial_blocks(inode, offset, len);
>> + if (ret)
>> + return ret;
>> +
>> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
>> if (IS_ERR(handle)) {
>> ret = PTR_ERR(handle);
>> ext4_std_error(inode->i_sb, ret);
>> return ret;
>> }
>>
>> - /* Zero out partial block at the edges of the range */
>> - ret = ext4_zero_partial_blocks(inode, offset, len);
>> - if (ret)
>> - goto out_handle;
>> -
>> if (new_size)
>> ext4_update_inode_size(inode, new_size);
>> ret = ext4_mark_inode_dirty(handle, inode);
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index d5b783a7c814..5288d36b0f09 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4443,8 +4443,12 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>> if (ret)
>> return ret;
>>
>> + ret = ext4_zero_partial_blocks(inode, offset, length);
>> + if (ret)
>> + return ret;
>> +
>> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
>> - credits = ext4_chunk_trans_extent(inode, 2);
>> + credits = ext4_chunk_trans_extent(inode, 0);
>> else
>> credits = ext4_blocks_for_truncate(inode);
>> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
>> @@ -4454,10 +4458,6 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>> return ret;
>> }
>>
>> - ret = ext4_zero_partial_blocks(inode, offset, length);
>> - if (ret)
>> - goto out_handle;
>> -
>> /* If there are blocks to remove, do it */
>> start_lblk = EXT4_B_TO_LBLK(inode, offset);
>> end_lblk = end >> inode->i_blkbits;
^ permalink raw reply
* [PATCH v2] ext4: skip split extent recovery on corruption
From: hongao @ 2026-03-24 1:58 UTC (permalink / raw)
To: tytso, adilger.kernel, jack, yi.zhang, ojaswin
Cc: linux-ext4, linux-kernel, syzbot+1ffa5d865557e51cb604
In-Reply-To: <FA074540F4E30F27+20260323015752.60064-1-hongao@uniontech.com>
ext4_split_extent_at() retries after ext4_ext_insert_extent() fails by
refinding the original extent and restoring its length. That recovery is
only safe for transient resource failures such as -ENOSPC, -EDQUOT, and
-ENOMEM.
When ext4_ext_insert_extent() fails because the extent tree is already
corrupted, ext4_find_extent() can return a leaf path without p_ext.
ext4_split_extent_at() then dereferences path[depth].p_ext while trying to
fix up the original extent length, causing a NULL pointer dereference while
handling a pre-existing filesystem corruption.
Do not enter the recovery path for corruption errors, and validate p_ext
after refinding the extent before touching it. This keeps the recovery path
limited to cases it can actually repair and turns the syzbot-triggered crash
into a proper corruption report.
Fixes: 716b9c23b862 ("ext4: refactor split and convert extents")
Reported-by: syzbot+1ffa5d865557e51cb604@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1ffa5d865557e51cb604
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Signed-off-by: hongao <hongao@uniontech.com>
---
v2:
- move the p_ext validation before ext4_ext_get_access()
- add Reviewed-by tags from Jan Kara and Zhang Yi
fs/ext4/extents.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ae3804f36535..10af8dee0f1a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3239,6 +3239,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
insert_err = PTR_ERR(path);
err = 0;
+ if (insert_err != -ENOSPC && insert_err != -EDQUOT &&
+ insert_err != -ENOMEM)
+ goto out_path;
/*
* Get a new path to try to zeroout or fix the extent length.
@@ -3255,13 +3258,20 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
goto out_path;
}
+ depth = ext_depth(inode);
+ ex = path[depth].p_ext;
+ if (!ex) {
+ EXT4_ERROR_INODE(inode,
+ "bad extent address lblock: %lu, depth: %d pblock %lld",
+ (unsigned long)ee_block, depth, path[depth].p_block);
+ err = -EFSCORRUPTED;
+ goto out;
+ }
+
err = ext4_ext_get_access(handle, inode, path + depth);
if (err)
goto out;
- depth = ext_depth(inode);
- ex = path[depth].p_ext;
-
fix_extent_len:
ex->ee_len = orig_ex.ee_len;
err = ext4_ext_dirty(handle, inode, path + path->p_depth);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] ext4: skip split extent recovery on corruption
From: hongao @ 2026-03-24 1:40 UTC (permalink / raw)
To: tytso, adilger.kernel, jack, yi.zhang, ojaswin
Cc: linux-ext4, linux-kernel, syzbot+1ffa5d865557e51cb604
In-Reply-To: <FA074540F4E30F27+20260323015752.60064-1-hongao@uniontech.com>
Hi Jan, Yi, Ojaswin,
Thank you for reviewing the patch.
I will send a v2 that moves the p_ext validation before
ext4_ext_get_access(), as Yi suggested.
Also, thanks for the Reviewed-by tags:
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Regarding Ojaswin's questions:
I do not have a local reproducer at the moment. My analysis was based on
the syzbot report, the crash trace, and code inspection.
For the p_ext == NULL case, a successful ext4_find_extent() only means
that we were able to walk the tree down to a leaf. It does not guarantee
that the leaf still contains a valid extent entry for the target logical
block.
path[depth].p_ext is derived from the extent entries stored in that leaf.
If the leaf metadata is already corrupted, ext4_find_extent() may still
return a non-error path, but p_ext can be NULL because there is no usable
extent entry there anymore.
So in the corruption case, a valid path is not enough to continue the
recovery path safely. Returning -EFSCORRUPTED is safer than
dereferencing p_ext and crashing while trying to repair already-broken
metadata.
Thanks,
Hongao
^ permalink raw reply
* [bug report] e2fsprogs 1.47.4: mke2fs creates oversized orphan file for 1k blocksize (breaks mount)
From: Yi Zhang @ 2026-03-24 0:54 UTC (permalink / raw)
To: linux-ext4; +Cc: linux-block, Shinichiro Kawasaki, Baokun Li, Theodore Ts'o
Hi
I'm reporting a regression in e2fsprogs-1.47.4 where mke2fs creates an
orphan file[2] that exceeds the kernel's maximum allowed size for
filesystems with a 1KB block size[1]. This causes subsequent mount
operations to fail.
This issue is currently causing blktests loop/007 to fail on latest
Fedora. Downgrading to e2fsprogs 1.47.3 resolves the issue.
And revert [3] fixed this issue, here is the reproducer[4].
[1]
Kernel limit for 1KB blocks: 512 * 1024 = 512 KB
[2]
Orphan file created by e2fsprogs 1.47.4: 1024 KB (1 MB)
[3]
6f03c698 libext2fs: fix orphan file size > kernel limit with large blocksize
[4]
# dd if=/dev/zero of=test.img bs=1M count=1024
# mkfs.ext4 -b 1024 test.img -F
mke2fs 1.47.4 (6-Mar-2025)
test.img contains a ext4 file system
last mounted on Mon Mar 23 20:32:40 2026
Discarding device blocks: done
Creating filesystem with 1048576 1k blocks and 65536 inodes
Filesystem UUID: 16683060-ed47-46cc-a040-c5e049785831
Superblock backups stored on blocks:
8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553,
1024001
Allocating group tables: done
Writing inode tables: done
Creating journal (16384 blocks): done
Writing superblocks and filesystem accounting information: done
# mount test.img /mnt
mount: /mnt: fsconfig() failed: Structure needs cleaning.
dmesg(1) may have more information after failed mount system call.
# dmesg
[ 5985.339637] loop0: detected capacity change from 0 to 2097152
[ 5985.347473] EXT4-fs (loop0): orphan file too big: 1048576
[ 5985.352939] EXT4-fs (loop0): mount failed
--
Best Regards,
Yi Zhang
^ permalink raw reply
* [REGRESSION] fs/overlayfs/ext4: Severe jbd2 lock contention and journal starvation on concurrent copy-up (v6.6 -> v6.12)
From: Chenglong Tang @ 2026-03-23 22:03 UTC (permalink / raw)
To: linux-unionfs, linux-ext4, linux-fsdevel, linux-kernel,
regressions
Cc: jack, Jan Kara, miklos, Amir Goldstein, tytso, adilger.kernel,
viro, brauner, Kevin Berry, Robert Kolchmeyer, Deepa Dinamani,
He Gao
Hi all,
We are tracking a severe performance regression in Google's
Container-Optimized OS (COS) that appeared when moving from the 6.6
LTS kernel to the 6.12 LTS kernel.
Under concurrent CI workloads (specifically, many containers doing
Python package compilation / .pyc generation simultaneously), the 6.12
kernel suffers from massive jbd2 journal contention. Processes hang
for 20-30 seconds waiting for VFS locks and journal space. On 6.6, the
exact same workload completes in ~4 seconds.
# Environment:
* Host FS: ext4 (backed by standard cloud block storage)
* Container FS: OverlayFS (Docker)
* Machine: n2d-highmem-96 (96 vCPU, high memory)
* Good Kernel: 6.6.87
* Bad Kernels: 6.12.55, 6.12.68
# The Bottleneck
During the 20+ second hang, `cat /proc/<pid>/stack` reveals three
distinct groups of blocked processes thrashing on the jbd2 journal.
The OverlayFS copy-up mechanism seems to be generating so many
synchronous ext4 transactions that it exhausts the jbd2 transaction
buffers.
1. Journal Space Exhaustion (Waiting to start transaction):
[<0>] __jbd2_log_wait_for_space+0xa3/0x240
[<0>] start_this_handle+0x42d/0x8a0
[<0>] jbd2__journal_start+0x103/0x1e0
[<0>] __ext4_journal_start_sb+0x129/0x1c0
[<0>] __ext4_new_inode+0x7cd/0x1290
[<0>] ext4_create+0xbc/0x1b0
[<0>] vfs_create+0x192/0x250
[<0>] ovl_create_real+0xd5/0x170
[<0>] ovl_create_or_link+0x1d7/0x7f0
2. VFS Rename / Copy-up Contention (Blocked by the slow sync):
[<0>] lock_rename+0x29/0x50
[<0>] ovl_copy_up_flags+0x84c/0x12e0
[<0>] ovl_create_object+0x4a/0x120
[<0>] vfs_mkdir+0x1aa/0x260
[<0>] do_mkdirat+0xb9/0x240
3. Synchronous Flush Blocking:
[<0>] jbd2_log_wait_commit+0x107/0x150
[<0>] jbd2_journal_force_commit+0x9c/0xc0
[<0>] ext4_sync_file+0x278/0x310
[<0>] ovl_sync_file+0x2f/0x50
[<0>] ovl_copy_up_metadata+0x455/0x4b0
# Minimal Reproducer
The issue is easily reproducible by triggering 20 concurrent cold
Python imports in Docker, which forces OverlayFS to copy-up the
`__pycache__` directories and write the `.pyc` files.
```bash
# 1. Build a clean image with no pre-compiled bytecode
cat << 'EOF' > Dockerfile
FROM python:3.10-slim
RUN pip install --quiet google-cloud-compute
RUN find /usr/local -type d -name "__pycache__" -exec rm -rf {} +
EOF
docker build -t clean-import-test .
# 2. Fire 20 concurrent imports
for i in {1..20}; do
docker run --rm clean-import-test bash -c 'time python -c "import
google.cloud.compute_v1"' > clean_test_cold_$i.log 2>&1 &
done
wait
grep "real" clean_test_cold_*.log
```
On 6.6.87, all 20 containers finish in ~4.3s.
On 6.12.x, they hang and finish between 17s and 27s. Bypassing disk
writes completely mitigates the regression on 6.12 (using
PYTHONDONTWRITEBYTECODE=1), confirming it is an ext4/overlayfs I/O
contention issue rather than a CPU scheduling one.
Because the regression spans from 6.6 to 6.12, bisection is quite
heavy. Before we initiate a full kernel bisect, does this symptom ring
a bell for any ext4 fast_commit, jbd2 locking, or OverlayFS
metacopy/sync changes introduced during this window?
Any pointers or patches you'd like us to test would be greatly appreciated.
Thanks,
Chenglong Tang
Google Container-Optimized OS Team
^ permalink raw reply
* Re: [PATCH v5 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer
From: Milos Nikic @ 2026-03-23 21:50 UTC (permalink / raw)
To: jack; +Cc: tytso, linux-ext4, linux-kernel, Zhang Yi, Andreas Dilger
In-Reply-To: <20260304172016.23525-2-nikic.milos@gmail.com>
Hello,
Just sending a gentle ping on this v5 series from March 4th.
It looks like both patches have collected Reviewed-by tags from Jan,
Andreas and Zhang.
Please let me know if there is anything else you need from my side, or
if this is good to be queued up in the ext4 tree for the next merge
window.
Thanks, Milos
On Wed, Mar 4, 2026 at 9:20 AM Milos Nikic <nikic.milos@gmail.com> wrote:
>
> In jbd2_journal_get_create_access(), if the caller passes an unlocked
> buffer, the code currently triggers a fatal J_ASSERT.
>
> While an unlocked buffer here is a clear API violation and a bug in the
> caller, crashing the entire system is an overly severe response. It brings
> down the whole machine for a localized filesystem inconsistency.
>
> Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
> stack trace, and return an error (-EINVAL). This allows the journal to
> gracefully abort the transaction, protecting data integrity without
> causing a kernel panic.
>
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
> fs/jbd2/transaction.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index dca4b5d8aaaa..04d17a5f2a82 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
> goto out;
> }
>
> - J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
> + if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
> + err = -EINVAL;
> + spin_unlock(&jh->b_state_lock);
> + jbd2_journal_abort(journal, err);
> + goto out;
> + }
>
> if (jh->b_transaction == NULL) {
> /*
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH 10/10] ext4: zero post-EOF partial block before appending write
From: Jan Kara @ 2026-03-23 20:31 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-11-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:41:01, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In cases of appending write beyond EOF, ext4_zero_partial_blocks() is
> called within ext4_*_write_end() to zero out the partial block beyond
> EOF. This prevents exposing stale data that might be written through
> mmap.
>
> However, supporting only the regular buffered write path is
> insufficient. It is also necessary to support the DAX path as well as
> the upcoming iomap buffered write path. Therefore, move this operation
> to ext4_write_checks().
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
I'd note that this allows page fault to race in between the zeroing and
actual write resulting in new possible result - previously for file size 8,
pwrite('WWWW...', 8, 16) racing with mmap writes of 'MMMMMM...' at offset 8
into the page you could see either:
DDDDDDDD00000000WWWWWWWW
or
DDDDDDDDMMMMMMMMMMMMMMMM
now you can see both of the above an also
DDDDDDDMMMMMMMMWWWWWWWWW
But I don't think that's strictly invalid content and userspace that would
depend on the outcome of such race would be silly. So feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/file.c | 14 ++++++++++++++
> fs/ext4/inode.c | 21 +++++++--------------
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index f1dc5ce791a7..b2e44601ab6a 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -271,6 +271,8 @@ static ssize_t ext4_generic_write_checks(struct kiocb *iocb,
>
> static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
> {
> + struct inode *inode = file_inode(iocb->ki_filp);
> + loff_t old_size = i_size_read(inode);
> ssize_t ret, count;
>
> count = ext4_generic_write_checks(iocb, from);
> @@ -280,6 +282,18 @@ static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
> ret = file_modified(iocb->ki_filp);
> if (ret)
> return ret;
> +
> + /*
> + * If the position is beyond the EOF, it is necessary to zero out the
> + * partial block that beyond the existing EOF, as it may contains
> + * stale data written through mmap.
> + */
> + if (iocb->ki_pos > old_size && !ext4_verity_in_progress(inode)) {
> + ret = ext4_block_zero_eof(inode, old_size, iocb->ki_pos);
> + if (ret < 0)
> + return ret;
> + }
> +
> return count;
> }
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5288d36b0f09..67a4d12fcb4d 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1456,10 +1456,9 @@ static int ext4_write_end(const struct kiocb *iocb,
> folio_unlock(folio);
> folio_put(folio);
>
> - if (old_size < pos && !verity) {
> + if (old_size < pos && !verity)
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_block_zero_eof(inode, old_size, pos);
> - }
> +
> /*
> * Don't mark the inode dirty under folio lock. First, it unnecessarily
> * makes the holding time of folio lock longer. Second, it forces lock
> @@ -1574,10 +1573,8 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
> folio_unlock(folio);
> folio_put(folio);
>
> - if (old_size < pos && !verity) {
> + if (old_size < pos && !verity)
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_block_zero_eof(inode, old_size, pos);
> - }
>
> if (size_changed) {
> ret2 = ext4_mark_inode_dirty(handle, inode);
> @@ -3196,7 +3193,7 @@ static int ext4_da_do_write_end(struct address_space *mapping,
> struct inode *inode = mapping->host;
> loff_t old_size = inode->i_size;
> bool disksize_changed = false;
> - loff_t new_i_size, zero_len = 0;
> + loff_t new_i_size;
> handle_t *handle;
>
> if (unlikely(!folio_buffers(folio))) {
> @@ -3240,19 +3237,15 @@ static int ext4_da_do_write_end(struct address_space *mapping,
> folio_unlock(folio);
> folio_put(folio);
>
> - if (pos > old_size) {
> + if (pos > old_size)
> pagecache_isize_extended(inode, old_size, pos);
> - zero_len = pos - old_size;
> - }
>
> - if (!disksize_changed && !zero_len)
> + if (!disksize_changed)
> return copied;
>
> - handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
> + handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> - if (zero_len)
> - ext4_block_zero_eof(inode, old_size, pos);
> ext4_mark_inode_dirty(handle, inode);
> ext4_journal_stop(handle);
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 09/10] ext4: move zero partial block range functions out of active handle
From: Jan Kara @ 2026-03-23 20:17 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-10-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:41:00, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Move ext4_block_zero_eof() and ext4_zero_partial_blocks() calls out of
> the active handle context, making them independent operations. This is
> safe because it still ensures data is updated before metadata for
> data=ordered mode and data=journal mode because we still zero data and
> ordering data before modifying the metadata.
>
> This change is required for iomap infrastructure conversion because the
> iomap buffered I/O path does not use the same journal infrastructure for
> partial block zeroing. The lock ordering of folio lock and starting
> transactions is "folio lock -> transaction start", which is opposite of
> the current path. Therefore, zeroing partial blocks cannot be performed
> under the active handle.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
So in this patch you also move ext4_zero_partial_blocks() before the
transaction start in ext4_zero_range() and ext4_punch_hole(). However
cannot these operations in theory fail with error such as EDQUOT or ENOSPC
when they need to grow the extent tree as a result of the operation? In
that case we'd return such error but partial blocks are already zeroed out
which is a problem... OTOH in these cases we don't need to order the data
at all so in principle we shouldn't need to move the zeroing earlier
here. Am I missing something?
Honza
> @@ -4722,25 +4724,18 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> if (IS_ALIGNED(offset | end, blocksize))
> return ret;
>
> - /*
> - * In worst case we have to writeout two nonadjacent unwritten
> - * blocks and update the inode
> - */
> - credits = (2 * ext4_ext_index_trans_blocks(inode, 2)) + 1;
> - if (ext4_should_journal_data(inode))
> - credits += 2;
> - handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
> + /* Zero out partial block at the edges of the range */
> + ret = ext4_zero_partial_blocks(inode, offset, len);
> + if (ret)
> + return ret;
> +
> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> if (IS_ERR(handle)) {
> ret = PTR_ERR(handle);
> ext4_std_error(inode->i_sb, ret);
> return ret;
> }
>
> - /* Zero out partial block at the edges of the range */
> - ret = ext4_zero_partial_blocks(inode, offset, len);
> - if (ret)
> - goto out_handle;
> -
> if (new_size)
> ext4_update_inode_size(inode, new_size);
> ret = ext4_mark_inode_dirty(handle, inode);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d5b783a7c814..5288d36b0f09 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4443,8 +4443,12 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> if (ret)
> return ret;
>
> + ret = ext4_zero_partial_blocks(inode, offset, length);
> + if (ret)
> + return ret;
> +
> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> - credits = ext4_chunk_trans_extent(inode, 2);
> + credits = ext4_chunk_trans_extent(inode, 0);
> else
> credits = ext4_blocks_for_truncate(inode);
> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
> @@ -4454,10 +4458,6 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> return ret;
> }
>
> - ret = ext4_zero_partial_blocks(inode, offset, length);
> - if (ret)
> - goto out_handle;
> -
> /* If there are blocks to remove, do it */
> start_lblk = EXT4_B_TO_LBLK(inode, offset);
> end_lblk = end >> inode->i_blkbits;
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 07/10] ext4: remove handle parameters from zero partial block functions
From: Jan Kara @ 2026-03-23 17:27 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-8-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:58, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Only journal data mode requires an active journal handle when zeroing
> partial blocks. Stop passing handle_t *handle to
> ext4_zero_partial_blocks() and related functions, and make
> ext4_block_journalled_zero_range() start a handle independently.
>
> This change has no practical impact now because all callers invoke these
> functions within the context of an active handle. It prepares for moving
> ext4_block_zero_eof() out of an active handle in the next patch, which
> is a prerequisite for converting block zero range operations to iomap
> infrastructure.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/ext4.h | 7 +++---
> fs/ext4/extents.c | 5 ++--
> fs/ext4/inode.c | 62 ++++++++++++++++++++++++++++-------------------
> 3 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index c62459ef9796..20545a9523e9 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3099,10 +3099,9 @@ extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
> extern int ext4_chunk_trans_extent(struct inode *inode, int nrblocks);
> extern int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
> int pextents);
> -extern int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> - loff_t from, loff_t end);
> -extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> - loff_t lstart, loff_t lend);
> +extern int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end);
> +extern int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart,
> + loff_t lend);
> extern vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf);
> extern qsize_t *ext4_get_reserved_space(struct inode *inode);
> extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index a265070c1b79..753a0f3418a4 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4625,8 +4625,7 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> inode_get_ctime(inode));
> if (epos > old_size) {
> pagecache_isize_extended(inode, old_size, epos);
> - ext4_block_zero_eof(handle, inode, old_size,
> - epos);
> + ext4_block_zero_eof(inode, old_size, epos);
> }
> }
> ret2 = ext4_mark_inode_dirty(handle, inode);
> @@ -4744,7 +4743,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> }
>
> /* Zero out partial block at the edges of the range */
> - ret = ext4_zero_partial_blocks(handle, inode, offset, len);
> + ret = ext4_zero_partial_blocks(inode, offset, len);
> if (ret)
> goto out_handle;
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 8fea044b3bff..d5b783a7c814 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1458,7 +1458,7 @@ static int ext4_write_end(const struct kiocb *iocb,
>
> if (old_size < pos && !verity) {
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_block_zero_eof(handle, inode, old_size, pos);
> + ext4_block_zero_eof(inode, old_size, pos);
> }
> /*
> * Don't mark the inode dirty under folio lock. First, it unnecessarily
> @@ -1576,7 +1576,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
>
> if (old_size < pos && !verity) {
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_block_zero_eof(handle, inode, old_size, pos);
> + ext4_block_zero_eof(inode, old_size, pos);
> }
>
> if (size_changed) {
> @@ -3252,7 +3252,7 @@ static int ext4_da_do_write_end(struct address_space *mapping,
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> if (zero_len)
> - ext4_block_zero_eof(handle, inode, old_size, pos);
> + ext4_block_zero_eof(inode, old_size, pos);
> ext4_mark_inode_dirty(handle, inode);
> ext4_journal_stop(handle);
>
> @@ -4102,16 +4102,23 @@ static int ext4_block_do_zero_range(struct inode *inode, loff_t from,
> return 0;
> }
>
> -static int ext4_block_journalled_zero_range(handle_t *handle,
> - struct inode *inode, loff_t from, loff_t length, bool *did_zero)
> +static int ext4_block_journalled_zero_range(struct inode *inode, loff_t from,
> + loff_t length, bool *did_zero)
> {
> struct buffer_head *bh;
> struct folio *folio;
> + handle_t *handle;
> int err;
>
> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> bh = ext4_block_get_zero_range(inode, from, length);
> - if (IS_ERR_OR_NULL(bh))
> - return PTR_ERR_OR_ZERO(bh);
> + if (IS_ERR_OR_NULL(bh)) {
> + err = PTR_ERR_OR_ZERO(bh);
> + goto out_handle;
> + }
> folio = bh->b_folio;
>
> BUFFER_TRACE(bh, "get write access");
> @@ -4132,6 +4139,8 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
> out:
> folio_unlock(folio);
> folio_put(folio);
> +out_handle:
> + ext4_journal_stop(handle);
> return err;
> }
>
> @@ -4142,7 +4151,7 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
> * the end of the block it will be shortened to end of the block
> * that corresponds to 'from'
> */
> -static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> +static int ext4_block_zero_range(struct inode *inode,
> loff_t from, loff_t length, bool *did_zero,
> bool *zero_written)
> {
> @@ -4160,8 +4169,8 @@ static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> return dax_zero_range(inode, from, length, did_zero,
> &ext4_iomap_ops);
> } else if (ext4_should_journal_data(inode)) {
> - return ext4_block_journalled_zero_range(handle, inode, from,
> - length, did_zero);
> + return ext4_block_journalled_zero_range(inode, from, length,
> + did_zero);
> }
> return ext4_block_do_zero_range(inode, from, length, did_zero,
> zero_written);
> @@ -4174,8 +4183,7 @@ static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> * to physically zero the tail end of that block so it doesn't yield old
> * data if the file is grown. Return the zeroed length on success.
> */
> -int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> - loff_t from, loff_t end)
> +int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> {
> unsigned int blocksize = i_blocksize(inode);
> unsigned int offset;
> @@ -4194,7 +4202,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> if (length > blocksize - offset)
> length = blocksize - offset;
>
> - err = ext4_block_zero_range(handle, inode, from, length,
> + err = ext4_block_zero_range(inode, from, length,
> &did_zero, &zero_written);
> if (err)
> return err;
> @@ -4206,7 +4214,14 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> */
> if (ext4_should_order_data(inode) &&
> did_zero && zero_written && !IS_DAX(inode)) {
> + handle_t *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);
> if (err)
> return err;
> }
> @@ -4214,8 +4229,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> return did_zero ? length : 0;
> }
>
> -int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> - loff_t lstart, loff_t length)
> +int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart, loff_t length)
> {
> struct super_block *sb = inode->i_sb;
> unsigned partial_start, partial_end;
> @@ -4232,21 +4246,19 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> /* Handle partial zero within the single block */
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> - err = ext4_block_zero_range(handle, inode, lstart,
> - length, NULL, NULL);
> + err = ext4_block_zero_range(inode, lstart, length, NULL, NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> - err = ext4_block_zero_range(handle, inode, lstart,
> - sb->s_blocksize, NULL, NULL);
> + err = ext4_block_zero_range(inode, lstart, sb->s_blocksize,
> + NULL, NULL);
> if (err)
> return err;
> }
> /* Handle partial zero out on the end of the range */
> if (partial_end != sb->s_blocksize - 1)
> - err = ext4_block_zero_range(handle, inode,
> - byte_end - partial_end,
> + err = ext4_block_zero_range(inode, byte_end - partial_end,
> partial_end + 1, NULL, NULL);
> return err;
> }
> @@ -4442,7 +4454,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> return ret;
> }
>
> - ret = ext4_zero_partial_blocks(handle, inode, offset, length);
> + ret = ext4_zero_partial_blocks(inode, offset, length);
> if (ret)
> goto out_handle;
>
> @@ -4592,7 +4604,7 @@ int ext4_truncate(struct inode *inode)
>
> /* Zero to the end of the block containing i_size */
> if (inode->i_size & (inode->i_sb->s_blocksize - 1))
> - ext4_block_zero_eof(handle, inode, inode->i_size, LLONG_MAX);
> + ext4_block_zero_eof(inode, inode->i_size, LLONG_MAX);
>
> /*
> * We add the inode to the orphan list, so that if this
> @@ -5951,8 +5963,8 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
> if (oldsize & (inode->i_sb->s_blocksize - 1))
> - ext4_block_zero_eof(handle, inode,
> - oldsize, LLONG_MAX);
> + ext4_block_zero_eof(inode, oldsize,
> + LLONG_MAX);
> }
>
> if (shrink)
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 08/10] ext4: pass allocate range as loff_t to ext4_alloc_file_blocks()
From: Jan Kara @ 2026-03-23 17:06 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-9-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:59, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Change ext4_alloc_file_blocks() to accept offset and len in byte
> granularity instead of block granularity. This allows callers to pass
> byte offsets and lengths directly, and this prepares for moving the
> ext4_zero_partial_blocks() call from the while(len) loop for unaligned
> append writes, where it only needs to be invoked once before doing block
> allocation.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 53 ++++++++++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 753a0f3418a4..57a686b600d9 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4542,15 +4542,15 @@ int ext4_ext_truncate(handle_t *handle, struct inode *inode)
> return err;
> }
>
> -static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> - ext4_lblk_t len, loff_t new_size,
> - int flags)
> +static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
> + loff_t new_size, int flags)
> {
> struct inode *inode = file_inode(file);
> handle_t *handle;
> int ret = 0, ret2 = 0, ret3 = 0;
> int retries = 0;
> int depth = 0;
> + ext4_lblk_t len_lblk;
> struct ext4_map_blocks map;
> unsigned int credits;
> loff_t epos, old_size = i_size_read(inode);
> @@ -4558,14 +4558,14 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> bool alloc_zero = false;
>
> BUG_ON(!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS));
> - map.m_lblk = offset;
> - map.m_len = len;
> + map.m_lblk = offset >> blkbits;
> + map.m_len = len_lblk = EXT4_MAX_BLOCKS(len, offset, blkbits);
> /*
> * Don't normalize the request if it can fit in one extent so
> * that it doesn't get unnecessarily split into multiple
> * extents.
> */
> - if (len <= EXT_UNWRITTEN_MAX_LEN)
> + if (len_lblk <= EXT_UNWRITTEN_MAX_LEN)
> flags |= EXT4_GET_BLOCKS_NO_NORMALIZE;
>
> /*
> @@ -4582,16 +4582,16 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> /*
> * credits to insert 1 extent into extent tree
> */
> - credits = ext4_chunk_trans_blocks(inode, len);
> + credits = ext4_chunk_trans_blocks(inode, len_lblk);
> depth = ext_depth(inode);
>
> retry:
> - while (len) {
> + while (len_lblk) {
> /*
> * Recalculate credits when extent tree depth changes.
> */
> if (depth != ext_depth(inode)) {
> - credits = ext4_chunk_trans_blocks(inode, len);
> + credits = ext4_chunk_trans_blocks(inode, len_lblk);
> depth = ext_depth(inode);
> }
>
> @@ -4648,7 +4648,7 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> }
>
> map.m_lblk += ret;
> - map.m_len = len = len - ret;
> + map.m_len = len_lblk = len_lblk - ret;
> }
> if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
> goto retry;
> @@ -4665,11 +4665,9 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> {
> struct inode *inode = file_inode(file);
> handle_t *handle = NULL;
> - loff_t new_size = 0;
> + loff_t align_start, align_end, new_size = 0;
> loff_t end = offset + len;
> - ext4_lblk_t start_lblk, end_lblk;
> unsigned int blocksize = i_blocksize(inode);
> - unsigned int blkbits = inode->i_blkbits;
> int ret, flags, credits;
>
> trace_ext4_zero_range(inode, offset, len, mode);
> @@ -4690,11 +4688,8 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> flags = EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT;
> /* Preallocate the range including the unaligned edges */
> if (!IS_ALIGNED(offset | end, blocksize)) {
> - ext4_lblk_t alloc_lblk = offset >> blkbits;
> - ext4_lblk_t len_lblk = EXT4_MAX_BLOCKS(len, offset, blkbits);
> -
> - ret = ext4_alloc_file_blocks(file, alloc_lblk, len_lblk,
> - new_size, flags);
> + ret = ext4_alloc_file_blocks(file, offset, len, new_size,
> + flags);
> if (ret)
> return ret;
> }
> @@ -4709,18 +4704,17 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> return ret;
>
> /* Zero range excluding the unaligned edges */
> - start_lblk = EXT4_B_TO_LBLK(inode, offset);
> - end_lblk = end >> blkbits;
> - if (end_lblk > start_lblk) {
> - ext4_lblk_t zero_blks = end_lblk - start_lblk;
> -
> + align_start = round_up(offset, blocksize);
> + align_end = round_down(end, blocksize);
> + if (align_end > align_start) {
> if (mode & FALLOC_FL_WRITE_ZEROES)
> flags = EXT4_GET_BLOCKS_CREATE_ZERO | EXT4_EX_NOCACHE;
> else
> flags |= (EXT4_GET_BLOCKS_CONVERT_UNWRITTEN |
> EXT4_EX_NOCACHE);
> - ret = ext4_alloc_file_blocks(file, start_lblk, zero_blks,
> - new_size, flags);
> + ret = ext4_alloc_file_blocks(file, align_start,
> + align_end - align_start, new_size,
> + flags);
> if (ret)
> return ret;
> }
> @@ -4768,15 +4762,11 @@ static long ext4_do_fallocate(struct file *file, loff_t offset,
> struct inode *inode = file_inode(file);
> loff_t end = offset + len;
> loff_t new_size = 0;
> - ext4_lblk_t start_lblk, len_lblk;
> int ret;
>
> trace_ext4_fallocate_enter(inode, offset, len, mode);
> WARN_ON_ONCE(!inode_is_locked(inode));
>
> - start_lblk = offset >> inode->i_blkbits;
> - len_lblk = EXT4_MAX_BLOCKS(len, offset, inode->i_blkbits);
> -
> /* We only support preallocation for extent-based files only. */
> if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
> ret = -EOPNOTSUPP;
> @@ -4791,7 +4781,7 @@ static long ext4_do_fallocate(struct file *file, loff_t offset,
> goto out;
> }
>
> - ret = ext4_alloc_file_blocks(file, start_lblk, len_lblk, new_size,
> + ret = ext4_alloc_file_blocks(file, offset, len, new_size,
> EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT);
> if (ret)
> goto out;
> @@ -4801,7 +4791,8 @@ static long ext4_do_fallocate(struct file *file, loff_t offset,
> EXT4_I(inode)->i_sync_tid);
> }
> out:
> - trace_ext4_fallocate_exit(inode, offset, len_lblk, ret);
> + trace_ext4_fallocate_exit(inode, offset,
> + EXT4_MAX_BLOCKS(len, offset, inode->i_blkbits), ret);
> return ret;
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 06/10] ext4: move ordered data handling out of ext4_block_do_zero_range()
From: Jan Kara @ 2026-03-23 16:59 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-7-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:57, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Remove the handle parameter from ext4_block_do_zero_range() and move the
> ordered data handling to ext4_block_zero_eof().
>
> This is necessary for truncate up and append writes across a range
> extending beyond EOF. The ordered data must be committed before updating
> i_disksize to prevent exposing stale on-disk data from concurrent
> post-EOF mmap writes during previous folio writeback or in case of
> system crash during append writes.
>
> This is unnecessary for partial block hole punching because the entire
> punch operation does not provide atomicity guarantees and can already
> expose intermediate results in case of crash.
Also because hole punching can only ever expose data that was there before
the punch but missed zeroing during append / truncate could expose data
that was not visible in the file before the operation.
> Since ordered data handling is no longer performed inside
> ext4_zero_partial_blocks(), ext4_punch_hole() no longer needs to attach
> jinode.
>
> This is prepared for the conversion to the iomap infrastructure, which
> does not use ordered data mode while zeroing post-EOF partial blocks.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
The patch looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 58 ++++++++++++++++++++++++-------------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 86f169df226a..8fea044b3bff 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4076,12 +4076,12 @@ static struct buffer_head *ext4_block_get_zero_range(struct inode *inode,
> return err ? ERR_PTR(err) : NULL;
> }
>
> -static int ext4_block_do_zero_range(handle_t *handle, struct inode *inode,
> - loff_t from, loff_t length, bool *did_zero)
> +static int ext4_block_do_zero_range(struct inode *inode, loff_t from,
> + loff_t length, bool *did_zero,
> + bool *zero_written)
> {
> struct buffer_head *bh;
> struct folio *folio;
> - int err = 0;
>
> bh = ext4_block_get_zero_range(inode, from, length);
> if (IS_ERR_OR_NULL(bh))
> @@ -4092,19 +4092,14 @@ static int ext4_block_do_zero_range(handle_t *handle, struct inode *inode,
> BUFFER_TRACE(bh, "zeroed end of block");
>
> mark_buffer_dirty(bh);
> - /*
> - * Only the written block requires ordered data to prevent exposing
> - * stale data.
> - */
> - if (ext4_should_order_data(inode) &&
> - !buffer_unwritten(bh) && !buffer_delay(bh))
> - err = ext4_jbd2_inode_add_write(handle, inode, from, length);
> - if (!err && did_zero)
> + if (did_zero)
> *did_zero = true;
> + if (zero_written && !buffer_unwritten(bh) && !buffer_delay(bh))
> + *zero_written = true;
>
> folio_unlock(folio);
> folio_put(folio);
> - return err;
> + return 0;
> }
>
> static int ext4_block_journalled_zero_range(handle_t *handle,
> @@ -4148,7 +4143,8 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
> * that corresponds to 'from'
> */
> static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> - loff_t from, loff_t length, bool *did_zero)
> + loff_t from, loff_t length, bool *did_zero,
> + bool *zero_written)
> {
> unsigned blocksize = inode->i_sb->s_blocksize;
> unsigned int max = blocksize - (from & (blocksize - 1));
> @@ -4167,7 +4163,8 @@ static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> return ext4_block_journalled_zero_range(handle, inode, from,
> length, did_zero);
> }
> - return ext4_block_do_zero_range(handle, inode, from, length, did_zero);
> + return ext4_block_do_zero_range(inode, from, length, did_zero,
> + zero_written);
> }
>
> /*
> @@ -4184,6 +4181,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> unsigned int offset;
> loff_t length = end - from;
> bool did_zero = false;
> + bool zero_written = false;
> int err;
>
> offset = from & (blocksize - 1);
> @@ -4196,9 +4194,22 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> if (length > blocksize - offset)
> length = blocksize - offset;
>
> - err = ext4_block_zero_range(handle, inode, from, length, &did_zero);
> + err = ext4_block_zero_range(handle, inode, from, length,
> + &did_zero, &zero_written);
> if (err)
> return err;
> + /*
> + * It's necessary to order zeroed data before update i_disksize when
> + * 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.
> + */
> + if (ext4_should_order_data(inode) &&
> + did_zero && zero_written && !IS_DAX(inode)) {
> + err = ext4_jbd2_inode_add_write(handle, inode, from, length);
> + if (err)
> + return err;
> + }
>
> return did_zero ? length : 0;
> }
> @@ -4222,13 +4233,13 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> err = ext4_block_zero_range(handle, inode, lstart,
> - length, NULL);
> + length, NULL, NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> err = ext4_block_zero_range(handle, inode, lstart,
> - sb->s_blocksize, NULL);
> + sb->s_blocksize, NULL, NULL);
> if (err)
> return err;
> }
> @@ -4236,7 +4247,7 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> if (partial_end != sb->s_blocksize - 1)
> err = ext4_block_zero_range(handle, inode,
> byte_end - partial_end,
> - partial_end + 1, NULL);
> + partial_end + 1, NULL, NULL);
> return err;
> }
>
> @@ -4411,17 +4422,6 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> end = max_end;
> length = end - offset;
>
> - /*
> - * Attach jinode to inode for jbd2 if we do any zeroing of partial
> - * block.
> - */
> - if (!IS_ALIGNED(offset | end, sb->s_blocksize)) {
> - ret = ext4_inode_attach_jinode(inode);
> - if (ret < 0)
> - return ret;
> - }
> -
> -
> ret = ext4_update_disksize_before_punch(inode, offset, length);
> if (ret)
> return ret;
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 05/10] ext4: rename ext4_block_zero_page_range() to ext4_block_zero_range()
From: Jan Kara @ 2026-03-23 16:49 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260310014101.4140698-6-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:56, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Rename ext4_block_zero_page_range() to ext4_block_zero_range() since the
> "page" naming is no longer appropriate for current context. Also change
> its signature to take an inode pointer instead of an address_space. This
> aligns with the caller ext4_block_zero_eof() and
> ext4_zero_partial_blocks().
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d63d455831b9..86f169df226a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4147,11 +4147,9 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
> * the end of the block it will be shortened to end of the block
> * that corresponds to 'from'
> */
> -static int ext4_block_zero_page_range(handle_t *handle,
> - struct address_space *mapping, loff_t from, loff_t length,
> - bool *did_zero)
> +static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> + loff_t from, loff_t length, bool *did_zero)
> {
> - struct inode *inode = mapping->host;
> unsigned blocksize = inode->i_sb->s_blocksize;
> unsigned int max = blocksize - (from & (blocksize - 1));
>
> @@ -4198,8 +4196,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> if (length > blocksize - offset)
> length = blocksize - offset;
>
> - err = ext4_block_zero_page_range(handle, inode->i_mapping, from, length,
> - &did_zero);
> + err = ext4_block_zero_range(handle, inode, from, length, &did_zero);
> if (err)
> return err;
>
> @@ -4210,7 +4207,6 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> loff_t lstart, loff_t length)
> {
> struct super_block *sb = inode->i_sb;
> - struct address_space *mapping = inode->i_mapping;
> unsigned partial_start, partial_end;
> ext4_fsblk_t start, end;
> loff_t byte_end = (lstart + length - 1);
> @@ -4225,22 +4221,22 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> /* Handle partial zero within the single block */
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> - err = ext4_block_zero_page_range(handle, mapping,
> - lstart, length, NULL);
> + err = ext4_block_zero_range(handle, inode, lstart,
> + length, NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> - err = ext4_block_zero_page_range(handle, mapping, lstart,
> - sb->s_blocksize, NULL);
> + err = ext4_block_zero_range(handle, inode, lstart,
> + sb->s_blocksize, NULL);
> if (err)
> return err;
> }
> /* Handle partial zero out on the end of the range */
> if (partial_end != sb->s_blocksize - 1)
> - err = ext4_block_zero_page_range(handle, mapping,
> - byte_end - partial_end,
> - partial_end + 1, NULL);
> + err = ext4_block_zero_range(handle, inode,
> + byte_end - partial_end,
> + partial_end + 1, NULL);
> return err;
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox