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

On Wed, Aug 05, 2026 at 05:35:47PM +0200, 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,

Looks mostly okay, feel free to add:

Reviewed-by: Ojaswin Mujoo <[email protected]>

just a couple comments 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);

Right, because we are not going to allocate anything but we might be
modifying upto 2 extent entries. By that logic shall we also change the
following:

  @@ -5046,7 +5046,7 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
   		/*
   		 * credits to insert 1 extent into extent tree
   		 */
  -		credits = ext4_chunk_trans_blocks(inode, max_blocks);
  +		credits = ext4_meta_trans_blocks(inode, max_blocks, max_blocks, 0);
   	}
   	while (ret >= 0 && ret < max_blocks) {
   		map.m_lblk += ret;

since we are not going to allocate anything.

>  	}
>  
>  	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 makes sense because we always come here in the bigalloc case when
the cluster is already allocated. If we ever support atomic writes
without bigalloc, we'll have to remember to take care of this. Maybe we
can add a small one line comment here?

/* 
 * we have an encountered mixed mapping in an already allocated bigalloc
 * cluster hence credits for allocation are not needed
 */


>  		} 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)
>  {
>  	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);
>  }
>  
>  /*
> -- 
> 2.51.0
>