Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents

On 8/5/2026 11:35 PM, Jan Kara wrote:
> So far ext4_meta_trans_blocks() expects that each extent counted in
> @pextents will be allocated in the transaction we estimate credits for.
> This is correct for the use in ext4_chunk_trans_blocks() and
> ext4_chunk_trans_extent() however the use in atomic write path
> (ext4_convert_unwritten_extents_atomic() and ext4_iomap_alloc() for
> IOMAP_ATOMIC) unnecessarily overestimates the number of necessary
> credits as neither of them allocates any data. Add argument to
> ext4_meta_trans_blocks() for number of extents that are going to be
> allocated in the transaction.
> 
> Signed-off-by: Jan Kara <[email protected]>

Hi Jan,

I have some questions below.

> ---
>  fs/ext4/ext4.h    |  2 +-
>  fs/ext4/extents.c |  2 +-
>  fs/ext4/inode.c   | 32 +++++++++++++++++---------------
>  3 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b37c136ea3ab..6e0cc9b845ae 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3184,7 +3184,7 @@ 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);
> +				  int pextents, int alloc_extents);
>  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 length, bool *did_zero);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 91c97af64b31..44ab246a3176 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4976,7 +4976,7 @@ int ext4_convert_unwritten_extents_atomic(handle_t *handle, struct inode *inode,
>  		 * it can tell if the extent in the cache is a split extent.
>  		 * But for now let's assume pextents as 2 always.
>  		 */
> -		credits = ext4_meta_trans_blocks(inode, max_blocks, 2);
> +		credits = ext4_meta_trans_blocks(inode, max_blocks, 2, 0);
>  	}
>  
>  	if (credits) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ce99807c5f5b..f324a54f1dae 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3693,9 +3693,11 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
>  		if (ret < 0)
>  			return ret;
>  		if (map->m_len < orig_mlen) {
> +			int bpg = EXT4_BLOCKS_PER_GROUP(inode->i_sb);
> +
>  			map->m_len = orig_mlen;
> -			dio_credits = ext4_meta_trans_blocks(inode, orig_mlen,
> -							     map->m_len);
> +			dio_credits = ext4_meta_trans_blocks(inode, map->m_len,
> +							     map->m_len, 0);

This pertains to the mixed map case, where holes may exist in between.
My understanding is that we cannot assume all blocks are already
allocated. Is this right?

>  		} else {
>  			dio_credits = ext4_chunk_trans_blocks(inode,
>  							      map->m_len);
> @@ -6307,17 +6309,17 @@ static int ext4_index_trans_blocks(struct inode *inode, int lblocks,
>  }
>  
>  /*
> - * Account for index blocks, block groups bitmaps and block group
> - * descriptor blocks if modify datablocks and index blocks
> - * worse case, the indexs blocks spread over different block groups
> - *
> - * If datablocks are discontiguous, they are possible to spread over
> - * different block groups too. If they are contiguous, with flexbg,
> - * they could still across block group boundary.
> - *
> - * Also account for superblock, inode, quota and xattr blocks
> + * Calculate number of credits needed in a transaction to:
> + *   * Allocate data blocks from @alloc_extents different groups - note that
> + *     with flexbg a single physical extent can span multiple groups but
> + *     single mballoc request only returns extent within one group.
> + *   * Allocate metatadata (extent tree blocks, indirect blocks) to store
> + *     pointers to @pextents data extents having @lblocks in total.
> + *   * Modify extent tree / indirect block tree, inode, superblock, quota
> + *     tracking, xattr blocks
>   */
> -int ext4_meta_trans_blocks(struct inode *inode, int lblocks, int pextents)
> +int ext4_meta_trans_blocks(struct inode *inode, int lblocks, int pextents,
> +			   int alloc_extents)

From what I can see, alloc_extents currently can only be 0 or equal to
pextents — 0 means a pure conversion, and equal to pextents means a new
allocation. pextents is supposed to cover the total count of
alloc_extents. Should we add some sanity checks to guard against invalid
inputs, like alloc_extents > pextents?

Thanks,
Yi.

>  {
>  	ext4_group_t groups, ngroups = ext4_get_groups_count(inode->i_sb);
>  	int gdpblocks;
> @@ -6334,7 +6336,7 @@ int ext4_meta_trans_blocks(struct inode *inode, int lblocks, int pextents)
>  	 * Now let's see how many group bitmaps and group descriptors need
>  	 * to account
>  	 */
> -	groups = idxblocks + pextents;
> +	groups = idxblocks + alloc_extents;
>  	gdpblocks = groups;
>  	if (groups > ngroups)
>  		groups = ngroups;
> @@ -6360,7 +6362,7 @@ int ext4_chunk_trans_extent(struct inode *inode, int nrblocks)
>  {
>  	int ret;
>  
> -	ret = ext4_meta_trans_blocks(inode, nrblocks, 1);
> +	ret = ext4_meta_trans_blocks(inode, nrblocks, 1, 1);
>  	/* Account for data blocks for journalled mode */
>  	if (ext4_should_journal_data(inode))
>  		ret += nrblocks;
> @@ -6378,7 +6380,7 @@ int ext4_chunk_trans_extent(struct inode *inode, int nrblocks)
>   */
>  int ext4_chunk_trans_blocks(struct inode *inode, int nrblocks)
>  {
> -	return ext4_meta_trans_blocks(inode, nrblocks, 1);
> +	return ext4_meta_trans_blocks(inode, nrblocks, 1, 1);
>  }
>  
>  /*