Re: [PATCH v3 2/2] Revert "ocfs2: fix DIO failure due to insufficient transaction credits"
From: Jan Kara
Date: Thu Mar 26 2026 - 05:47:53 EST
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@xxxxxxxx>
> ---
> 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@xxxxxxxx>
SUSE Labs, CR