From: Jan Kara <jack@suse.cz>
To: 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, jack@suse.cz, ojaswin@linux.ibm.com,
ritesh.list@gmail.com, libaokun@linux.alibaba.com,
yi.zhang@huawei.com, yizhang089@gmail.com, yangerkun@huawei.com,
yukuai@fnnas.com
Subject: Re: [PATCH 07/10] ext4: remove handle parameters from zero partial block functions
Date: Mon, 23 Mar 2026 18:27:35 +0100 [thread overview]
Message-ID: <kajhuay7hptzag6sg4476272lbo4yy4l3c5pi6ve5izfygdnuh@gjjshtoowngy> (raw)
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
next prev parent reply other threads:[~2026-03-23 17:27 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-10 1:40 [PATCH 00/10] ext4: refactor partial block zero-out for iomap conversion Zhang Yi
2026-03-10 1:40 ` [PATCH 01/10] ext4: add did_zero output parameter to ext4_block_zero_page_range() Zhang Yi
2026-03-23 16:33 ` Jan Kara
2026-03-10 1:40 ` [PATCH 02/10] ext4: ext4_block_truncate_page() returns zeroed length on success Zhang Yi
2026-03-23 16:34 ` Jan Kara
2026-03-10 1:40 ` [PATCH 03/10] ext4: rename and extend ext4_block_truncate_page() Zhang Yi
2026-03-23 16:42 ` Jan Kara
2026-03-10 1:40 ` [PATCH 04/10] ext4: factor out journalled block zeroing range Zhang Yi
2026-03-23 16:48 ` Jan Kara
2026-03-24 3:16 ` Zhang Yi
2026-03-10 1:40 ` [PATCH 05/10] ext4: rename ext4_block_zero_page_range() to ext4_block_zero_range() Zhang Yi
2026-03-23 16:49 ` Jan Kara
2026-03-10 1:40 ` [PATCH 06/10] ext4: move ordered data handling out of ext4_block_do_zero_range() Zhang Yi
2026-03-23 16:59 ` Jan Kara
2026-03-24 3:17 ` Zhang Yi
2026-03-10 1:40 ` [PATCH 07/10] ext4: remove handle parameters from zero partial block functions Zhang Yi
2026-03-23 17:27 ` Jan Kara [this message]
2026-03-10 1:40 ` [PATCH 08/10] ext4: pass allocate range as loff_t to ext4_alloc_file_blocks() Zhang Yi
2026-03-23 17:06 ` Jan Kara
2026-03-10 1:41 ` [PATCH 09/10] ext4: move zero partial block range functions out of active handle Zhang Yi
2026-03-23 20:17 ` Jan Kara
2026-03-24 3:10 ` Zhang Yi
2026-03-24 3:14 ` Zhang Yi
2026-03-24 13:56 ` Jan Kara
2026-03-10 1:41 ` [PATCH 10/10] ext4: zero post-EOF partial block before appending write Zhang Yi
2026-03-23 20:31 ` Jan Kara
2026-03-24 3:29 ` 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=kajhuay7hptzag6sg4476272lbo4yy4l3c5pi6ve5izfygdnuh@gjjshtoowngy \
--to=jack@suse.cz \
--cc=adilger.kernel@dilger.ca \
--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=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yi.zhang@huaweicloud.com \
--cc=yizhang089@gmail.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