On 2025/5/20 4:24, Jan Kara wrote: > On Mon 12-05-25 14:33:16, 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> > > One comment below > >> 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; >> } >> 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; > > I don't think you can drop 'pextents' from this computation... Yes, you now > account possible number of modified extent tree leaf blocks in > ext4_index_trans_blocks() but additionally, each extent separately may be > allocated from a different group and thus need to update different bitmap > and group descriptor block. That is separate from the computation you do in > ext4_index_trans_blocks() AFAICT... > Yes, that's right! Sorry for my mistake. I will fix this. Thanks, Yi. > >> 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); >> -- >> 2.46.1 >>
