public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage
@ 2026-03-03  0:55 Milos Nikic
  2026-03-03  0:55 ` [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer Milos Nikic
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Milos Nikic @ 2026-03-03  0:55 UTC (permalink / raw)
  To: jack; +Cc: tytso, linux-ext4, linux-kernel, Milos Nikic

Hello Jan and the ext4 team,

This patch series follows up on the previous discussion regarding
converting hard J_ASSERT panics into graceful journal aborts.

In v1, we addressed a specific panic on unlock. Per Jan's suggestion,
I have audited fs/jbd2/transaction.c for other low-hanging fruit
where state machine invariants are enforced by J_ASSERT inside
functions that natively support error returns.

Changes in v3:

    Patch 2: Added pr_err() statements inside the ambiguous WARN_ON_ONCE()
    blocks (where multiple conditions are checked via logical OR/AND) to
    explicitly dump the b_transaction, b_next_transaction, and
    j_committing_transaction pointers. This provides necessary context for
    debugging state machine corruptions from the dmesg stack trace.

Changes in v2:

    Patch 1: Unmodified from v1. Collected Reviewed-by tags.

    Patch 2: New patch resulting from the broader audit. Systematically
    replaces J_ASSERTs with WARN_ON_ONCE and graceful -EINVAL returns
    across 6 core transaction lifecycle functions. Careful attention was
    paid to ensuring spinlocks are safely dropped before triggering
    jbd2_journal_abort(), and no memory is leaked on the error paths.

Milos Nikic (2):
  jbd2: gracefully abort instead of panicking on unlocked buffer
  jbd2: gracefully abort on transaction state corruptions

 fs/jbd2/transaction.c | 115 +++++++++++++++++++++++++++++++++---------
 1 file changed, 91 insertions(+), 24 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer
  2026-03-03  0:55 [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage Milos Nikic
@ 2026-03-03  0:55 ` Milos Nikic
  2026-03-03  1:05   ` Andreas Dilger
  2026-03-03  0:55 ` [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions Milos Nikic
  2026-03-03  2:11 ` [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage yebin (H)
  2 siblings, 1 reply; 6+ messages in thread
From: Milos Nikic @ 2026-03-03  0:55 UTC (permalink / raw)
  To: jack; +Cc: tytso, linux-ext4, linux-kernel, Milos Nikic, Zhang Yi

In jbd2_journal_get_create_access(), if the caller passes an unlocked
buffer, the code currently triggers a fatal J_ASSERT.

While an unlocked buffer here is a clear API violation and a bug in the
caller, crashing the entire system is an overly severe response. It brings
down the whole machine for a localized filesystem inconsistency.

Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
stack trace, and return an error (-EINVAL). This allows the journal to
gracefully abort the transaction, protecting data integrity without
causing a kernel panic.

Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/jbd2/transaction.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index dca4b5d8aaaa..04d17a5f2a82 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
 		goto out;
 	}
 
-	J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
+	if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
+		err = -EINVAL;
+		spin_unlock(&jh->b_state_lock);
+		jbd2_journal_abort(journal, err);
+		goto out;
+	}
 
 	if (jh->b_transaction == NULL) {
 		/*
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions
  2026-03-03  0:55 [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage Milos Nikic
  2026-03-03  0:55 ` [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer Milos Nikic
@ 2026-03-03  0:55 ` Milos Nikic
  2026-03-03  6:16   ` kernel test robot
  2026-03-03  2:11 ` [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage yebin (H)
  2 siblings, 1 reply; 6+ messages in thread
From: Milos Nikic @ 2026-03-03  0:55 UTC (permalink / raw)
  To: jack; +Cc: tytso, linux-ext4, linux-kernel, Milos Nikic

Auditing the jbd2 codebase reveals several legacy J_ASSERT calls
that enforce internal state machine invariants (e.g., verifying
jh->b_transaction or jh->b_next_transaction pointers).

When these invariants are broken, the journal is in a corrupted
state. However, triggering a fatal panic brings down the entire
system for a localized filesystem error.

This patch targets a specific class of these asserts: those
residing inside functions that natively return integer error codes,
booleans, or error pointers. It replaces the hard J_ASSERTs with
WARN_ON_ONCE to capture the offending stack trace, safely drops
any held locks, gracefully aborts the journal, and returns -EINVAL.

This prevents a catastrophic kernel panic while ensuring the
corrupted journal state is safely contained and upstream callers
(like ext4 or ocfs2) can gracefully handle the aborted handle.

Functions modified in fs/jbd2/transaction.c:
- jbd2__journal_start()
- do_get_write_access()
- jbd2_journal_dirty_metadata()
- jbd2_journal_forget()
- jbd2_journal_try_to_free_buffers()
- jbd2_journal_file_inode()

Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
---
 fs/jbd2/transaction.c | 108 +++++++++++++++++++++++++++++++++---------
 1 file changed, 85 insertions(+), 23 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 04d17a5f2a82..3cb4e524e0a6 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -474,7 +474,8 @@ handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int rsv_blocks,
 		return ERR_PTR(-EROFS);
 
 	if (handle) {
-		J_ASSERT(handle->h_transaction->t_journal == journal);
+		if (WARN_ON_ONCE(handle->h_transaction->t_journal != journal))
+			return ERR_PTR(-EINVAL);
 		handle->h_ref++;
 		return handle;
 	}
@@ -1036,7 +1037,13 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	 */
 	if (!jh->b_transaction) {
 		JBUFFER_TRACE(jh, "no transaction");
-		J_ASSERT_JH(jh, !jh->b_next_transaction);
+		if (WARN_ON_ONCE(jh->b_next_transaction)) {
+			spin_unlock(&jh->b_state_lock);
+			unlock_buffer(bh);
+			error = -EINVAL;
+			jbd2_journal_abort(journal, error);
+			goto out;
+		}
 		JBUFFER_TRACE(jh, "file as BJ_Reserved");
 		/*
 		 * Make sure all stores to jh (b_modified, b_frozen_data) are
@@ -1069,13 +1076,27 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	 */
 	if (jh->b_frozen_data) {
 		JBUFFER_TRACE(jh, "has frozen data");
-		J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
+		if (WARN_ON_ONCE(jh->b_next_transaction)) {
+			spin_unlock(&jh->b_state_lock);
+			error = -EINVAL;
+			jbd2_journal_abort(journal, error);
+			goto out;
+		}
 		goto attach_next;
 	}
 
 	JBUFFER_TRACE(jh, "owned by older transaction");
-	J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
-	J_ASSERT_JH(jh, jh->b_transaction == journal->j_committing_transaction);
+	if (WARN_ON_ONCE(jh->b_next_transaction ||
+			 jh->b_transaction !=
+			 journal->j_committing_transaction)) {
+		pr_err("JBD2: %s: assertion failure: b_next_transaction=%p b_transaction=%p j_committing_transaction=%p\n",
+		       journal->j_devname, jh->b_next_transaction,
+		       jh->b_transaction, journal->j_committing_transaction);
+		spin_unlock(&jh->b_state_lock);
+		error = -EINVAL;
+		jbd2_journal_abort(journal, error);
+		goto out;
+	}
 
 	/*
 	 * There is one case we have to be very careful about.  If the
@@ -1520,8 +1541,14 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
 	if (data_race(jh->b_transaction != transaction &&
 	    jh->b_next_transaction != transaction)) {
 		spin_lock(&jh->b_state_lock);
-		J_ASSERT_JH(jh, jh->b_transaction == transaction ||
-				jh->b_next_transaction == transaction);
+		if (WARN_ON_ONCE(jh->b_transaction != transaction &&
+				 jh->b_next_transaction != transaction)) {
+			pr_err("JBD2: %s: assertion failure: b_transaction=%p transaction=%p b_next_transaction=%p\n",
+			       journal->j_devname, jh->b_transaction,
+			       transaction, jh->b_next_transaction);
+			ret = -EINVAL;
+			goto out_unlock_bh;
+		}
 		spin_unlock(&jh->b_state_lock);
 	}
 	if (data_race(jh->b_modified == 1)) {
@@ -1531,13 +1558,15 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
 			spin_lock(&jh->b_state_lock);
 			if (jh->b_transaction == transaction &&
 			    jh->b_jlist != BJ_Metadata)
-				pr_err("JBD2: assertion failure: h_type=%u "
-				       "h_line_no=%u block_no=%llu jlist=%u\n",
+				pr_err("JBD2: assertion failure: h_type=%u h_line_no=%u block_no=%llu jlist=%u\n",
 				       handle->h_type, handle->h_line_no,
 				       (unsigned long long) bh->b_blocknr,
 				       jh->b_jlist);
-			J_ASSERT_JH(jh, jh->b_transaction != transaction ||
-					jh->b_jlist == BJ_Metadata);
+			if (WARN_ON_ONCE(jh->b_transaction == transaction &&
+					 jh->b_jlist != BJ_Metadata)) {
+				ret = -EINVAL;
+				goto out_unlock_bh;
+			}
 			spin_unlock(&jh->b_state_lock);
 		}
 		goto out;
@@ -1636,7 +1665,10 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
 	}
 
 	/* That test should have eliminated the following case: */
-	J_ASSERT_JH(jh, jh->b_frozen_data == NULL);
+	if (WARN_ON_ONCE(jh->b_frozen_data)) {
+		ret = -EINVAL;
+		goto out_unlock_bh;
+	}
 
 	JBUFFER_TRACE(jh, "file as BJ_Metadata");
 	spin_lock(&journal->j_list_lock);
@@ -1675,6 +1707,7 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 	int err = 0;
 	int was_modified = 0;
 	int wait_for_writeback = 0;
+	int abort_journal = 0;
 
 	if (is_handle_aborted(handle))
 		return -EROFS;
@@ -1708,7 +1741,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 	jh->b_modified = 0;
 
 	if (jh->b_transaction == transaction) {
-		J_ASSERT_JH(jh, !jh->b_frozen_data);
+		if (WARN_ON_ONCE(jh->b_frozen_data)) {
+			err = -EINVAL;
+			abort_journal = 1;
+			goto drop;
+		}
 
 		/* If we are forgetting a buffer which is already part
 		 * of this transaction, then we can just drop it from
@@ -1747,8 +1784,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 		}
 		spin_unlock(&journal->j_list_lock);
 	} else if (jh->b_transaction) {
-		J_ASSERT_JH(jh, (jh->b_transaction ==
-				 journal->j_committing_transaction));
+		if (WARN_ON_ONCE(jh->b_transaction != journal->j_committing_transaction)) {
+			err = -EINVAL;
+			abort_journal = 1;
+			goto drop;
+		}
 		/* However, if the buffer is still owned by a prior
 		 * (committing) transaction, we can't drop it yet... */
 		JBUFFER_TRACE(jh, "belongs to older transaction");
@@ -1766,7 +1806,11 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 			jh->b_next_transaction = transaction;
 			spin_unlock(&journal->j_list_lock);
 		} else {
-			J_ASSERT(jh->b_next_transaction == transaction);
+			if (WARN_ON_ONCE(jh->b_next_transaction != transaction)) {
+				err = -EINVAL;
+				abort_journal = 1;
+				goto drop;
+			}
 
 			/*
 			 * only drop a reference if this transaction modified
@@ -1812,6 +1856,8 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 drop:
 	__brelse(bh);
 	spin_unlock(&jh->b_state_lock);
+	if (abort_journal)
+		jbd2_journal_abort(journal, err);
 	if (wait_for_writeback)
 		wait_on_buffer(bh);
 	jbd2_journal_put_journal_head(jh);
@@ -2136,7 +2182,8 @@ bool jbd2_journal_try_to_free_buffers(journal_t *journal, struct folio *folio)
 	struct buffer_head *bh;
 	bool ret = false;
 
-	J_ASSERT(folio_test_locked(folio));
+	if (WARN_ON_ONCE(!folio_test_locked(folio)))
+		return false;
 
 	head = folio_buffers(folio);
 	bh = head;
@@ -2651,6 +2698,8 @@ static int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *jinode,
 {
 	transaction_t *transaction = handle->h_transaction;
 	journal_t *journal;
+	int err = 0;
+	int abort_transaction = 0;
 
 	if (is_handle_aborted(handle))
 		return -EROFS;
@@ -2685,20 +2734,33 @@ static int jbd2_journal_file_inode(handle_t *handle, struct jbd2_inode *jinode,
 	/* On some different transaction's list - should be
 	 * the committing one */
 	if (jinode->i_transaction) {
-		J_ASSERT(jinode->i_next_transaction == NULL);
-		J_ASSERT(jinode->i_transaction ==
-					journal->j_committing_transaction);
+		if (WARN_ON_ONCE(jinode->i_next_transaction ||
+				 jinode->i_transaction !=
+				 journal->j_committing_transaction)) {
+			pr_err("JBD2: %s: assertion failure: i_next_transaction=%p i_transaction=%p j_committing_transaction=%p\n",
+			       journal->j_devname, jinode->i_next_transaction,
+			       jinode->i_transaction,
+			       journal->j_committing_transaction);
+			err = -EINVAL;
+			abort_transaction = 1;
+			goto done;
+		}
 		jinode->i_next_transaction = transaction;
 		goto done;
 	}
 	/* Not on any transaction list... */
-	J_ASSERT(!jinode->i_next_transaction);
+	if (WARN_ON_ONCE(jinode->i_next_transaction)) {
+		err = -EINVAL;
+		abort_transaction = 1;
+		goto done;
+	}
 	jinode->i_transaction = transaction;
 	list_add(&jinode->i_list, &transaction->t_inode_list);
 done:
 	spin_unlock(&journal->j_list_lock);
-
-	return 0;
+	if (abort_transaction)
+		jbd2_journal_abort(journal, err);
+	return err;
 }
 
 int jbd2_journal_inode_ranged_write(handle_t *handle,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer
  2026-03-03  0:55 ` [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer Milos Nikic
@ 2026-03-03  1:05   ` Andreas Dilger
  0 siblings, 0 replies; 6+ messages in thread
From: Andreas Dilger @ 2026-03-03  1:05 UTC (permalink / raw)
  To: Milos Nikic; +Cc: jack, tytso, linux-ext4, linux-kernel, Zhang Yi

On Mar 2, 2026, at 17:55, Milos Nikic <nikic.milos@gmail.com> wrote:
> 
> In jbd2_journal_get_create_access(), if the caller passes an unlocked
> buffer, the code currently triggers a fatal J_ASSERT.
> 
> While an unlocked buffer here is a clear API violation and a bug in the
> caller, crashing the entire system is an overly severe response. It brings
> down the whole machine for a localized filesystem inconsistency.
> 
> Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
> stack trace, and return an error (-EINVAL). This allows the journal to
> gracefully abort the transaction, protecting data integrity without
> causing a kernel panic.
> 
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>

> ---
> fs/jbd2/transaction.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index dca4b5d8aaaa..04d17a5f2a82 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
> goto out;
> }
> 
> -	J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
> +	if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
> +		err = -EINVAL;
> +		spin_unlock(&jh->b_state_lock);
> +		jbd2_journal_abort(journal, err);
> +	goto out;
> +	}
> 
> if (jh->b_transaction == NULL) {
> /*
> -- 
> 2.53.0
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage
  2026-03-03  0:55 [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage Milos Nikic
  2026-03-03  0:55 ` [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer Milos Nikic
  2026-03-03  0:55 ` [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions Milos Nikic
@ 2026-03-03  2:11 ` yebin (H)
  2 siblings, 0 replies; 6+ messages in thread
From: yebin (H) @ 2026-03-03  2:11 UTC (permalink / raw)
  To: jack; +Cc: tytso, linux-ext4, linux-kernel

The macro `J_ASSERT_JH` is a rather troublesome implementation. There
are numerous calls to `J_ASSERT_JH` within
`jbd2_journal_commit_transaction()`, and after compilation, these may
all jump to the same address for execution, making it difficult to
determine exactly where the assertion is being triggered. If there is a
functional issue in just a single file system, using `BUG_ON` to handle
it seems a bit too aggressive.
I wonder if you all have any good ideas or suggestions.

On 2026/3/3 8:55, Milos Nikic wrote:
> Hello Jan and the ext4 team,
>
> This patch series follows up on the previous discussion regarding
> converting hard J_ASSERT panics into graceful journal aborts.
>
> In v1, we addressed a specific panic on unlock. Per Jan's suggestion,
> I have audited fs/jbd2/transaction.c for other low-hanging fruit
> where state machine invariants are enforced by J_ASSERT inside
> functions that natively support error returns.
>
> Changes in v3:
>
>      Patch 2: Added pr_err() statements inside the ambiguous WARN_ON_ONCE()
>      blocks (where multiple conditions are checked via logical OR/AND) to
>      explicitly dump the b_transaction, b_next_transaction, and
>      j_committing_transaction pointers. This provides necessary context for
>      debugging state machine corruptions from the dmesg stack trace.
>
> Changes in v2:
>
>      Patch 1: Unmodified from v1. Collected Reviewed-by tags.
>
>      Patch 2: New patch resulting from the broader audit. Systematically
>      replaces J_ASSERTs with WARN_ON_ONCE and graceful -EINVAL returns
>      across 6 core transaction lifecycle functions. Careful attention was
>      paid to ensuring spinlocks are safely dropped before triggering
>      jbd2_journal_abort(), and no memory is leaked on the error paths.
>
> Milos Nikic (2):
>    jbd2: gracefully abort instead of panicking on unlocked buffer
>    jbd2: gracefully abort on transaction state corruptions
>
>   fs/jbd2/transaction.c | 115 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 91 insertions(+), 24 deletions(-)
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions
  2026-03-03  0:55 ` [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions Milos Nikic
@ 2026-03-03  6:16   ` kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-03-03  6:16 UTC (permalink / raw)
  To: Milos Nikic, jack
  Cc: llvm, oe-kbuild-all, tytso, linux-ext4, linux-kernel, Milos Nikic

Hi Milos,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on tytso-ext4/dev next-20260302]
[cannot apply to linus/master v6.16-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Milos-Nikic/jbd2-gracefully-abort-instead-of-panicking-on-unlocked-buffer/20260303-085946
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20260303005502.337108-3-nikic.milos%40gmail.com
patch subject: [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260303/202603030706.NpyA7pqe-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260303/202603030706.NpyA7pqe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603030706.NpyA7pqe-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/jbd2/transaction.c:1547:11: warning: variable 'journal' is uninitialized when used here [-Wuninitialized]
    1547 |                                journal->j_devname, jh->b_transaction,
         |                                ^~~~~~~
   include/linux/printk.h:554:33: note: expanded from macro 'pr_err'
     554 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                                        ^~~~~~~~~~~
   include/linux/printk.h:511:60: note: expanded from macro 'printk'
     511 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
         |                                                            ^~~~~~~~~~~
   include/linux/printk.h:483:19: note: expanded from macro 'printk_index_wrap'
     483 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                                 ^~~~~~~~~~~
   fs/jbd2/transaction.c:1520:20: note: initialize the variable 'journal' to silence this warning
    1520 |         journal_t *journal;
         |                           ^
         |                            = NULL
   1 warning generated.


vim +/journal +1547 fs/jbd2/transaction.c

  1493	
  1494	/**
  1495	 * jbd2_journal_dirty_metadata() -  mark a buffer as containing dirty metadata
  1496	 * @handle: transaction to add buffer to.
  1497	 * @bh: buffer to mark
  1498	 *
  1499	 * mark dirty metadata which needs to be journaled as part of the current
  1500	 * transaction.
  1501	 *
  1502	 * The buffer must have previously had jbd2_journal_get_write_access()
  1503	 * called so that it has a valid journal_head attached to the buffer
  1504	 * head.
  1505	 *
  1506	 * The buffer is placed on the transaction's metadata list and is marked
  1507	 * as belonging to the transaction.
  1508	 *
  1509	 * Returns error number or 0 on success.
  1510	 *
  1511	 * Special care needs to be taken if the buffer already belongs to the
  1512	 * current committing transaction (in which case we should have frozen
  1513	 * data present for that commit).  In that case, we don't relink the
  1514	 * buffer: that only gets done when the old transaction finally
  1515	 * completes its commit.
  1516	 */
  1517	int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
  1518	{
  1519		transaction_t *transaction = handle->h_transaction;
  1520		journal_t *journal;
  1521		struct journal_head *jh;
  1522		int ret = 0;
  1523	
  1524		if (!buffer_jbd(bh))
  1525			return -EUCLEAN;
  1526	
  1527		/*
  1528		 * We don't grab jh reference here since the buffer must be part
  1529		 * of the running transaction.
  1530		 */
  1531		jh = bh2jh(bh);
  1532		jbd2_debug(5, "journal_head %p\n", jh);
  1533		JBUFFER_TRACE(jh, "entry");
  1534	
  1535		/*
  1536		 * This and the following assertions are unreliable since we may see jh
  1537		 * in inconsistent state unless we grab bh_state lock. But this is
  1538		 * crucial to catch bugs so let's do a reliable check until the
  1539		 * lockless handling is fully proven.
  1540		 */
  1541		if (data_race(jh->b_transaction != transaction &&
  1542		    jh->b_next_transaction != transaction)) {
  1543			spin_lock(&jh->b_state_lock);
  1544			if (WARN_ON_ONCE(jh->b_transaction != transaction &&
  1545					 jh->b_next_transaction != transaction)) {
  1546				pr_err("JBD2: %s: assertion failure: b_transaction=%p transaction=%p b_next_transaction=%p\n",
> 1547				       journal->j_devname, jh->b_transaction,
  1548				       transaction, jh->b_next_transaction);
  1549				ret = -EINVAL;
  1550				goto out_unlock_bh;
  1551			}
  1552			spin_unlock(&jh->b_state_lock);
  1553		}
  1554		if (data_race(jh->b_modified == 1)) {
  1555			/* If it's in our transaction it must be in BJ_Metadata list. */
  1556			if (data_race(jh->b_transaction == transaction &&
  1557			    jh->b_jlist != BJ_Metadata)) {
  1558				spin_lock(&jh->b_state_lock);
  1559				if (jh->b_transaction == transaction &&
  1560				    jh->b_jlist != BJ_Metadata)
  1561					pr_err("JBD2: assertion failure: h_type=%u h_line_no=%u block_no=%llu jlist=%u\n",
  1562					       handle->h_type, handle->h_line_no,
  1563					       (unsigned long long) bh->b_blocknr,
  1564					       jh->b_jlist);
  1565				if (WARN_ON_ONCE(jh->b_transaction == transaction &&
  1566						 jh->b_jlist != BJ_Metadata)) {
  1567					ret = -EINVAL;
  1568					goto out_unlock_bh;
  1569				}
  1570				spin_unlock(&jh->b_state_lock);
  1571			}
  1572			goto out;
  1573		}
  1574	
  1575		spin_lock(&jh->b_state_lock);
  1576	
  1577		if (is_handle_aborted(handle)) {
  1578			/*
  1579			 * Check journal aborting with @jh->b_state_lock locked,
  1580			 * since 'jh->b_transaction' could be replaced with
  1581			 * 'jh->b_next_transaction' during old transaction
  1582			 * committing if journal aborted, which may fail
  1583			 * assertion on 'jh->b_frozen_data == NULL'.
  1584			 */
  1585			ret = -EROFS;
  1586			goto out_unlock_bh;
  1587		}
  1588	
  1589		journal = transaction->t_journal;
  1590	
  1591		if (jh->b_modified == 0) {
  1592			/*
  1593			 * This buffer's got modified and becoming part
  1594			 * of the transaction. This needs to be done
  1595			 * once a transaction -bzzz
  1596			 */
  1597			if (WARN_ON_ONCE(jbd2_handle_buffer_credits(handle) <= 0)) {
  1598				ret = -ENOSPC;
  1599				goto out_unlock_bh;
  1600			}
  1601			jh->b_modified = 1;
  1602			handle->h_total_credits--;
  1603		}
  1604	
  1605		/*
  1606		 * fastpath, to avoid expensive locking.  If this buffer is already
  1607		 * on the running transaction's metadata list there is nothing to do.
  1608		 * Nobody can take it off again because there is a handle open.
  1609		 * I _think_ we're OK here with SMP barriers - a mistaken decision will
  1610		 * result in this test being false, so we go in and take the locks.
  1611		 */
  1612		if (jh->b_transaction == transaction && jh->b_jlist == BJ_Metadata) {
  1613			JBUFFER_TRACE(jh, "fastpath");
  1614			if (unlikely(jh->b_transaction !=
  1615				     journal->j_running_transaction)) {
  1616				printk(KERN_ERR "JBD2: %s: "
  1617				       "jh->b_transaction (%llu, %p, %u) != "
  1618				       "journal->j_running_transaction (%p, %u)\n",
  1619				       journal->j_devname,
  1620				       (unsigned long long) bh->b_blocknr,
  1621				       jh->b_transaction,
  1622				       jh->b_transaction ? jh->b_transaction->t_tid : 0,
  1623				       journal->j_running_transaction,
  1624				       journal->j_running_transaction ?
  1625				       journal->j_running_transaction->t_tid : 0);
  1626				ret = -EINVAL;
  1627			}
  1628			goto out_unlock_bh;
  1629		}
  1630	
  1631		set_buffer_jbddirty(bh);
  1632	
  1633		/*
  1634		 * Metadata already on the current transaction list doesn't
  1635		 * need to be filed.  Metadata on another transaction's list must
  1636		 * be committing, and will be refiled once the commit completes:
  1637		 * leave it alone for now.
  1638		 */
  1639		if (jh->b_transaction != transaction) {
  1640			JBUFFER_TRACE(jh, "already on other transaction");
  1641			if (unlikely(((jh->b_transaction !=
  1642				       journal->j_committing_transaction)) ||
  1643				     (jh->b_next_transaction != transaction))) {
  1644				printk(KERN_ERR "jbd2_journal_dirty_metadata: %s: "
  1645				       "bad jh for block %llu: "
  1646				       "transaction (%p, %u), "
  1647				       "jh->b_transaction (%p, %u), "
  1648				       "jh->b_next_transaction (%p, %u), jlist %u\n",
  1649				       journal->j_devname,
  1650				       (unsigned long long) bh->b_blocknr,
  1651				       transaction, transaction->t_tid,
  1652				       jh->b_transaction,
  1653				       jh->b_transaction ?
  1654				       jh->b_transaction->t_tid : 0,
  1655				       jh->b_next_transaction,
  1656				       jh->b_next_transaction ?
  1657				       jh->b_next_transaction->t_tid : 0,
  1658				       jh->b_jlist);
  1659				WARN_ON(1);
  1660				ret = -EINVAL;
  1661			}
  1662			/* And this case is illegal: we can't reuse another
  1663			 * transaction's data buffer, ever. */
  1664			goto out_unlock_bh;
  1665		}
  1666	
  1667		/* That test should have eliminated the following case: */
  1668		if (WARN_ON_ONCE(jh->b_frozen_data)) {
  1669			ret = -EINVAL;
  1670			goto out_unlock_bh;
  1671		}
  1672	
  1673		JBUFFER_TRACE(jh, "file as BJ_Metadata");
  1674		spin_lock(&journal->j_list_lock);
  1675		__jbd2_journal_file_buffer(jh, transaction, BJ_Metadata);
  1676		spin_unlock(&journal->j_list_lock);
  1677	out_unlock_bh:
  1678		spin_unlock(&jh->b_state_lock);
  1679	out:
  1680		JBUFFER_TRACE(jh, "exit");
  1681		return ret;
  1682	}
  1683	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-03  6:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03  0:55 [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage Milos Nikic
2026-03-03  0:55 ` [PATCH v3 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer Milos Nikic
2026-03-03  1:05   ` Andreas Dilger
2026-03-03  0:55 ` [PATCH v3 2/2] jbd2: gracefully abort on transaction state corruptions Milos Nikic
2026-03-03  6:16   ` kernel test robot
2026-03-03  2:11 ` [PATCH v3 0/2] jbd2: audit and convert legacy J_ASSERT usage yebin (H)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox