[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bq37dhlk3uxrwmejrhqkvjthlq7j3fac7dggkchpdzx7m223jl@njfevshlrkgh>
Date: Wed, 2 Jul 2025 16:50:21 +0200
From: Jan Kara <jack@...e.cz>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
ojaswin@...ux.ibm.com, sashal@...nel.org, yi.zhang@...wei.com, libaokun1@...wei.com,
yukuai3@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v3 09/10] ext4: replace ext4_writepage_trans_blocks()
On Tue 01-07-25 21:06:34, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@...wei.com>
>
> After ext4 supports large folios, the semantics of reserving credits in
> pages is no longer applicable. In most scenarios, reserving credits in
> extents is sufficient. Therefore, introduce ext4_chunk_trans_extent()
> to replace ext4_writepage_trans_blocks(). move_extent_per_page() is the
> only remaining location where we are still processing extents in pages.
>
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/extents.c | 6 +++---
> fs/ext4/inline.c | 6 +++---
> fs/ext4/inode.c | 33 +++++++++++++++------------------
> fs/ext4/move_extent.c | 3 ++-
> fs/ext4/xattr.c | 2 +-
> 6 files changed, 25 insertions(+), 27 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 18373de980f2..f705046ba6c6 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3064,9 +3064,9 @@ extern int ext4_punch_hole(struct file *file, loff_t offset, loff_t length);
> extern void ext4_set_inode_flags(struct inode *, bool init);
> extern int ext4_alloc_da_blocks(struct inode *inode);
> extern void ext4_set_aops(struct inode *inode);
> -extern int ext4_writepage_trans_blocks(struct inode *);
> extern int ext4_normal_submit_inode_data_buffers(struct jbd2_inode *jinode);
> 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_zero_partial_blocks(handle_t *handle, struct inode *inode,
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b543a46fc809..f0f155458697 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5171,7 +5171,7 @@ ext4_ext_shift_path_extents(struct ext4_ext_path *path, ext4_lblk_t shift,
> credits = depth + 2;
> }
>
> - restart_credits = ext4_writepage_trans_blocks(inode);
> + restart_credits = ext4_chunk_trans_extent(inode, 0);
> err = ext4_datasem_ensure_credits(handle, inode, credits,
> restart_credits, 0);
> if (err) {
> @@ -5431,7 +5431,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
>
> truncate_pagecache(inode, start);
>
> - credits = ext4_writepage_trans_blocks(inode);
> + credits = ext4_chunk_trans_extent(inode, 0);
> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> @@ -5527,7 +5527,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
>
> truncate_pagecache(inode, start);
>
> - credits = ext4_writepage_trans_blocks(inode);
> + credits = ext4_chunk_trans_extent(inode, 0);
> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index a1bbcdf40824..d5b32d242495 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -562,7 +562,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
> return 0;
> }
>
> - needed_blocks = ext4_writepage_trans_blocks(inode);
> + needed_blocks = ext4_chunk_trans_extent(inode, 1);
>
> ret = ext4_get_inode_loc(inode, &iloc);
> if (ret)
> @@ -1864,7 +1864,7 @@ int ext4_inline_data_truncate(struct inode *inode, int *has_inline)
> };
>
>
> - needed_blocks = ext4_writepage_trans_blocks(inode);
> + needed_blocks = ext4_chunk_trans_extent(inode, 1);
> handle = ext4_journal_start(inode, EXT4_HT_INODE, needed_blocks);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> @@ -1979,7 +1979,7 @@ int ext4_convert_inline_data(struct inode *inode)
> return 0;
> }
>
> - needed_blocks = ext4_writepage_trans_blocks(inode);
> + needed_blocks = ext4_chunk_trans_extent(inode, 1);
>
> iloc.bh = NULL;
> error = ext4_get_inode_loc(inode, &iloc);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ceaede80d791..572a70b6a934 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1295,7 +1295,8 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
> * Reserve one block more for addition to orphan list in case
> * we allocate blocks but write fails for some reason
> */
> - needed_blocks = ext4_writepage_trans_blocks(inode) + 1;
> + needed_blocks = ext4_chunk_trans_extent(inode,
> + ext4_journal_blocks_per_folio(inode)) + 1;
> index = pos >> PAGE_SHIFT;
>
> if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
> @@ -4462,7 +4463,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> return ret;
>
> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> - credits = ext4_writepage_trans_blocks(inode);
> + credits = ext4_chunk_trans_extent(inode, 2);
> else
> credits = ext4_blocks_for_truncate(inode);
> handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
> @@ -4611,7 +4612,7 @@ int ext4_truncate(struct inode *inode)
> }
>
> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> - credits = ext4_writepage_trans_blocks(inode);
> + credits = ext4_chunk_trans_extent(inode, 1);
> else
> credits = ext4_blocks_for_truncate(inode);
>
> @@ -6238,25 +6239,19 @@ int ext4_meta_trans_blocks(struct inode *inode, int lblocks, int pextents)
> }
>
> /*
> - * Calculate the total number of credits to reserve to fit
> - * the modification of a single pages into a single transaction,
> - * which may include multiple chunks of block allocations.
> - *
> - * This could be called via ext4_write_begin()
> - *
> - * We need to consider the worse case, when
> - * one new block per extent.
> + * Calculate the journal credits for modifying the number of blocks
> + * in a single extent within one transaction. 'nrblocks' is used only
> + * for non-extent inodes. For extent type inodes, 'nrblocks' can be
> + * zero if the exact number of blocks is unknown.
> */
> -int ext4_writepage_trans_blocks(struct inode *inode)
> +int ext4_chunk_trans_extent(struct inode *inode, int nrblocks)
> {
> - int bpp = ext4_journal_blocks_per_folio(inode);
> int ret;
>
> - ret = ext4_meta_trans_blocks(inode, bpp, bpp);
> -
> + ret = ext4_meta_trans_blocks(inode, nrblocks, 1);
> /* Account for data blocks for journalled mode */
> if (ext4_should_journal_data(inode))
> - ret += bpp;
> + ret += nrblocks;
> return ret;
> }
>
> @@ -6634,10 +6629,12 @@ static int ext4_block_page_mkwrite(struct inode *inode, struct folio *folio,
> handle_t *handle;
> loff_t size;
> unsigned long len;
> + int credits;
> int ret;
>
> - handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE,
> - ext4_writepage_trans_blocks(inode));
> + credits = ext4_chunk_trans_extent(inode,
> + ext4_journal_blocks_per_folio(inode));
> + handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, credits);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 1f8493a56e8f..adae3caf175a 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -280,7 +280,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
> */
> again:
> *err = 0;
> - jblocks = ext4_writepage_trans_blocks(orig_inode) * 2;
> + jblocks = ext4_meta_trans_blocks(orig_inode, block_len_in_page,
> + block_len_in_page) * 2;
> handle = ext4_journal_start(orig_inode, EXT4_HT_MOVE_EXTENTS, jblocks);
> if (IS_ERR(handle)) {
> *err = PTR_ERR(handle);
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 8d15acbacc20..3fb93247330d 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -962,7 +962,7 @@ int __ext4_xattr_set_credits(struct super_block *sb, struct inode *inode,
> * so we need to reserve credits for this eventuality
> */
> if (inode && ext4_has_inline_data(inode))
> - credits += ext4_writepage_trans_blocks(inode) + 1;
> + credits += ext4_chunk_trans_extent(inode, 1) + 1;
>
> /* We are done if ea_inode feature is not enabled. */
> if (!ext4_has_feature_ea_inode(sb))
> --
> 2.46.1
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists