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... Honza > 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 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
