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

On Fri, Aug 07, 2026 at 10:59:17AM +0800, Zhang Yi wrote:
> 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?

Hi Yi,

With atomic writes, we can only allow multi block writes when bigalloc
is set. So the common cases are that the whole bigalloc cluster is
present or whole is missing.

If we do see map->m_len < orig_mlen that means the bigalloc cluster
range we queried has some allocation. This can only happen if someone
punched through the cluster because punch-hole can still happen at block
granularity.

Regardless, if we do see a mixed map here, it means the cluster is
allocated already, just a few blocks are punched in it, hence we won't
need to do any allocation.

Regards,
ojaswin

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