* [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications
@ 2026-08-05 15:35 Jan Kara
2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Jan Kara @ 2026-08-05 15:35 UTC (permalink / raw)
To: Ted Tso
Cc: linux-ext4, Baokun Li, Zhang Yi, Ojaswin Mujoo, Ritesh Harjani,
Jan Kara
Hello!
This patch set fixes credit estimates for extent tree modification during
convertion of unwritten extents on writeback completion. Otherwise we are
occasionally seeing assertion failures during extent conversion in end io
completion callback due to insufficient transation credits.
Honza
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-05 15:35 [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Jan Kara @ 2026-08-05 15:35 ` Jan Kara 2026-08-06 18:09 ` Ojaswin Mujoo 2026-08-07 2:59 ` Zhang Yi 2026-08-05 15:35 ` [PATCH 2/3] ext4: Fix transaction overflow during writeback Jan Kara ` (2 subsequent siblings) 3 siblings, 2 replies; 14+ messages in thread From: Jan Kara @ 2026-08-05 15:35 UTC (permalink / raw) To: Ted Tso Cc: linux-ext4, Baokun Li, Zhang Yi, Ojaswin Mujoo, Ritesh Harjani, Jan Kara 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 <jack@suse.cz> --- 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); } 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara @ 2026-08-06 18:09 ` Ojaswin Mujoo 2026-08-07 2:59 ` Zhang Yi 1 sibling, 0 replies; 14+ messages in thread From: Ojaswin Mujoo @ 2026-08-06 18:09 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Zhang Yi, Ritesh Harjani 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 <jack@suse.cz> Hi Jan, Looks mostly okay, feel free to add: Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara 2026-08-06 18:09 ` Ojaswin Mujoo @ 2026-08-07 2:59 ` Zhang Yi 2026-08-07 5:59 ` Ojaswin Mujoo 1 sibling, 1 reply; 14+ messages in thread From: Zhang Yi @ 2026-08-07 2:59 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Ojaswin Mujoo, Ritesh Harjani 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 <jack@suse.cz> 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); > } > > /* ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-07 2:59 ` Zhang Yi @ 2026-08-07 5:59 ` Ojaswin Mujoo 2026-08-07 7:05 ` Zhang Yi 0 siblings, 1 reply; 14+ messages in thread From: Ojaswin Mujoo @ 2026-08-07 5:59 UTC (permalink / raw) To: Zhang Yi; +Cc: Jan Kara, Ted Tso, linux-ext4, Baokun Li, Ritesh Harjani 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 <jack@suse.cz> > > 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); > > } > > > > /* > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-07 5:59 ` Ojaswin Mujoo @ 2026-08-07 7:05 ` Zhang Yi 2026-08-07 7:28 ` Ojaswin Mujoo 0 siblings, 1 reply; 14+ messages in thread From: Zhang Yi @ 2026-08-07 7:05 UTC (permalink / raw) To: Ojaswin Mujoo; +Cc: Jan Kara, Ted Tso, linux-ext4, Baokun Li, Ritesh Harjani On 8/7/2026 1:59 PM, Ojaswin Mujoo wrote: > 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 <jack@suse.cz> >> >> 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 Hi, ojaswin, Thank you for the clarification! My initial concern wasn't about the new cluster allocation, but rather the possibility of a new extent being allocated in this cluster. Which means the hole-covered portion might require a new extent. But after looking at ext4_map_blocks_atomic_write_slow(), I see that it guarantees both the old and new blocks in this cluster are written type, so the new range ought to merge into the existing extent. Furthermore, ext4_ext_insert_extent() does not perform merging across leaf extent blocks, so the extent count remains unchanged — no new extent allocation should happen. But this point is indeed subtle, as you suggested, adding a comment would be helpful. Thanks, Yi. > >> >>> } 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); >>> } >>> >>> /* >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents 2026-08-07 7:05 ` Zhang Yi @ 2026-08-07 7:28 ` Ojaswin Mujoo 0 siblings, 0 replies; 14+ messages in thread From: Ojaswin Mujoo @ 2026-08-07 7:28 UTC (permalink / raw) To: Zhang Yi; +Cc: Jan Kara, Ted Tso, linux-ext4, Baokun Li, Ritesh Harjani On Fri, Aug 07, 2026 at 03:05:09PM +0800, Zhang Yi wrote: > On 8/7/2026 1:59 PM, Ojaswin Mujoo wrote: > > 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 <jack@suse.cz> > >> > >> 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 > > Hi, ojaswin, > > Thank you for the clarification! > > My initial concern wasn't about the new cluster allocation, but rather > the possibility of a new extent being allocated in this cluster. Which > means the hole-covered portion might require a new extent. > > But after looking at ext4_map_blocks_atomic_write_slow(), I see that it > guarantees both the old and new blocks in this cluster are written type, > so the new range ought to merge into the existing extent. Furthermore, > ext4_ext_insert_extent() does not perform merging across leaf extent > blocks, so the extent count remains unchanged — no new extent > allocation should happen. But this point is indeed subtle, as you > suggested, adding a comment would be helpful. Yes that's correct, we will keep merging into the existing extents so there will be no new allocation (data or metadata) needed. Regards, ojaswin > > Thanks, > Yi. > > > > >> > >>> } 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); > >>> } > >>> > >>> /* > >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] ext4: Fix transaction overflow during writeback 2026-08-05 15:35 [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Jan Kara 2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara @ 2026-08-05 15:35 ` Jan Kara 2026-08-07 3:36 ` Zhang Yi 2026-08-07 8:40 ` Ojaswin Mujoo 2026-08-05 15:35 ` [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() Jan Kara 2026-08-10 1:33 ` [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Theodore Ts'o 3 siblings, 2 replies; 14+ messages in thread From: Jan Kara @ 2026-08-05 15:35 UTC (permalink / raw) To: Ted Tso Cc: linux-ext4, Baokun Li, Zhang Yi, Ojaswin Mujoo, Ritesh Harjani, Jan Kara Commit 95ad8ee45cdb ("ext4: correct the reserved credits for extent conversion") was correct to note that we need to reserve enough credits for all extents possibly underlying a large folio. However it was too eager to reduce the number of reserved credits. Extent conversion may not only need to touch several leaf extent blocks, it may also need to split extents - for example a single large unwritten extent may need to be split into many small written ones in case of sparse folio dirtying. This can thus result not only in extent leaf modifications but also in a need to allocate new extent tree nodes. As a result the reserved transaction credits were not sufficient in some corner cases. Use ext4_meta_trans_blocks() for correct upper bound credit estimate. Fixes: 95ad8ee45cdb ("ext4: correct the reserved credits for extent conversion") Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/inode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index f324a54f1dae..ddce319e53c5 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2852,10 +2852,10 @@ static int ext4_do_writepages(struct mpage_da_data *mpd) if (ext4_should_dioread_nolock(inode)) { int bpf = ext4_journal_blocks_per_folio(inode); /* - * We may need to convert up to one extent per block in - * the folio and we may dirty the inode. + * We may need to convert up to one extent per block in the + * folio. */ - rsv_blocks = 1 + ext4_ext_index_trans_blocks(inode, bpf); + rsv_blocks = ext4_meta_trans_blocks(inode, bpf, bpf, 0); } if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX) -- 2.51.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ext4: Fix transaction overflow during writeback 2026-08-05 15:35 ` [PATCH 2/3] ext4: Fix transaction overflow during writeback Jan Kara @ 2026-08-07 3:36 ` Zhang Yi 2026-08-07 8:40 ` Ojaswin Mujoo 1 sibling, 0 replies; 14+ messages in thread From: Zhang Yi @ 2026-08-07 3:36 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Ojaswin Mujoo, Ritesh Harjani On 8/5/2026 11:35 PM, Jan Kara wrote: > Commit 95ad8ee45cdb ("ext4: correct the reserved credits for extent > conversion") was correct to note that we need to reserve enough credits > for all extents possibly underlying a large folio. However it was too > eager to reduce the number of reserved credits. Extent conversion may > not only need to touch several leaf extent blocks, it may also need to > split extents - for example a single large unwritten extent may need to > be split into many small written ones in case of sparse folio dirtying. > This can thus result not only in extent leaf modifications but also in a > need to allocate new extent tree nodes. As a result the reserved > transaction credits were not sufficient in some corner cases. Use > ext4_meta_trans_blocks() for correct upper bound credit estimate. > > Fixes: 95ad8ee45cdb ("ext4: correct the reserved credits for extent conversion") > Signed-off-by: Jan Kara <jack@suse.cz> Indeed, I overlooked that the newly allocated index blocks will modify bitmaps and group descriptor blocks. Given that the folio length is capped at 2048 filesystem blocks, calling ext4_meta_trans_blocks(bpf, 0) shouldn't result in an overly large credits calculation. So, Reviewed-by: Zhang Yi <yi.zhang@huawei.com> > --- > fs/ext4/inode.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index f324a54f1dae..ddce319e53c5 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -2852,10 +2852,10 @@ static int ext4_do_writepages(struct mpage_da_data *mpd) > if (ext4_should_dioread_nolock(inode)) { > int bpf = ext4_journal_blocks_per_folio(inode); > /* > - * We may need to convert up to one extent per block in > - * the folio and we may dirty the inode. > + * We may need to convert up to one extent per block in the > + * folio. > */ > - rsv_blocks = 1 + ext4_ext_index_trans_blocks(inode, bpf); > + rsv_blocks = ext4_meta_trans_blocks(inode, bpf, bpf, 0); > } > > if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ext4: Fix transaction overflow during writeback 2026-08-05 15:35 ` [PATCH 2/3] ext4: Fix transaction overflow during writeback Jan Kara 2026-08-07 3:36 ` Zhang Yi @ 2026-08-07 8:40 ` Ojaswin Mujoo 1 sibling, 0 replies; 14+ messages in thread From: Ojaswin Mujoo @ 2026-08-07 8:40 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Zhang Yi, Ritesh Harjani On Wed, Aug 05, 2026 at 05:35:48PM +0200, Jan Kara wrote: > Commit 95ad8ee45cdb ("ext4: correct the reserved credits for extent > conversion") was correct to note that we need to reserve enough credits > for all extents possibly underlying a large folio. However it was too > eager to reduce the number of reserved credits. Extent conversion may > not only need to touch several leaf extent blocks, it may also need to > split extents - for example a single large unwritten extent may need to > be split into many small written ones in case of sparse folio dirtying. > This can thus result not only in extent leaf modifications but also in a > need to allocate new extent tree nodes. As a result the reserved > transaction credits were not sufficient in some corner cases. Use > ext4_meta_trans_blocks() for correct upper bound credit estimate. > > Fixes: 95ad8ee45cdb ("ext4: correct the reserved credits for extent conversion") > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/ext4/inode.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index f324a54f1dae..ddce319e53c5 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -2852,10 +2852,10 @@ static int ext4_do_writepages(struct mpage_da_data *mpd) > if (ext4_should_dioread_nolock(inode)) { > int bpf = ext4_journal_blocks_per_folio(inode); > /* > - * We may need to convert up to one extent per block in > - * the folio and we may dirty the inode. > + * We may need to convert up to one extent per block in the > + * folio. > */ > - rsv_blocks = 1 + ext4_ext_index_trans_blocks(inode, bpf); > + rsv_blocks = ext4_meta_trans_blocks(inode, bpf, bpf, 0); Looks good Jan, feel free to add: Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Regards, ojaswin > } > > if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX) > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() 2026-08-05 15:35 [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Jan Kara 2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara 2026-08-05 15:35 ` [PATCH 2/3] ext4: Fix transaction overflow during writeback Jan Kara @ 2026-08-05 15:35 ` Jan Kara 2026-08-07 4:46 ` Zhang Yi 2026-08-07 6:37 ` Ojaswin Mujoo 2026-08-10 1:33 ` [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Theodore Ts'o 3 siblings, 2 replies; 14+ messages in thread From: Jan Kara @ 2026-08-05 15:35 UTC (permalink / raw) To: Ted Tso Cc: linux-ext4, Baokun Li, Zhang Yi, Ojaswin Mujoo, Ritesh Harjani, Jan Kara The estimate of the number of impacted extent tree index blocks could be one-too-low. If we modify say 2 extents, already two leaf index blocks could be impacted, not just one the current estimate counts with. Fix the estimate. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/extents.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 44ab246a3176..713b2c098af5 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -2427,9 +2427,17 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents) */ if (extents <= 1) index = (EXT4_MAX_EXTENT_DEPTH * 2) + extents; - else - index = (EXT4_MAX_EXTENT_DEPTH * 3) + - DIV_ROUND_UP(extents, ext4_ext_space_block(inode, 0)); + else { + int ext_max = ext4_ext_space_block(inode, 0); + + index = EXT4_MAX_EXTENT_DEPTH * 3; + /* + * Modified extents need not start at the beginning of the + * leaf. Already two extents may need two leaf block + * modifications... + */ + index += DIV_ROUND_UP(extents + ext_max - 1, ext_max); + } return index; } -- 2.51.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() 2026-08-05 15:35 ` [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() Jan Kara @ 2026-08-07 4:46 ` Zhang Yi 2026-08-07 6:37 ` Ojaswin Mujoo 1 sibling, 0 replies; 14+ messages in thread From: Zhang Yi @ 2026-08-07 4:46 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Ojaswin Mujoo, Ritesh Harjani On 8/5/2026 11:35 PM, Jan Kara wrote: > The estimate of the number of impacted extent tree index blocks could be > one-too-low. If we modify say 2 extents, already two leaf index blocks > could be impacted, not just one the current estimate counts with. Fix > the estimate. > > Signed-off-by: Jan Kara <jack@suse.cz> This is a good catch! Reviewed-by: Zhang Yi <yi.zhang@huawei.com> > --- > fs/ext4/extents.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 44ab246a3176..713b2c098af5 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2427,9 +2427,17 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents) > */ > if (extents <= 1) > index = (EXT4_MAX_EXTENT_DEPTH * 2) + extents; > - else > - index = (EXT4_MAX_EXTENT_DEPTH * 3) + > - DIV_ROUND_UP(extents, ext4_ext_space_block(inode, 0)); > + else { > + int ext_max = ext4_ext_space_block(inode, 0); > + > + index = EXT4_MAX_EXTENT_DEPTH * 3; > + /* > + * Modified extents need not start at the beginning of the > + * leaf. Already two extents may need two leaf block > + * modifications... > + */ > + index += DIV_ROUND_UP(extents + ext_max - 1, ext_max); > + } > > return index; > } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() 2026-08-05 15:35 ` [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() Jan Kara 2026-08-07 4:46 ` Zhang Yi @ 2026-08-07 6:37 ` Ojaswin Mujoo 1 sibling, 0 replies; 14+ messages in thread From: Ojaswin Mujoo @ 2026-08-07 6:37 UTC (permalink / raw) To: Jan Kara; +Cc: Ted Tso, linux-ext4, Baokun Li, Zhang Yi, Ritesh Harjani On Wed, Aug 05, 2026 at 05:35:49PM +0200, Jan Kara wrote: > The estimate of the number of impacted extent tree index blocks could be > one-too-low. If we modify say 2 extents, already two leaf index blocks Hi Jan, Its minor but just to be sure, by two "leaf index blocks" do you mean the two leaf blocks that hold extents or the last index blocks which hold pointers to leaf blocks? > could be impacted, not just one the current estimate counts with. Fix > the estimate. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/ext4/extents.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 44ab246a3176..713b2c098af5 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2427,9 +2427,17 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents) > */ > if (extents <= 1) > index = (EXT4_MAX_EXTENT_DEPTH * 2) + extents; > - else > - index = (EXT4_MAX_EXTENT_DEPTH * 3) + > - DIV_ROUND_UP(extents, ext4_ext_space_block(inode, 0)); > + else { > + int ext_max = ext4_ext_space_block(inode, 0); > + > + index = EXT4_MAX_EXTENT_DEPTH * 3; > + /* > + * Modified extents need not start at the beginning of the > + * leaf. Already two extents may need two leaf block > + * modifications... > + */ > + index += DIV_ROUND_UP(extents + ext_max - 1, ext_max); > + } > > return index; > } > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications 2026-08-05 15:35 [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Jan Kara ` (2 preceding siblings ...) 2026-08-05 15:35 ` [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() Jan Kara @ 2026-08-10 1:33 ` Theodore Ts'o 3 siblings, 0 replies; 14+ messages in thread From: Theodore Ts'o @ 2026-08-10 1:33 UTC (permalink / raw) To: Jan Kara Cc: Theodore Ts'o, linux-ext4, Baokun Li, Zhang Yi, Ojaswin Mujoo, Ritesh Harjani On Wed, 05 Aug 2026 17:35:46 +0200, Jan Kara wrote: > This patch set fixes credit estimates for extent tree modification during > convertion of unwritten extents on writeback completion. Otherwise we are > occasionally seeing assertion failures during extent conversion in end io > completion callback due to insufficient transation credits. > > Honza > > [...] Applied, thanks! [1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents commit: 25b2a7e8d420c49fc46f25b6f953603e4dae1dbf [2/3] ext4: Fix transaction overflow during writeback commit: 46e8e31771f4f1c5e1cdec37a889a6730e42e9f1 [3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() commit: 9091c97be34083587a75db174aab51551d8e8543 Best regards, -- Theodore Ts'o <tytso@mit.edu> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-10 1:34 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-05 15:35 [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Jan Kara 2026-08-05 15:35 ` [PATCH 1/3] ext4: Teach ext4_meta_trans_blocks() about number of allocated extents Jan Kara 2026-08-06 18:09 ` Ojaswin Mujoo 2026-08-07 2:59 ` Zhang Yi 2026-08-07 5:59 ` Ojaswin Mujoo 2026-08-07 7:05 ` Zhang Yi 2026-08-07 7:28 ` Ojaswin Mujoo 2026-08-05 15:35 ` [PATCH 2/3] ext4: Fix transaction overflow during writeback Jan Kara 2026-08-07 3:36 ` Zhang Yi 2026-08-07 8:40 ` Ojaswin Mujoo 2026-08-05 15:35 ` [PATCH 3/3] ext4: Fix estimate extent index blocks in ext4_ext_index_trans_blocks() Jan Kara 2026-08-07 4:46 ` Zhang Yi 2026-08-07 6:37 ` Ojaswin Mujoo 2026-08-10 1:33 ` [PATCH 0/3] ext4: Fix credit estimates for extent tree modifications Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox