* [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
* [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
* [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 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 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 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 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 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 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
* 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
* 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