Re: [PATCH v1] jbd2: check transaction state before stopping handle

From: liubaolin

Date: Fri Mar 06 2026 - 04:23:19 EST

Dear Honza,

Thank you for your feedback. Yes, the BUG is exactly at:
J_ASSERT(atomic_read(&transaction->t_updates) > 0)

Dear Honza,

Thank you for your feedback. Yes, the BUG is exactly at:
J_ASSERT(atomic_read(&transaction->t_updates) > 0)

I understand your concern that this shouldn't happen in normal operation. However, from the vmcore dump, we can confirm that:
- handle->h_transaction->t_updates == 0
- handle->h_transaction->t_state == T_FINISHED
- The crash occurred in stop_this_handle() when it tried to assert t_updates > 0

This is a real crash that happened in production. The crash stack shows it occurred during ext4_create(), and the process name was MemTableFlushTh.

I understand you want us to find and fix the root cause. I've reviewed the code but haven't found an obvious bug from the code that would cause this issue. However, I suspect the issue might be related to credit exhaustion during ext4_create() that triggers a handle restart, but this is just speculation and I'm still studying the code to confirm.

This crash only occurred once in production and we haven't been able to reproduce it. The crash happened under high concurrency, and appears to be timing-dependent. I will continue to investigate the code and try to find a way to reproduce this issue to identify the root cause.

The patch I proposed is a defensive check to prevent the kernel BUG when this edge case occurs.

If you have any thoughts on where to look for the root cause, I'd really appreciate any suggestions.

Best regards,
Baolin Liu

在 2026/3/5 21:08, Jan Kara 写道:

On Thu 05-03-26 20:57:18, Baolin Liu wrote:

At 2026-03-05 20:54:02, "Baolin Liu" <liubaolin12138@xxxxxxx> wrote:

From: Baolin Liu <liubaolin@xxxxxxxxxx>

When a transaction enters T_FLUSH or later states,
handle->h_transaction may still point to it.
If jbd2_journal_stop() or jbd2__journal_restart() is called,
stop_this_handle() checks t_updates > 0, but t_updates is
already 0 for these states, causing a kernel BUG.

Which bug please? Do you mean

J_ASSERT(atomic_read(&transaction->t_updates) > 0)

? Anyway this just doesn't make sense. When stop_this_handle() the caller
is holding t_updates reference which stop_this_handle() is supposed to drop
and the transaction should never transition past T_LOCKED state. If you
have a handle that's pointing to a transaction past T_LOCKED state, there's
a bug somewhere and that bug needs to be fixed, not paper over it like you
do in this patch. More details about reproducer etc. would be useful.

Honza

Fix by checking transaction->t_state in jbd2_journal_stop()
and jbd2__journal_restart() before calling stop_this_handle().
If the transaction is not in T_RUNNING or T_LOCKED state,
clear handle->h_transaction and skip stop_this_handle().

Crash stack:
Call trace:
stop_this_handle+0x148/0x158
jbd2_journal_stop+0x198/0x388
__ext4_journal_stop+0x70/0xf0
ext4_create+0x12c/0x188
lookup_open+0x214/0x6d8
do_last+0x364/0x878
path_openat+0x6c/0x280
do_filp_open+0x70/0xe8
do_sys_open+0x178/0x200
sys_openat+0x3c/0x50
el0_svc_naked+0x44/0x48

Signed-off-by: Baolin Liu <liubaolin@xxxxxxxxxx>
---
fs/jbd2/transaction.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index dca4b5d8aaaa..3779382dbb80 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -772,14 +772,25 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, int revoke_records,
journal = transaction->t_journal;
tid = transaction->t_tid;

+ jbd2_debug(2, "restarting handle %p\n", handle);
+
+ /* Check if transaction is in invalid state */
+ if (transaction->t_state != T_RUNNING &&
+ transaction->t_state != T_LOCKED) {
+ if (current->journal_info == handle)
+ current->journal_info = NULL;
+ handle->h_transaction = NULL;
+ memalloc_nofs_restore(handle->saved_alloc_context);
+ goto skip_stop;
+ }
+
/*
* First unlink the handle from its current transaction, and start the
* commit on that.
*/
- jbd2_debug(2, "restarting handle %p\n", handle);
stop_this_handle(handle);
handle->h_transaction = NULL;
-
+skip_stop:
/*
* TODO: If we use READ_ONCE / WRITE_ONCE for j_commit_request we can
* get rid of pointless j_state_lock traffic like this.
@@ -1856,6 +1867,16 @@ int jbd2_journal_stop(handle_t *handle)
memalloc_nofs_restore(handle->saved_alloc_context);
goto free_and_exit;
}
+ /* Check if transaction is in invalid state */
+ if (transaction->t_state != T_RUNNING &&
+ transaction->t_state != T_LOCKED) {
+ if (current->journal_info == handle)
+ current->journal_info = NULL;
+ handle->h_transaction = NULL;
+ memalloc_nofs_restore(handle->saved_alloc_context);
+ goto free_and_exit;
+ }
+
journal = transaction->t_journal;
tid = transaction->t_tid;

--
2.39.2