Re: [PATCH 09/10] ext4: move zero partial block range functions out of active handle

[Jan Kara <jack@suse.cz>]

On Tue 24-03-26 11:10:13, 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.

Yes, I was referring to those.

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

Indeed, I've got confused. Your patch is fine. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR