Hi Ted.
On 2025/5/12 14:33, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> The journal credits calculation in ext4_ext_index_trans_blocks() is
> currently inadequate. It only multiplies the depth of the extents tree
> and doesn't account for the blocks that may be required for adding the
> leaf extents themselves.
>
> After enabling large folios, we can easily run out of handle credits,
> triggering a warning in jbd2_journal_dirty_metadata() on filesystems
> with a 1KB block size. This occurs because we may need more extents when
> iterating through each large folio in
> ext4_do_writepages()->mpage_map_and_submit_extent(). Therefore, we
> should modify ext4_ext_index_trans_blocks() to include a count of the
> leaf extents in the worst case as well.
>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
> fs/ext4/extents.c | 5 +++--
> fs/ext4/inode.c | 10 ++++------
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index c616a16a9f36..e759941bd262 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2405,9 +2405,10 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents)
> depth = ext_depth(inode);
>
> if (extents <= 1)
> - index = depth * 2;
> + index = depth * 2 + extents;
> else
> - index = depth * 3;
> + index = depth * 3 +
> + DIV_ROUND_UP(extents, ext4_ext_space_block(inode, 0));
>
> return index;
> }
This patch conflicts with Jan's patch e18d4f11d240 ("ext4: fix
calculation of credits for extent tree modification") in
ext4_ext_index_trans_blocks(), the conflict should be resolved when
merging this patch. However, I checked the merged commit of this patch
in your dev branch[1], and the changes in ext4_ext_index_trans_blocks()
seem to be incorrect, which could result in insufficient credit
reservations on 1K block size filesystems.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?h=dev&id=d80af138eb8873eb13f5fece1adabb3ca4325134
I think the correct conflict resolution in ext4_ext_index_trans_blocks()
should be:
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 9053fe68ee4c..431d66181721 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2409,9 +2409,10 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents)
* the time we actually modify the tree. Assume the worst case.
*/
if (extents <= 1)
- index = EXT4_MAX_EXTENT_DEPTH * 2;
+ index = EXT4_MAX_EXTENT_DEPTH * 2 + extents;
else
- index = EXT4_MAX_EXTENT_DEPTH * 3;
+ index = EXT4_MAX_EXTENT_DEPTH * 3 +
+ DIV_ROUND_UP(extents, ext4_ext_space_block(inode, 0));
return index;
Best Regards,
Yi.
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ffbf444b56d4..3e962a760d71 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5792,18 +5792,16 @@ static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
> int ret;
>
> /*
> - * How many index blocks need to touch to map @lblocks logical blocks
> - * to @pextents physical extents?
> + * How many index and lead blocks need to touch to map @lblocks
> + * logical blocks to @pextents physical extents?
> */
> idxblocks = ext4_index_trans_blocks(inode, lblocks, pextents);
>
> - ret = idxblocks;
> -
> /*
> * Now let's see how many group bitmaps and group descriptors need
> * to account
> */
> - groups = idxblocks + pextents;
> + groups = idxblocks;
> gdpblocks = groups;
> if (groups > ngroups)
> groups = ngroups;
> @@ -5811,7 +5809,7 @@ static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
> gdpblocks = EXT4_SB(inode->i_sb)->s_gdb_count;
>
> /* bitmaps and block group descriptor blocks */
> - ret += groups + gdpblocks;
> + ret = idxblocks + groups + gdpblocks;
>
> /* Blocks for super block, inode, quota and xattr blocks */
> ret += EXT4_META_TRANS_BLOCKS(inode->i_sb);
