* [PATCH v3 0/2] ocfs2: split transactions in dio completion to avoid credit exhaustion
@ 2026-03-26 6:58 Heming Zhao
2026-03-26 6:58 ` [PATCH v3 1/2] " Heming Zhao
2026-03-26 6:58 ` [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits" Heming Zhao
0 siblings, 2 replies; 6+ messages in thread
From: Heming Zhao @ 2026-03-26 6:58 UTC (permalink / raw)
To: joseph.qi; +Cc: Heming Zhao, ocfs2-devel, linux-kernel, jack, glass.su
For easier merging, this patch is based on Joseph's patch [1].
v2->v3:
Following the discussion, use 'batch' and 'handle' to control
restarting the jbd2 transaction.
v1->v2:
following the review comments, restore the i_size update code in
ocfs2_dio_end_io_write().
the runtime of the test script [2].
real 1m49.650s
user 0m0.337s
sys 0m22.780s
[1]:
https://lore.kernel.org/ocfs2-devel/46yilbaq5z5x6gdfdpoa6lprf6sf3gbxriuku2odje4kx4bovf@jd735cphfutz/T/#t
[2]:
https://lore.kernel.org/ocfs2-devel/75f89a17-213b-42a0-a30e-d52fb2d077a6@linux.alibaba.com/T/#mbe2b5f52ee249178e1ad4c76d964de2dc818eb32
Heming Zhao (2):
ocfs2: split transactions in dio completion to avoid credit exhaustion
Revert "ocfs2: fix DIO failure due to insufficient transaction
credits"
fs/ocfs2/aops.c | 75 ++++++++++++++++++++++++------------------
fs/ocfs2/journal.c | 17 ----------
fs/ocfs2/journal.h | 2 --
fs/ocfs2/ocfs2_trace.h | 2 --
4 files changed, 43 insertions(+), 53 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] ocfs2: split transactions in dio completion to avoid credit exhaustion
2026-03-26 6:58 [PATCH v3 0/2] ocfs2: split transactions in dio completion to avoid credit exhaustion Heming Zhao
@ 2026-03-26 6:58 ` Heming Zhao
2026-03-26 9:31 ` Jan Kara
2026-03-26 6:58 ` [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits" Heming Zhao
1 sibling, 1 reply; 6+ messages in thread
From: Heming Zhao @ 2026-03-26 6:58 UTC (permalink / raw)
To: joseph.qi; +Cc: Heming Zhao, ocfs2-devel, linux-kernel, jack, glass.su
During ocfs2 dio operations, JBD2 may report warnings via following call trace:
ocfs2_dio_end_io_write
ocfs2_mark_extent_written
ocfs2_change_extent_flag
ocfs2_split_extent
ocfs2_try_to_merge_extent
ocfs2_extend_rotate_transaction
ocfs2_extend_trans
jbd2__journal_restart
start_this_handle
output: JBD2: kworker/6:2 wants too many credits credits:5450 rsv_credits:0 max:5449
To prevent exceeding the credits limit, modify ocfs2_dio_end_io_write()
to handle extent in a batch of transaction.
Additionally, relocate ocfs2_del_inode_from_orphan(). The orphan inode should
only be removed from the orphan list after the extent tree update is complete.
this ensures that if a crash occurs in the middle of extent tree updates, we
won't leave stale blocks beyond EOF.
Finally, thanks to Jans and Joseph for providing the bug fix prototype and
suggestions.
Suggested-by: Jan Kara <jack@suse.cz>
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/aops.c | 72 ++++++++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 28 deletions(-)
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 09146b43d1f0..60f1b607022f 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -37,6 +37,8 @@
#include "namei.h"
#include "sysfile.h"
+#define OCFS2_DIO_MARK_EXTENT_BATCH 200
+
static int ocfs2_symlink_get_block(struct inode *inode, sector_t iblock,
struct buffer_head *bh_result, int create)
{
@@ -2277,7 +2279,7 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
struct ocfs2_alloc_context *meta_ac = NULL;
handle_t *handle = NULL;
loff_t end = offset + bytes;
- int ret = 0, credits = 0;
+ int ret = 0, credits = 0, batch = 0;
ocfs2_init_dealloc_ctxt(&dealloc);
@@ -2294,18 +2296,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
goto out;
}
- /* Delete orphan before acquire i_rwsem. */
- if (dwc->dw_orphaned) {
- BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
-
- end = end > i_size_read(inode) ? end : 0;
-
- ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh,
- !!end, end);
- if (ret < 0)
- mlog_errno(ret);
- }
-
down_write(&oi->ip_alloc_sem);
di = (struct ocfs2_dinode *)di_bh->b_data;
@@ -2326,20 +2316,22 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
credits = ocfs2_calc_extend_credits(inode->i_sb, &di->id2.i_list);
- handle = ocfs2_start_trans(osb, credits);
- if (IS_ERR(handle)) {
- ret = PTR_ERR(handle);
- mlog_errno(ret);
- goto unlock;
- }
- ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
- OCFS2_JOURNAL_ACCESS_WRITE);
- if (ret) {
- mlog_errno(ret);
- goto commit;
- }
-
list_for_each_entry(ue, &dwc->dw_zero_list, ue_node) {
+ if (!handle) {
+ handle = ocfs2_start_trans(osb, credits);
+ if (IS_ERR(handle)) {
+ ret = PTR_ERR(handle);
+ mlog_errno(ret);
+ handle = NULL;
+ break;
+ }
+ ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
+ OCFS2_JOURNAL_ACCESS_WRITE);
+ if (ret) {
+ mlog_errno(ret);
+ break;
+ }
+ }
ret = ocfs2_assure_trans_credits(handle, credits);
if (ret < 0) {
mlog_errno(ret);
@@ -2353,17 +2345,41 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
mlog_errno(ret);
break;
}
+
+ if (++batch == OCFS2_DIO_MARK_EXTENT_BATCH) {
+ ocfs2_commit_trans(osb, handle);
+ handle = NULL;
+ batch = 0;
+ }
}
if (end > i_size_read(inode)) {
+ if (!handle) {
+ handle = ocfs2_start_trans(osb, credits);
+ if (IS_ERR(handle)) {
+ ret = PTR_ERR(handle);
+ mlog_errno(ret);
+ goto unlock;
+ }
+ }
ret = ocfs2_set_inode_size(handle, inode, di_bh, end);
if (ret < 0)
mlog_errno(ret);
}
-commit:
- ocfs2_commit_trans(osb, handle);
+ if (handle)
+ ocfs2_commit_trans(osb, handle);
+
unlock:
up_write(&oi->ip_alloc_sem);
+
+ /* everything looks good, let's start the cleanup */
+ if (dwc->dw_orphaned) {
+ BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
+
+ ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
+ if (ret < 0)
+ mlog_errno(ret);
+ }
ocfs2_inode_unlock(inode, 1);
brelse(di_bh);
out:
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits"
2026-03-26 6:58 [PATCH v3 0/2] ocfs2: split transactions in dio completion to avoid credit exhaustion Heming Zhao
2026-03-26 6:58 ` [PATCH v3 1/2] " Heming Zhao
@ 2026-03-26 6:58 ` Heming Zhao
2026-03-26 9:38 ` Jan Kara
1 sibling, 1 reply; 6+ messages in thread
From: Heming Zhao @ 2026-03-26 6:58 UTC (permalink / raw)
To: joseph.qi; +Cc: Heming Zhao, ocfs2-devel, linux-kernel, jack, glass.su
This reverts commit be346c1a6eeb49d8fda827d2a9522124c2f72f36.
With the patch "ocfs2: split transactions in dio completion to avoid
credit exhaustion," the jbd2 transaction is now restarted every
OCFS2_DIO_MARK_EXTENT_BATCH loops. In theory, this should prevent
credit exhaustion issues from occurring.
Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/aops.c | 5 -----
fs/ocfs2/journal.c | 17 -----------------
fs/ocfs2/journal.h | 2 --
fs/ocfs2/ocfs2_trace.h | 2 --
4 files changed, 26 deletions(-)
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 60f1b607022f..e58314c6506a 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -2332,11 +2332,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
break;
}
}
- ret = ocfs2_assure_trans_credits(handle, credits);
- if (ret < 0) {
- mlog_errno(ret);
- break;
- }
ret = ocfs2_mark_extent_written(inode, &et, handle,
ue->ue_cpos, 1,
ue->ue_phys,
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 4c86a9d46870..7e7546e070fb 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -464,23 +464,6 @@ int ocfs2_extend_trans(handle_t *handle, int nblocks)
return status;
}
-/*
- * Make sure handle has at least 'nblocks' credits available. If it does not
- * have that many credits available, we will try to extend the handle to have
- * enough credits. If that fails, we will restart transaction to have enough
- * credits. Similar notes regarding data consistency and locking implications
- * as for ocfs2_extend_trans() apply here.
- */
-int ocfs2_assure_trans_credits(handle_t *handle, int nblocks)
-{
- int old_nblks = jbd2_handle_buffer_credits(handle);
-
- trace_ocfs2_assure_trans_credits(old_nblks);
- if (old_nblks >= nblocks)
- return 0;
- return ocfs2_extend_trans(handle, nblocks - old_nblks);
-}
-
/*
* If we have fewer than thresh credits, extend by OCFS2_MAX_TRANS_DATA.
* If that fails, restart the transaction & regain write access for the
diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
index 6397170f302f..9d4763f758ad 100644
--- a/fs/ocfs2/journal.h
+++ b/fs/ocfs2/journal.h
@@ -244,8 +244,6 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb,
int ocfs2_commit_trans(struct ocfs2_super *osb,
handle_t *handle);
int ocfs2_extend_trans(handle_t *handle, int nblocks);
-int ocfs2_assure_trans_credits(handle_t *handle,
- int nblocks);
int ocfs2_allocate_extend_trans(handle_t *handle,
int thresh);
diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h
index 4b32fb5658ad..74b283875e17 100644
--- a/fs/ocfs2/ocfs2_trace.h
+++ b/fs/ocfs2/ocfs2_trace.h
@@ -2575,8 +2575,6 @@ DEFINE_OCFS2_ULL_UINT_EVENT(ocfs2_commit_cache_end);
DEFINE_OCFS2_INT_INT_EVENT(ocfs2_extend_trans);
-DEFINE_OCFS2_INT_EVENT(ocfs2_assure_trans_credits);
-
DEFINE_OCFS2_INT_EVENT(ocfs2_extend_trans_restart);
DEFINE_OCFS2_INT_INT_EVENT(ocfs2_allocate_extend_trans);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] ocfs2: split transactions in dio completion to avoid credit exhaustion
2026-03-26 6:58 ` [PATCH v3 1/2] " Heming Zhao
@ 2026-03-26 9:31 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2026-03-26 9:31 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, ocfs2-devel, linux-kernel, jack, glass.su
On Thu 26-03-26 14:58:17, Heming Zhao wrote:
> During ocfs2 dio operations, JBD2 may report warnings via following call trace:
> ocfs2_dio_end_io_write
> ocfs2_mark_extent_written
> ocfs2_change_extent_flag
> ocfs2_split_extent
> ocfs2_try_to_merge_extent
> ocfs2_extend_rotate_transaction
> ocfs2_extend_trans
> jbd2__journal_restart
> start_this_handle
> output: JBD2: kworker/6:2 wants too many credits credits:5450 rsv_credits:0 max:5449
>
> To prevent exceeding the credits limit, modify ocfs2_dio_end_io_write()
> to handle extent in a batch of transaction.
>
> Additionally, relocate ocfs2_del_inode_from_orphan(). The orphan inode should
> only be removed from the orphan list after the extent tree update is complete.
> this ensures that if a crash occurs in the middle of extent tree updates, we
> won't leave stale blocks beyond EOF.
>
> Finally, thanks to Jans and Joseph for providing the bug fix prototype and
> suggestions.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ocfs2/aops.c | 72 ++++++++++++++++++++++++++++++-------------------
> 1 file changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 09146b43d1f0..60f1b607022f 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -37,6 +37,8 @@
> #include "namei.h"
> #include "sysfile.h"
>
> +#define OCFS2_DIO_MARK_EXTENT_BATCH 200
> +
> static int ocfs2_symlink_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create)
> {
> @@ -2277,7 +2279,7 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
> struct ocfs2_alloc_context *meta_ac = NULL;
> handle_t *handle = NULL;
> loff_t end = offset + bytes;
> - int ret = 0, credits = 0;
> + int ret = 0, credits = 0, batch = 0;
>
> ocfs2_init_dealloc_ctxt(&dealloc);
>
> @@ -2294,18 +2296,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
> goto out;
> }
>
> - /* Delete orphan before acquire i_rwsem. */
> - if (dwc->dw_orphaned) {
> - BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
> -
> - end = end > i_size_read(inode) ? end : 0;
> -
> - ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh,
> - !!end, end);
> - if (ret < 0)
> - mlog_errno(ret);
> - }
> -
> down_write(&oi->ip_alloc_sem);
> di = (struct ocfs2_dinode *)di_bh->b_data;
>
> @@ -2326,20 +2316,22 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
>
> credits = ocfs2_calc_extend_credits(inode->i_sb, &di->id2.i_list);
>
> - handle = ocfs2_start_trans(osb, credits);
> - if (IS_ERR(handle)) {
> - ret = PTR_ERR(handle);
> - mlog_errno(ret);
> - goto unlock;
> - }
> - ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
> - OCFS2_JOURNAL_ACCESS_WRITE);
> - if (ret) {
> - mlog_errno(ret);
> - goto commit;
> - }
> -
> list_for_each_entry(ue, &dwc->dw_zero_list, ue_node) {
> + if (!handle) {
> + handle = ocfs2_start_trans(osb, credits);
> + if (IS_ERR(handle)) {
> + ret = PTR_ERR(handle);
> + mlog_errno(ret);
> + handle = NULL;
> + break;
> + }
> + ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh,
> + OCFS2_JOURNAL_ACCESS_WRITE);
> + if (ret) {
> + mlog_errno(ret);
> + break;
> + }
> + }
> ret = ocfs2_assure_trans_credits(handle, credits);
> if (ret < 0) {
> mlog_errno(ret);
> @@ -2353,17 +2345,41 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
> mlog_errno(ret);
> break;
> }
> +
> + if (++batch == OCFS2_DIO_MARK_EXTENT_BATCH) {
> + ocfs2_commit_trans(osb, handle);
> + handle = NULL;
> + batch = 0;
> + }
> }
>
> if (end > i_size_read(inode)) {
> + if (!handle) {
> + handle = ocfs2_start_trans(osb, credits);
> + if (IS_ERR(handle)) {
> + ret = PTR_ERR(handle);
> + mlog_errno(ret);
> + goto unlock;
> + }
> + }
> ret = ocfs2_set_inode_size(handle, inode, di_bh, end);
> if (ret < 0)
> mlog_errno(ret);
> }
> -commit:
> - ocfs2_commit_trans(osb, handle);
> + if (handle)
> + ocfs2_commit_trans(osb, handle);
> +
> unlock:
> up_write(&oi->ip_alloc_sem);
> +
> + /* everything looks good, let's start the cleanup */
> + if (dwc->dw_orphaned) {
> + BUG_ON(dwc->dw_writer_pid != task_pid_nr(current));
> +
> + ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0);
> + if (ret < 0)
> + mlog_errno(ret);
> + }
> ocfs2_inode_unlock(inode, 1);
> brelse(di_bh);
> out:
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits"
2026-03-26 6:58 ` [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits" Heming Zhao
@ 2026-03-26 9:38 ` Jan Kara
2026-03-26 14:15 ` Heming Zhao
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2026-03-26 9:38 UTC (permalink / raw)
To: Heming Zhao; +Cc: joseph.qi, ocfs2-devel, linux-kernel, jack, glass.su
On Thu 26-03-26 14:58:18, Heming Zhao wrote:
> This reverts commit be346c1a6eeb49d8fda827d2a9522124c2f72f36.
>
> With the patch "ocfs2: split transactions in dio completion to avoid
> credit exhaustion," the jbd2 transaction is now restarted every
> OCFS2_DIO_MARK_EXTENT_BATCH loops. In theory, this should prevent
> credit exhaustion issues from occurring.
I don't see why that would be the case. Look at the commit message of
be346c1a6eeb49d8fda827d2a9522124c2f72f36. The problem is that
ocfs2_calc_extend_credits() doesn't (and hardly can) estimate maximum
number of credits needed for all extent tree manipulations. Some extent tree
manipulations in ocfs2_mark_extent_written() don't reserve additional
credits on their own. This ocfs2_assure_trans_credits() call won't be
needed if you didn't do any batching of extent conversions into a single
transaction. But if you do that, there's no guarantee that all conversions
in a single batch can fit in the amount of credits reserved initially.
Usually it will be the case but I don't see how that would be guaranteed in
every case.
Honza
>
> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> ---
> fs/ocfs2/aops.c | 5 -----
> fs/ocfs2/journal.c | 17 -----------------
> fs/ocfs2/journal.h | 2 --
> fs/ocfs2/ocfs2_trace.h | 2 --
> 4 files changed, 26 deletions(-)
>
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index 60f1b607022f..e58314c6506a 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -2332,11 +2332,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
> break;
> }
> }
> - ret = ocfs2_assure_trans_credits(handle, credits);
> - if (ret < 0) {
> - mlog_errno(ret);
> - break;
> - }
> ret = ocfs2_mark_extent_written(inode, &et, handle,
> ue->ue_cpos, 1,
> ue->ue_phys,
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 4c86a9d46870..7e7546e070fb 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -464,23 +464,6 @@ int ocfs2_extend_trans(handle_t *handle, int nblocks)
> return status;
> }
>
> -/*
> - * Make sure handle has at least 'nblocks' credits available. If it does not
> - * have that many credits available, we will try to extend the handle to have
> - * enough credits. If that fails, we will restart transaction to have enough
> - * credits. Similar notes regarding data consistency and locking implications
> - * as for ocfs2_extend_trans() apply here.
> - */
> -int ocfs2_assure_trans_credits(handle_t *handle, int nblocks)
> -{
> - int old_nblks = jbd2_handle_buffer_credits(handle);
> -
> - trace_ocfs2_assure_trans_credits(old_nblks);
> - if (old_nblks >= nblocks)
> - return 0;
> - return ocfs2_extend_trans(handle, nblocks - old_nblks);
> -}
> -
> /*
> * If we have fewer than thresh credits, extend by OCFS2_MAX_TRANS_DATA.
> * If that fails, restart the transaction & regain write access for the
> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
> index 6397170f302f..9d4763f758ad 100644
> --- a/fs/ocfs2/journal.h
> +++ b/fs/ocfs2/journal.h
> @@ -244,8 +244,6 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb,
> int ocfs2_commit_trans(struct ocfs2_super *osb,
> handle_t *handle);
> int ocfs2_extend_trans(handle_t *handle, int nblocks);
> -int ocfs2_assure_trans_credits(handle_t *handle,
> - int nblocks);
> int ocfs2_allocate_extend_trans(handle_t *handle,
> int thresh);
>
> diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h
> index 4b32fb5658ad..74b283875e17 100644
> --- a/fs/ocfs2/ocfs2_trace.h
> +++ b/fs/ocfs2/ocfs2_trace.h
> @@ -2575,8 +2575,6 @@ DEFINE_OCFS2_ULL_UINT_EVENT(ocfs2_commit_cache_end);
>
> DEFINE_OCFS2_INT_INT_EVENT(ocfs2_extend_trans);
>
> -DEFINE_OCFS2_INT_EVENT(ocfs2_assure_trans_credits);
> -
> DEFINE_OCFS2_INT_EVENT(ocfs2_extend_trans_restart);
>
> DEFINE_OCFS2_INT_INT_EVENT(ocfs2_allocate_extend_trans);
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits"
2026-03-26 9:38 ` Jan Kara
@ 2026-03-26 14:15 ` Heming Zhao
0 siblings, 0 replies; 6+ messages in thread
From: Heming Zhao @ 2026-03-26 14:15 UTC (permalink / raw)
To: Jan Kara; +Cc: joseph.qi, ocfs2-devel, linux-kernel, glass.su
On Thu, Mar 26, 2026 at 10:38:06AM +0100, Jan Kara wrote:
> On Thu 26-03-26 14:58:18, Heming Zhao wrote:
> > This reverts commit be346c1a6eeb49d8fda827d2a9522124c2f72f36.
> >
> > With the patch "ocfs2: split transactions in dio completion to avoid
> > credit exhaustion," the jbd2 transaction is now restarted every
> > OCFS2_DIO_MARK_EXTENT_BATCH loops. In theory, this should prevent
> > credit exhaustion issues from occurring.
>
> I don't see why that would be the case. Look at the commit message of
> be346c1a6eeb49d8fda827d2a9522124c2f72f36. The problem is that
> ocfs2_calc_extend_credits() doesn't (and hardly can) estimate maximum
> number of credits needed for all extent tree manipulations. Some extent tree
> manipulations in ocfs2_mark_extent_written() don't reserve additional
> credits on their own. This ocfs2_assure_trans_credits() call won't be
> needed if you didn't do any batching of extent conversions into a single
> transaction. But if you do that, there's no guarantee that all conversions
> in a single batch can fit in the amount of credits reserved initially.
> Usually it will be the case but I don't see how that would be guaranteed in
> every case.
>
> Honza
Thanks for your careful review. I agree with your point, and I will remove
the revert patch in the v4.
Meanwhile, I have done some tests: the case handled by ocfs2_assure_trans_credits()
is when "old_nblks < nblocks". In my env, old_nblks is a fixed value of 15
("15 < 16"). When writing a fragmented 2GB file, ~400000 loops occur in
ocfs2_dio_end_io_write(), and this condition is triggered about 5 times in total.
- Heming
>
> >
> > Signed-off-by: Heming Zhao <heming.zhao@suse.com>
> > ---
> > fs/ocfs2/aops.c | 5 -----
> > fs/ocfs2/journal.c | 17 -----------------
> > fs/ocfs2/journal.h | 2 --
> > fs/ocfs2/ocfs2_trace.h | 2 --
> > 4 files changed, 26 deletions(-)
> >
> > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> > index 60f1b607022f..e58314c6506a 100644
> > --- a/fs/ocfs2/aops.c
> > +++ b/fs/ocfs2/aops.c
> > @@ -2332,11 +2332,6 @@ static int ocfs2_dio_end_io_write(struct inode *inode,
> > break;
> > }
> > }
> > - ret = ocfs2_assure_trans_credits(handle, credits);
> > - if (ret < 0) {
> > - mlog_errno(ret);
> > - break;
> > - }
> > ret = ocfs2_mark_extent_written(inode, &et, handle,
> > ue->ue_cpos, 1,
> > ue->ue_phys,
> > diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> > index 4c86a9d46870..7e7546e070fb 100644
> > --- a/fs/ocfs2/journal.c
> > +++ b/fs/ocfs2/journal.c
> > @@ -464,23 +464,6 @@ int ocfs2_extend_trans(handle_t *handle, int nblocks)
> > return status;
> > }
> >
> > -/*
> > - * Make sure handle has at least 'nblocks' credits available. If it does not
> > - * have that many credits available, we will try to extend the handle to have
> > - * enough credits. If that fails, we will restart transaction to have enough
> > - * credits. Similar notes regarding data consistency and locking implications
> > - * as for ocfs2_extend_trans() apply here.
> > - */
> > -int ocfs2_assure_trans_credits(handle_t *handle, int nblocks)
> > -{
> > - int old_nblks = jbd2_handle_buffer_credits(handle);
> > -
> > - trace_ocfs2_assure_trans_credits(old_nblks);
> > - if (old_nblks >= nblocks)
> > - return 0;
> > - return ocfs2_extend_trans(handle, nblocks - old_nblks);
> > -}
> > -
> > /*
> > * If we have fewer than thresh credits, extend by OCFS2_MAX_TRANS_DATA.
> > * If that fails, restart the transaction & regain write access for the
> > diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
> > index 6397170f302f..9d4763f758ad 100644
> > --- a/fs/ocfs2/journal.h
> > +++ b/fs/ocfs2/journal.h
> > @@ -244,8 +244,6 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb,
> > int ocfs2_commit_trans(struct ocfs2_super *osb,
> > handle_t *handle);
> > int ocfs2_extend_trans(handle_t *handle, int nblocks);
> > -int ocfs2_assure_trans_credits(handle_t *handle,
> > - int nblocks);
> > int ocfs2_allocate_extend_trans(handle_t *handle,
> > int thresh);
> >
> > diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h
> > index 4b32fb5658ad..74b283875e17 100644
> > --- a/fs/ocfs2/ocfs2_trace.h
> > +++ b/fs/ocfs2/ocfs2_trace.h
> > @@ -2575,8 +2575,6 @@ DEFINE_OCFS2_ULL_UINT_EVENT(ocfs2_commit_cache_end);
> >
> > DEFINE_OCFS2_INT_INT_EVENT(ocfs2_extend_trans);
> >
> > -DEFINE_OCFS2_INT_EVENT(ocfs2_assure_trans_credits);
> > -
> > DEFINE_OCFS2_INT_EVENT(ocfs2_extend_trans_restart);
> >
> > DEFINE_OCFS2_INT_INT_EVENT(ocfs2_allocate_extend_trans);
> > --
> > 2.43.0
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-26 14:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-26 6:58 [PATCH v3 0/2] ocfs2: split transactions in dio completion to avoid credit exhaustion Heming Zhao
2026-03-26 6:58 ` [PATCH v3 1/2] " Heming Zhao
2026-03-26 9:31 ` Jan Kara
2026-03-26 6:58 ` [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits" Heming Zhao
2026-03-26 9:38 ` Jan Kara
2026-03-26 14:15 ` Heming Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®