fs/ext4/super.c | 2 +- fs/jbd2/journal.c | 70 ++++++++++++++++++++++++++++++++------------ include/linux/jbd2.h | 6 ++-- 3 files changed, 55 insertions(+), 23 deletions(-)
syzbot is reporting possibility of deadlock due to sharing lock_class_key
for jbd2_handle across ext4 and ocfs2. But one disk partition can't have
two filesystems at the same time, and how locks in journal_t interacts
with other filesystem specific locks can vary depending on filesystems.
Therefore, lockdep should treat locks in journal_t different locks if
the filesystem which allocated the journal_t differs.
Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
---
Is assigning different lock_class_key for j_abort_mutex etc. correct
when filesystem differs?
Should we ignore possibility of jbd2 being used by filesystems other than
ext4 and ocfs2, and use "bool is_ext4" than "unsigned long fsmagic" ?
fs/ext4/super.c | 2 +-
fs/jbd2/journal.c | 70 ++++++++++++++++++++++++++++++++------------
include/linux/jbd2.h | 6 ++--
3 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 33e7c08c9529..4e206209a476 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5964,7 +5964,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
return ERR_CAST(bdev_file);
journal = jbd2_journal_init_dev(file_bdev(bdev_file), sb->s_bdev, j_start,
- j_len, sb->s_blocksize);
+ j_len, sb->s_blocksize, EXT4_SUPER_MAGIC);
if (IS_ERR(journal)) {
ext4_msg(sb, KERN_ERR, "failed to create device journal");
errno = PTR_ERR(journal);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index d480b94117cd..2f4fbd74cf76 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1517,11 +1517,13 @@ static int journal_load_superblock(journal_t *journal)
* superblock and initialize the journal_t object.
*/
-static journal_t *journal_init_common(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int blocksize)
+static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int blocksize,
+ unsigned long fsmagic)
{
- static struct lock_class_key jbd2_trans_commit_key;
+ static struct lock_class_key jbd2_trans_commit_key_ext4;
+ static struct lock_class_key jbd2_trans_commit_key_ocfs2;
+ static struct lock_class_key jbd2_trans_commit_key_unknown;
journal_t *journal;
int err;
int n;
@@ -1547,20 +1549,49 @@ static journal_t *journal_init_common(struct block_device *bdev,
init_waitqueue_head(&journal->j_wait_updates);
init_waitqueue_head(&journal->j_wait_reserved);
init_waitqueue_head(&journal->j_fc_wait);
- mutex_init(&journal->j_abort_mutex);
- mutex_init(&journal->j_barrier);
- mutex_init(&journal->j_checkpoint_mutex);
- spin_lock_init(&journal->j_revoke_lock);
- spin_lock_init(&journal->j_list_lock);
- spin_lock_init(&journal->j_history_lock);
- rwlock_init(&journal->j_state_lock);
+ if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
+ fsmagic == EXT4_SUPER_MAGIC) {
+ mutex_init(&journal->j_abort_mutex);
+ mutex_init(&journal->j_barrier);
+ mutex_init(&journal->j_checkpoint_mutex);
+ spin_lock_init(&journal->j_revoke_lock);
+ spin_lock_init(&journal->j_list_lock);
+ spin_lock_init(&journal->j_history_lock);
+ rwlock_init(&journal->j_state_lock);
+ } else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
+ fsmagic == OCFS2_SUPER_MAGIC) {
+ mutex_init(&journal->j_abort_mutex);
+ mutex_init(&journal->j_barrier);
+ mutex_init(&journal->j_checkpoint_mutex);
+ spin_lock_init(&journal->j_revoke_lock);
+ spin_lock_init(&journal->j_list_lock);
+ spin_lock_init(&journal->j_history_lock);
+ rwlock_init(&journal->j_state_lock);
+ } else {
+ mutex_init(&journal->j_abort_mutex);
+ mutex_init(&journal->j_barrier);
+ mutex_init(&journal->j_checkpoint_mutex);
+ spin_lock_init(&journal->j_revoke_lock);
+ spin_lock_init(&journal->j_list_lock);
+ spin_lock_init(&journal->j_history_lock);
+ rwlock_init(&journal->j_state_lock);
+ }
journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
journal->j_min_batch_time = 0;
journal->j_max_batch_time = 15000; /* 15ms */
atomic_set(&journal->j_reserved_credits, 0);
- lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
- &jbd2_trans_commit_key, 0);
+ if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
+ fsmagic == EXT4_SUPER_MAGIC)
+ lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ext4",
+ &jbd2_trans_commit_key_ext4, 0);
+ else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
+ fsmagic == OCFS2_SUPER_MAGIC)
+ lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ocfs2",
+ &jbd2_trans_commit_key_ocfs2, 0);
+ else
+ lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_unknown",
+ &jbd2_trans_commit_key_unknown, 0);
/* The journal is marked for error until we succeed with recovery! */
journal->j_flags = JBD2_ABORT;
@@ -1631,6 +1662,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
* @start: Block nr Start of journal.
* @len: Length of the journal in blocks.
* @blocksize: blocksize of journalling device
+ * @fsmagic: filesystem magic number for lockdep annotation
*
* Returns: a newly created journal_t *
*
@@ -1638,13 +1670,13 @@ static journal_t *journal_init_common(struct block_device *bdev,
* range of blocks on an arbitrary block device.
*
*/
-journal_t *jbd2_journal_init_dev(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int blocksize)
+journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int blocksize,
+ unsigned long fsmagic)
{
journal_t *journal;
- journal = journal_init_common(bdev, fs_dev, start, len, blocksize);
+ journal = journal_init_common(bdev, fs_dev, start, len, blocksize, fsmagic);
if (IS_ERR(journal))
return ERR_CAST(journal);
@@ -1682,8 +1714,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
inode->i_sb->s_blocksize_bits, inode->i_sb->s_blocksize);
journal = journal_init_common(inode->i_sb->s_bdev, inode->i_sb->s_bdev,
- blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
- inode->i_sb->s_blocksize);
+ blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
+ inode->i_sb->s_blocksize, inode->i_sb->s_magic);
if (IS_ERR(journal))
return ERR_CAST(journal);
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 43b9297fe8a7..56aa7ae4ec0b 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1523,9 +1523,9 @@ extern void jbd2_journal_unlock_updates (journal_t *);
void jbd2_journal_wait_updates(journal_t *);
-extern journal_t * jbd2_journal_init_dev(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int bsize);
+extern journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int bsize,
+ unsigned long fsmagic);
extern journal_t * jbd2_journal_init_inode (struct inode *);
extern int jbd2_journal_update_format (journal_t *);
extern int jbd2_journal_check_used_features
--
2.47.3On Sun 19-10-25 19:52:43, Tetsuo Handa wrote:
> syzbot is reporting possibility of deadlock due to sharing lock_class_key
> for jbd2_handle across ext4 and ocfs2. But one disk partition can't have
> two filesystems at the same time, and how locks in journal_t interacts
> with other filesystem specific locks can vary depending on filesystems.
> Therefore, lockdep should treat locks in journal_t different locks if
> the filesystem which allocated the journal_t differs.
>
> Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
Thanks for debugging this. I agree with the idea of your solution but the
implementation is just ugly. Just let the filesystem pass the lockdep key
into jbd2_journal_init_dev() / jbd2_journal_init_inode() and make ext4 and
ocfs2 functions initializing the journal each have its own lock_class_key
declared and pass it to jbd2 functions. That way changes are much simpler
and also jbd2 doesn't have to be aware about which filesystems are using
it.
Honza
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 33e7c08c9529..4e206209a476 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5964,7 +5964,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
> return ERR_CAST(bdev_file);
>
> journal = jbd2_journal_init_dev(file_bdev(bdev_file), sb->s_bdev, j_start,
> - j_len, sb->s_blocksize);
> + j_len, sb->s_blocksize, EXT4_SUPER_MAGIC);
> if (IS_ERR(journal)) {
> ext4_msg(sb, KERN_ERR, "failed to create device journal");
> errno = PTR_ERR(journal);
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..2f4fbd74cf76 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1517,11 +1517,13 @@ static int journal_load_superblock(journal_t *journal)
> * superblock and initialize the journal_t object.
> */
>
> -static journal_t *journal_init_common(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int blocksize)
> +static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int blocksize,
> + unsigned long fsmagic)
> {
> - static struct lock_class_key jbd2_trans_commit_key;
> + static struct lock_class_key jbd2_trans_commit_key_ext4;
> + static struct lock_class_key jbd2_trans_commit_key_ocfs2;
> + static struct lock_class_key jbd2_trans_commit_key_unknown;
> journal_t *journal;
> int err;
> int n;
> @@ -1547,20 +1549,49 @@ static journal_t *journal_init_common(struct block_device *bdev,
> init_waitqueue_head(&journal->j_wait_updates);
> init_waitqueue_head(&journal->j_wait_reserved);
> init_waitqueue_head(&journal->j_fc_wait);
> - mutex_init(&journal->j_abort_mutex);
> - mutex_init(&journal->j_barrier);
> - mutex_init(&journal->j_checkpoint_mutex);
> - spin_lock_init(&journal->j_revoke_lock);
> - spin_lock_init(&journal->j_list_lock);
> - spin_lock_init(&journal->j_history_lock);
> - rwlock_init(&journal->j_state_lock);
> + if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
> + fsmagic == EXT4_SUPER_MAGIC) {
> + mutex_init(&journal->j_abort_mutex);
> + mutex_init(&journal->j_barrier);
> + mutex_init(&journal->j_checkpoint_mutex);
> + spin_lock_init(&journal->j_revoke_lock);
> + spin_lock_init(&journal->j_list_lock);
> + spin_lock_init(&journal->j_history_lock);
> + rwlock_init(&journal->j_state_lock);
> + } else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
> + fsmagic == OCFS2_SUPER_MAGIC) {
> + mutex_init(&journal->j_abort_mutex);
> + mutex_init(&journal->j_barrier);
> + mutex_init(&journal->j_checkpoint_mutex);
> + spin_lock_init(&journal->j_revoke_lock);
> + spin_lock_init(&journal->j_list_lock);
> + spin_lock_init(&journal->j_history_lock);
> + rwlock_init(&journal->j_state_lock);
> + } else {
> + mutex_init(&journal->j_abort_mutex);
> + mutex_init(&journal->j_barrier);
> + mutex_init(&journal->j_checkpoint_mutex);
> + spin_lock_init(&journal->j_revoke_lock);
> + spin_lock_init(&journal->j_list_lock);
> + spin_lock_init(&journal->j_history_lock);
> + rwlock_init(&journal->j_state_lock);
> + }
>
> journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
> journal->j_min_batch_time = 0;
> journal->j_max_batch_time = 15000; /* 15ms */
> atomic_set(&journal->j_reserved_credits, 0);
> - lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
> - &jbd2_trans_commit_key, 0);
> + if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
> + fsmagic == EXT4_SUPER_MAGIC)
> + lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ext4",
> + &jbd2_trans_commit_key_ext4, 0);
> + else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
> + fsmagic == OCFS2_SUPER_MAGIC)
> + lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ocfs2",
> + &jbd2_trans_commit_key_ocfs2, 0);
> + else
> + lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_unknown",
> + &jbd2_trans_commit_key_unknown, 0);
>
> /* The journal is marked for error until we succeed with recovery! */
> journal->j_flags = JBD2_ABORT;
> @@ -1631,6 +1662,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> * @start: Block nr Start of journal.
> * @len: Length of the journal in blocks.
> * @blocksize: blocksize of journalling device
> + * @fsmagic: filesystem magic number for lockdep annotation
> *
> * Returns: a newly created journal_t *
> *
> @@ -1638,13 +1670,13 @@ static journal_t *journal_init_common(struct block_device *bdev,
> * range of blocks on an arbitrary block device.
> *
> */
> -journal_t *jbd2_journal_init_dev(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int blocksize)
> +journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int blocksize,
> + unsigned long fsmagic)
> {
> journal_t *journal;
>
> - journal = journal_init_common(bdev, fs_dev, start, len, blocksize);
> + journal = journal_init_common(bdev, fs_dev, start, len, blocksize, fsmagic);
> if (IS_ERR(journal))
> return ERR_CAST(journal);
>
> @@ -1682,8 +1714,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
> inode->i_sb->s_blocksize_bits, inode->i_sb->s_blocksize);
>
> journal = journal_init_common(inode->i_sb->s_bdev, inode->i_sb->s_bdev,
> - blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
> - inode->i_sb->s_blocksize);
> + blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
> + inode->i_sb->s_blocksize, inode->i_sb->s_magic);
> if (IS_ERR(journal))
> return ERR_CAST(journal);
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 43b9297fe8a7..56aa7ae4ec0b 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1523,9 +1523,9 @@ extern void jbd2_journal_unlock_updates (journal_t *);
>
> void jbd2_journal_wait_updates(journal_t *);
>
> -extern journal_t * jbd2_journal_init_dev(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int bsize);
> +extern journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int bsize,
> + unsigned long fsmagic);
> extern journal_t * jbd2_journal_init_inode (struct inode *);
> extern int jbd2_journal_update_format (journal_t *);
> extern int jbd2_journal_check_used_features
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On 2025/10/20 18:31, Jan Kara wrote:
> On Sun 19-10-25 19:52:43, Tetsuo Handa wrote:
>> syzbot is reporting possibility of deadlock due to sharing lock_class_key
>> for jbd2_handle across ext4 and ocfs2. But one disk partition can't have
>> two filesystems at the same time, and how locks in journal_t interacts
>> with other filesystem specific locks can vary depending on filesystems.
>> Therefore, lockdep should treat locks in journal_t different locks if
>> the filesystem which allocated the journal_t differs.
>
> Thanks for debugging this. I agree with the idea of your solution but the
> implementation is just ugly. Just let the filesystem pass the lockdep key
> into jbd2_journal_init_dev() / jbd2_journal_init_inode() and make ext4 and
> ocfs2 functions initializing the journal each have its own lock_class_key
> declared and pass it to jbd2 functions. That way changes are much simpler
> and also jbd2 doesn't have to be aware about which filesystems are using
> it.
Well, do you mean something like below diff? If we can assume that only ext4
and ocfs2 are using jbd2, the diff becomes smaller...
fs/ext4/super.c | 6 +++-
fs/jbd2/journal.c | 68 ++++++++++++++++++++++++++++++++++++---------------
fs/ocfs2/journal.c | 6 +++-
include/linux/jbd2.h | 15 ++++++++---
4 files changed, 67 insertions(+), 28 deletions(-)
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 43b9297fe8a7..184d025341da 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1287,6 +1287,13 @@ struct journal_s
int (*j_bmap)(struct journal_s *journal, sector_t *block);
};
+struct jbd2_lock_class_keys {
+ const char *name;
+ struct lock_class_key jbd2_trans_commit_key, j_abort_mutex, j_barrier,
+ j_checkpoint_mutex, j_revoke_lock, j_list_lock, j_history_lock,
+ j_state_lock;
+};
+
#define jbd2_might_wait_for_commit(j) \
do { \
rwsem_acquire(&j->j_trans_commit_map, 0, 0, _THIS_IP_); \
@@ -1523,10 +1530,10 @@ extern void jbd2_journal_unlock_updates (journal_t *);
void jbd2_journal_wait_updates(journal_t *);
-extern journal_t * jbd2_journal_init_dev(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int bsize);
-extern journal_t * jbd2_journal_init_inode (struct inode *);
+extern journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int bsize,
+ struct jbd2_lock_class_keys *keys);
+extern journal_t *jbd2_journal_init_inode(struct inode *inode, struct jbd2_lock_class_keys *keys);
extern int jbd2_journal_update_format (journal_t *);
extern int jbd2_journal_check_used_features
(journal_t *, unsigned long, unsigned long, unsigned long);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index d480b94117cd..775c1cfa452a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1506,6 +1506,34 @@ static int journal_load_superblock(journal_t *journal)
return 0;
}
+#ifndef CONFIG_RT
+/* This change goes to include/linux/spinlock.h */
+#ifdef CONFIG_DEBUG_SPINLOCK
+#define spin_lock_init_with_key(lock, key) \
+ __raw_spin_lock_init(spinlock_check(lock), #lock, key, LD_WAIT_CONFIG)
+#else
+#define spin_lock_init_with_key(lock, key) do { spin_lock_init(lock); (void)(key); } while (0)
+#endif
+#else
+/* This change goes to include/linux/spinlock_rt.h */
+#define spin_lock_init_with_key(slock, key) __spin_lock_init(slock, #slock, key, false)
+#endif
+
+#ifndef CONFIG_RT
+/* This change goes to include/linux/rwlock.h */
+#ifdef CONFIG_DEBUG_SPINLOCK
+#define rwlock_init_with_key(lock, key) __rwlock_init((lock), #lock, key)
+#else
+#define rwlock_init_with_key(lock, key) do { rwlock_init(lock); (void)(key); } while (0)
+#endif
+#else
+/* This change goes to include/linux/rwlock_rt.h */
+#define rwlock_init_with_key(rwl, key) \
+ do { \
+ init_rwbase_rt(&(rwl)->rwbase); \
+ __rt_rwlock_init(rwl, #rwl, key); \
+ } while (0)
+#endif
/*
* Management for journal control blocks: functions to create and
@@ -1517,11 +1545,10 @@ static int journal_load_superblock(journal_t *journal)
* superblock and initialize the journal_t object.
*/
-static journal_t *journal_init_common(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int blocksize)
+static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int blocksize,
+ struct jbd2_lock_class_keys *keys)
{
- static struct lock_class_key jbd2_trans_commit_key;
journal_t *journal;
int err;
int n;
@@ -1547,20 +1574,20 @@ static journal_t *journal_init_common(struct block_device *bdev,
init_waitqueue_head(&journal->j_wait_updates);
init_waitqueue_head(&journal->j_wait_reserved);
init_waitqueue_head(&journal->j_fc_wait);
- mutex_init(&journal->j_abort_mutex);
- mutex_init(&journal->j_barrier);
- mutex_init(&journal->j_checkpoint_mutex);
- spin_lock_init(&journal->j_revoke_lock);
- spin_lock_init(&journal->j_list_lock);
- spin_lock_init(&journal->j_history_lock);
- rwlock_init(&journal->j_state_lock);
+ mutex_init_with_key(&journal->j_abort_mutex, &keys->j_abort_mutex);
+ mutex_init_with_key(&journal->j_barrier, &keys->j_barrier);
+ mutex_init_with_key(&journal->j_checkpoint_mutex, &keys->j_checkpoint_mutex);
+ spin_lock_init_with_key(&journal->j_revoke_lock, &keys->j_revoke_lock);
+ spin_lock_init_with_key(&journal->j_list_lock, &keys->j_list_lock);
+ spin_lock_init_with_key(&journal->j_history_lock, &keys->j_history_lock);
+ rwlock_init_with_key(&journal->j_state_lock, &keys->j_state_lock);
journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
journal->j_min_batch_time = 0;
journal->j_max_batch_time = 15000; /* 15ms */
atomic_set(&journal->j_reserved_credits, 0);
- lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
- &jbd2_trans_commit_key, 0);
+ lockdep_init_map(&journal->j_trans_commit_map, keys->name,
+ &keys->jbd2_trans_commit_key, 0);
/* The journal is marked for error until we succeed with recovery! */
journal->j_flags = JBD2_ABORT;
@@ -1631,6 +1658,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
* @start: Block nr Start of journal.
* @len: Length of the journal in blocks.
* @blocksize: blocksize of journalling device
+ * @keys: Pointer to struct jbd2_lock_class_keys for lockdep annotation
*
* Returns: a newly created journal_t *
*
@@ -1638,13 +1666,13 @@ static journal_t *journal_init_common(struct block_device *bdev,
* range of blocks on an arbitrary block device.
*
*/
-journal_t *jbd2_journal_init_dev(struct block_device *bdev,
- struct block_device *fs_dev,
- unsigned long long start, int len, int blocksize)
+journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
+ unsigned long long start, int len, int blocksize,
+ struct jbd2_lock_class_keys *keys)
{
journal_t *journal;
- journal = journal_init_common(bdev, fs_dev, start, len, blocksize);
+ journal = journal_init_common(bdev, fs_dev, start, len, blocksize, keys);
if (IS_ERR(journal))
return ERR_CAST(journal);
@@ -1664,7 +1692,7 @@ journal_t *jbd2_journal_init_dev(struct block_device *bdev,
* the journal. The inode must exist already, must support bmap() and
* must have all data blocks preallocated.
*/
-journal_t *jbd2_journal_init_inode(struct inode *inode)
+journal_t *jbd2_journal_init_inode(struct inode *inode, struct jbd2_lock_class_keys *keys)
{
journal_t *journal;
sector_t blocknr;
@@ -1682,8 +1710,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
inode->i_sb->s_blocksize_bits, inode->i_sb->s_blocksize);
journal = journal_init_common(inode->i_sb->s_bdev, inode->i_sb->s_bdev,
- blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
- inode->i_sb->s_blocksize);
+ blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
+ inode->i_sb->s_blocksize, keys);
if (IS_ERR(journal))
return ERR_CAST(journal);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 699c15db28a8..fc93a2ef5c2d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -60,6 +60,8 @@
#define CREATE_TRACE_POINTS
#include <trace/events/ext4.h>
+static struct jbd2_lock_class_keys jbd2_ext4_lock_class_keys = { .name = "jbd2_ext4_handle" };
+
static struct ext4_lazy_init *ext4_li_info;
static DEFINE_MUTEX(ext4_li_mtx);
static struct ratelimit_state ext4_mount_msg_ratelimit;
@@ -5851,7 +5853,7 @@ static journal_t *ext4_open_inode_journal(struct super_block *sb,
if (IS_ERR(journal_inode))
return ERR_CAST(journal_inode);
- journal = jbd2_journal_init_inode(journal_inode);
+ journal = jbd2_journal_init_inode(journal_inode, &jbd2_ext4_lock_class_keys);
if (IS_ERR(journal)) {
ext4_msg(sb, KERN_ERR, "Could not load journal inode");
iput(journal_inode);
@@ -5956,7 +5958,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
return ERR_CAST(bdev_file);
journal = jbd2_journal_init_dev(file_bdev(bdev_file), sb->s_bdev, j_start,
- j_len, sb->s_blocksize);
+ j_len, sb->s_blocksize, &jbd2_ext4_lock_class_keys);
if (IS_ERR(journal)) {
ext4_msg(sb, KERN_ERR, "failed to create device journal");
errno = PTR_ERR(journal);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index e5f58ff2175f..e92746e3cb0b 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -43,6 +43,8 @@
DEFINE_SPINLOCK(trans_inc_lock);
+static struct jbd2_lock_class_keys jbd2_ocfs2_lock_class_keys = { .name = "jbd2_ocfs2_handle" };
+
#define ORPHAN_SCAN_SCHEDULE_TIMEOUT 300000
static int ocfs2_force_read_journal(struct inode *inode);
@@ -968,7 +970,7 @@ int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
OCFS2_I(inode)->ip_clusters);
/* call the kernels journal init function now */
- j_journal = jbd2_journal_init_inode(inode);
+ j_journal = jbd2_journal_init_inode(inode, &jbd2_ocfs2_lock_class_keys);
if (IS_ERR(j_journal)) {
mlog(ML_ERROR, "Linux journal layer error\n");
status = PTR_ERR(j_journal);
@@ -1759,7 +1761,7 @@ static int ocfs2_replay_journal(struct ocfs2_super *osb,
goto done;
}
- journal = jbd2_journal_init_inode(inode);
+ journal = jbd2_journal_init_inode(inode, &jbd2_ocfs2_lock_class_keys);
if (IS_ERR(journal)) {
mlog(ML_ERROR, "Linux journal layer error\n");
status = PTR_ERR(journal);
On Mon 20-10-25 20:28:54, Tetsuo Handa wrote:
> On 2025/10/20 18:31, Jan Kara wrote:
> > On Sun 19-10-25 19:52:43, Tetsuo Handa wrote:
> >> syzbot is reporting possibility of deadlock due to sharing lock_class_key
> >> for jbd2_handle across ext4 and ocfs2. But one disk partition can't have
> >> two filesystems at the same time, and how locks in journal_t interacts
> >> with other filesystem specific locks can vary depending on filesystems.
> >> Therefore, lockdep should treat locks in journal_t different locks if
> >> the filesystem which allocated the journal_t differs.
> >
> > Thanks for debugging this. I agree with the idea of your solution but the
> > implementation is just ugly. Just let the filesystem pass the lockdep key
> > into jbd2_journal_init_dev() / jbd2_journal_init_inode() and make ext4 and
> > ocfs2 functions initializing the journal each have its own lock_class_key
> > declared and pass it to jbd2 functions. That way changes are much simpler
> > and also jbd2 doesn't have to be aware about which filesystems are using
> > it.
>
> Well, do you mean something like below diff? If we can assume that only ext4
> and ocfs2 are using jbd2, the diff becomes smaller...
Yes, something like this. In fact, I think we could get away with just
jbd2_trans_commit_key. There's definitely no need for j_revoke_lock,
j_list_lock, j_history_lock, j_state_lock, j_abort_mutex keys as these are
internal to jbd2. j_checkpoint_mutex and j_barrier do wrap around some
filesystem code so maybe we'll need to specify keys for them but I'd start
with just jbd2_trans_commit_key and wait whether syzbot manages to trigger
another false positive report with that.
Honza
>
> fs/ext4/super.c | 6 +++-
> fs/jbd2/journal.c | 68 ++++++++++++++++++++++++++++++++++++---------------
> fs/ocfs2/journal.c | 6 +++-
> include/linux/jbd2.h | 15 ++++++++---
> 4 files changed, 67 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 43b9297fe8a7..184d025341da 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1287,6 +1287,13 @@ struct journal_s
> int (*j_bmap)(struct journal_s *journal, sector_t *block);
> };
>
> +struct jbd2_lock_class_keys {
> + const char *name;
> + struct lock_class_key jbd2_trans_commit_key, j_abort_mutex, j_barrier,
> + j_checkpoint_mutex, j_revoke_lock, j_list_lock, j_history_lock,
> + j_state_lock;
> +};
> +
> #define jbd2_might_wait_for_commit(j) \
> do { \
> rwsem_acquire(&j->j_trans_commit_map, 0, 0, _THIS_IP_); \
> @@ -1523,10 +1530,10 @@ extern void jbd2_journal_unlock_updates (journal_t *);
>
> void jbd2_journal_wait_updates(journal_t *);
>
> -extern journal_t * jbd2_journal_init_dev(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int bsize);
> -extern journal_t * jbd2_journal_init_inode (struct inode *);
> +extern journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int bsize,
> + struct jbd2_lock_class_keys *keys);
> +extern journal_t *jbd2_journal_init_inode(struct inode *inode, struct jbd2_lock_class_keys *keys);
> extern int jbd2_journal_update_format (journal_t *);
> extern int jbd2_journal_check_used_features
> (journal_t *, unsigned long, unsigned long, unsigned long);
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..775c1cfa452a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1506,6 +1506,34 @@ static int journal_load_superblock(journal_t *journal)
> return 0;
> }
>
> +#ifndef CONFIG_RT
> +/* This change goes to include/linux/spinlock.h */
> +#ifdef CONFIG_DEBUG_SPINLOCK
> +#define spin_lock_init_with_key(lock, key) \
> + __raw_spin_lock_init(spinlock_check(lock), #lock, key, LD_WAIT_CONFIG)
> +#else
> +#define spin_lock_init_with_key(lock, key) do { spin_lock_init(lock); (void)(key); } while (0)
> +#endif
> +#else
> +/* This change goes to include/linux/spinlock_rt.h */
> +#define spin_lock_init_with_key(slock, key) __spin_lock_init(slock, #slock, key, false)
> +#endif
> +
> +#ifndef CONFIG_RT
> +/* This change goes to include/linux/rwlock.h */
> +#ifdef CONFIG_DEBUG_SPINLOCK
> +#define rwlock_init_with_key(lock, key) __rwlock_init((lock), #lock, key)
> +#else
> +#define rwlock_init_with_key(lock, key) do { rwlock_init(lock); (void)(key); } while (0)
> +#endif
> +#else
> +/* This change goes to include/linux/rwlock_rt.h */
> +#define rwlock_init_with_key(rwl, key) \
> + do { \
> + init_rwbase_rt(&(rwl)->rwbase); \
> + __rt_rwlock_init(rwl, #rwl, key); \
> + } while (0)
> +#endif
>
> /*
> * Management for journal control blocks: functions to create and
> @@ -1517,11 +1545,10 @@ static int journal_load_superblock(journal_t *journal)
> * superblock and initialize the journal_t object.
> */
>
> -static journal_t *journal_init_common(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int blocksize)
> +static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int blocksize,
> + struct jbd2_lock_class_keys *keys)
> {
> - static struct lock_class_key jbd2_trans_commit_key;
> journal_t *journal;
> int err;
> int n;
> @@ -1547,20 +1574,20 @@ static journal_t *journal_init_common(struct block_device *bdev,
> init_waitqueue_head(&journal->j_wait_updates);
> init_waitqueue_head(&journal->j_wait_reserved);
> init_waitqueue_head(&journal->j_fc_wait);
> - mutex_init(&journal->j_abort_mutex);
> - mutex_init(&journal->j_barrier);
> - mutex_init(&journal->j_checkpoint_mutex);
> - spin_lock_init(&journal->j_revoke_lock);
> - spin_lock_init(&journal->j_list_lock);
> - spin_lock_init(&journal->j_history_lock);
> - rwlock_init(&journal->j_state_lock);
> + mutex_init_with_key(&journal->j_abort_mutex, &keys->j_abort_mutex);
> + mutex_init_with_key(&journal->j_barrier, &keys->j_barrier);
> + mutex_init_with_key(&journal->j_checkpoint_mutex, &keys->j_checkpoint_mutex);
> + spin_lock_init_with_key(&journal->j_revoke_lock, &keys->j_revoke_lock);
> + spin_lock_init_with_key(&journal->j_list_lock, &keys->j_list_lock);
> + spin_lock_init_with_key(&journal->j_history_lock, &keys->j_history_lock);
> + rwlock_init_with_key(&journal->j_state_lock, &keys->j_state_lock);
>
> journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
> journal->j_min_batch_time = 0;
> journal->j_max_batch_time = 15000; /* 15ms */
> atomic_set(&journal->j_reserved_credits, 0);
> - lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
> - &jbd2_trans_commit_key, 0);
> + lockdep_init_map(&journal->j_trans_commit_map, keys->name,
> + &keys->jbd2_trans_commit_key, 0);
>
> /* The journal is marked for error until we succeed with recovery! */
> journal->j_flags = JBD2_ABORT;
> @@ -1631,6 +1658,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> * @start: Block nr Start of journal.
> * @len: Length of the journal in blocks.
> * @blocksize: blocksize of journalling device
> + * @keys: Pointer to struct jbd2_lock_class_keys for lockdep annotation
> *
> * Returns: a newly created journal_t *
> *
> @@ -1638,13 +1666,13 @@ static journal_t *journal_init_common(struct block_device *bdev,
> * range of blocks on an arbitrary block device.
> *
> */
> -journal_t *jbd2_journal_init_dev(struct block_device *bdev,
> - struct block_device *fs_dev,
> - unsigned long long start, int len, int blocksize)
> +journal_t *jbd2_journal_init_dev(struct block_device *bdev, struct block_device *fs_dev,
> + unsigned long long start, int len, int blocksize,
> + struct jbd2_lock_class_keys *keys)
> {
> journal_t *journal;
>
> - journal = journal_init_common(bdev, fs_dev, start, len, blocksize);
> + journal = journal_init_common(bdev, fs_dev, start, len, blocksize, keys);
> if (IS_ERR(journal))
> return ERR_CAST(journal);
>
> @@ -1664,7 +1692,7 @@ journal_t *jbd2_journal_init_dev(struct block_device *bdev,
> * the journal. The inode must exist already, must support bmap() and
> * must have all data blocks preallocated.
> */
> -journal_t *jbd2_journal_init_inode(struct inode *inode)
> +journal_t *jbd2_journal_init_inode(struct inode *inode, struct jbd2_lock_class_keys *keys)
> {
> journal_t *journal;
> sector_t blocknr;
> @@ -1682,8 +1710,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
> inode->i_sb->s_blocksize_bits, inode->i_sb->s_blocksize);
>
> journal = journal_init_common(inode->i_sb->s_bdev, inode->i_sb->s_bdev,
> - blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
> - inode->i_sb->s_blocksize);
> + blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
> + inode->i_sb->s_blocksize, keys);
> if (IS_ERR(journal))
> return ERR_CAST(journal);
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 699c15db28a8..fc93a2ef5c2d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -60,6 +60,8 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/ext4.h>
>
> +static struct jbd2_lock_class_keys jbd2_ext4_lock_class_keys = { .name = "jbd2_ext4_handle" };
> +
> static struct ext4_lazy_init *ext4_li_info;
> static DEFINE_MUTEX(ext4_li_mtx);
> static struct ratelimit_state ext4_mount_msg_ratelimit;
> @@ -5851,7 +5853,7 @@ static journal_t *ext4_open_inode_journal(struct super_block *sb,
> if (IS_ERR(journal_inode))
> return ERR_CAST(journal_inode);
>
> - journal = jbd2_journal_init_inode(journal_inode);
> + journal = jbd2_journal_init_inode(journal_inode, &jbd2_ext4_lock_class_keys);
> if (IS_ERR(journal)) {
> ext4_msg(sb, KERN_ERR, "Could not load journal inode");
> iput(journal_inode);
> @@ -5956,7 +5958,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
> return ERR_CAST(bdev_file);
>
> journal = jbd2_journal_init_dev(file_bdev(bdev_file), sb->s_bdev, j_start,
> - j_len, sb->s_blocksize);
> + j_len, sb->s_blocksize, &jbd2_ext4_lock_class_keys);
> if (IS_ERR(journal)) {
> ext4_msg(sb, KERN_ERR, "failed to create device journal");
> errno = PTR_ERR(journal);
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index e5f58ff2175f..e92746e3cb0b 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -43,6 +43,8 @@
>
> DEFINE_SPINLOCK(trans_inc_lock);
>
> +static struct jbd2_lock_class_keys jbd2_ocfs2_lock_class_keys = { .name = "jbd2_ocfs2_handle" };
> +
> #define ORPHAN_SCAN_SCHEDULE_TIMEOUT 300000
>
> static int ocfs2_force_read_journal(struct inode *inode);
> @@ -968,7 +970,7 @@ int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
> OCFS2_I(inode)->ip_clusters);
>
> /* call the kernels journal init function now */
> - j_journal = jbd2_journal_init_inode(inode);
> + j_journal = jbd2_journal_init_inode(inode, &jbd2_ocfs2_lock_class_keys);
> if (IS_ERR(j_journal)) {
> mlog(ML_ERROR, "Linux journal layer error\n");
> status = PTR_ERR(j_journal);
> @@ -1759,7 +1761,7 @@ static int ocfs2_replay_journal(struct ocfs2_super *osb,
> goto done;
> }
>
> - journal = jbd2_journal_init_inode(inode);
> + journal = jbd2_journal_init_inode(inode, &jbd2_ocfs2_lock_class_keys);
> if (IS_ERR(journal)) {
> mlog(ML_ERROR, "Linux journal layer error\n");
> status = PTR_ERR(journal);
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On 2025/10/20 21:55, Jan Kara wrote: > On Mon 20-10-25 20:28:54, Tetsuo Handa wrote: >> On 2025/10/20 18:31, Jan Kara wrote: >>> On Sun 19-10-25 19:52:43, Tetsuo Handa wrote: >>>> syzbot is reporting possibility of deadlock due to sharing lock_class_key >>>> for jbd2_handle across ext4 and ocfs2. But one disk partition can't have >>>> two filesystems at the same time, and how locks in journal_t interacts >>>> with other filesystem specific locks can vary depending on filesystems. >>>> Therefore, lockdep should treat locks in journal_t different locks if >>>> the filesystem which allocated the journal_t differs. >>> >>> Thanks for debugging this. I agree with the idea of your solution but the >>> implementation is just ugly. Just let the filesystem pass the lockdep key >>> into jbd2_journal_init_dev() / jbd2_journal_init_inode() and make ext4 and >>> ocfs2 functions initializing the journal each have its own lock_class_key >>> declared and pass it to jbd2 functions. That way changes are much simpler >>> and also jbd2 doesn't have to be aware about which filesystems are using >>> it. >> >> Well, do you mean something like below diff? If we can assume that only ext4 >> and ocfs2 are using jbd2, the diff becomes smaller... > > Yes, something like this. In fact, I think we could get away with just > jbd2_trans_commit_key. There's definitely no need for j_revoke_lock, > j_list_lock, j_history_lock, j_state_lock, j_abort_mutex keys as these are > internal to jbd2. j_checkpoint_mutex and j_barrier do wrap around some > filesystem code so maybe we'll need to specify keys for them but I'd start > with just jbd2_trans_commit_key and wait whether syzbot manages to trigger > another false positive report with that. I tried https://syzkaller.appspot.com/x/patch.diff?x=11b4dde2580000 . But I think https://syzkaller.appspot.com/x/patch.diff?x=1644c3cd980000 pattern which all mutex_init_with_key() users follow seems more simpler and easier to apply. What do you think?
On Tue 21-10-25 20:16:53, Tetsuo Handa wrote: > On 2025/10/20 21:55, Jan Kara wrote: > > On Mon 20-10-25 20:28:54, Tetsuo Handa wrote: > >> On 2025/10/20 18:31, Jan Kara wrote: > >>> On Sun 19-10-25 19:52:43, Tetsuo Handa wrote: > >>>> syzbot is reporting possibility of deadlock due to sharing lock_class_key > >>>> for jbd2_handle across ext4 and ocfs2. But one disk partition can't have > >>>> two filesystems at the same time, and how locks in journal_t interacts > >>>> with other filesystem specific locks can vary depending on filesystems. > >>>> Therefore, lockdep should treat locks in journal_t different locks if > >>>> the filesystem which allocated the journal_t differs. > >>> > >>> Thanks for debugging this. I agree with the idea of your solution but the > >>> implementation is just ugly. Just let the filesystem pass the lockdep key > >>> into jbd2_journal_init_dev() / jbd2_journal_init_inode() and make ext4 and > >>> ocfs2 functions initializing the journal each have its own lock_class_key > >>> declared and pass it to jbd2 functions. That way changes are much simpler > >>> and also jbd2 doesn't have to be aware about which filesystems are using > >>> it. > >> > >> Well, do you mean something like below diff? If we can assume that only ext4 > >> and ocfs2 are using jbd2, the diff becomes smaller... > > > > Yes, something like this. In fact, I think we could get away with just > > jbd2_trans_commit_key. There's definitely no need for j_revoke_lock, > > j_list_lock, j_history_lock, j_state_lock, j_abort_mutex keys as these are > > internal to jbd2. j_checkpoint_mutex and j_barrier do wrap around some > > filesystem code so maybe we'll need to specify keys for them but I'd start > > with just jbd2_trans_commit_key and wait whether syzbot manages to trigger > > another false positive report with that. > > I tried https://syzkaller.appspot.com/x/patch.diff?x=11b4dde2580000 . > But I think https://syzkaller.appspot.com/x/patch.diff?x=1644c3cd980000 > pattern which all mutex_init_with_key() users follow seems more simpler > and easier to apply. What do you think? Yes, the second version looks nicer. Thanks! BTW, did you verify that annotating j_barrier, j_checkpoint_mutex, and j_abort_mutex is really needed? Because I'd be slightly surprised if it really was... Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
On 2025/10/21 22:39, Jan Kara wrote: >>> Yes, something like this. In fact, I think we could get away with just >>> jbd2_trans_commit_key. There's definitely no need for j_revoke_lock, >>> j_list_lock, j_history_lock, j_state_lock, j_abort_mutex keys as these are >>> internal to jbd2. j_checkpoint_mutex and j_barrier do wrap around some >>> filesystem code so maybe we'll need to specify keys for them but I'd start >>> with just jbd2_trans_commit_key and wait whether syzbot manages to trigger >>> another false positive report with that. >> >> I tried https://syzkaller.appspot.com/x/patch.diff?x=11b4dde2580000 . >> But I think https://syzkaller.appspot.com/x/patch.diff?x=1644c3cd980000 >> pattern which all mutex_init_with_key() users follow seems more simpler >> and easier to apply. What do you think? > > Yes, the second version looks nicer. Thanks! BTW, did you verify that > annotating j_barrier, j_checkpoint_mutex, and j_abort_mutex is really > needed? Because I'd be slightly surprised if it really was... No. https://syzkaller.appspot.com/x/patch.diff?x=13a94e7c580000 was sufficient for this specific report. But I don't know what will happen with ocfs2 which has so complicated locking dependency ( https://syzkaller.appspot.com/upstream/s/ocfs2 has currently 27 open "possible deadlock" reports). Do you want me to try this minimal change in linux-next via my tree for a while? Or do you want to just apply this minimal change first?
On Wed 22-10-25 19:04:33, Tetsuo Handa wrote: > On 2025/10/21 22:39, Jan Kara wrote: > >>> Yes, something like this. In fact, I think we could get away with just > >>> jbd2_trans_commit_key. There's definitely no need for j_revoke_lock, > >>> j_list_lock, j_history_lock, j_state_lock, j_abort_mutex keys as these are > >>> internal to jbd2. j_checkpoint_mutex and j_barrier do wrap around some > >>> filesystem code so maybe we'll need to specify keys for them but I'd start > >>> with just jbd2_trans_commit_key and wait whether syzbot manages to trigger > >>> another false positive report with that. > >> > >> I tried https://syzkaller.appspot.com/x/patch.diff?x=11b4dde2580000 . > >> But I think https://syzkaller.appspot.com/x/patch.diff?x=1644c3cd980000 > >> pattern which all mutex_init_with_key() users follow seems more simpler > >> and easier to apply. What do you think? > > > > Yes, the second version looks nicer. Thanks! BTW, did you verify that > > annotating j_barrier, j_checkpoint_mutex, and j_abort_mutex is really > > needed? Because I'd be slightly surprised if it really was... > > No. https://syzkaller.appspot.com/x/patch.diff?x=13a94e7c580000 was sufficient > for this specific report. But I don't know what will happen with ocfs2 which has > so complicated locking dependency ( https://syzkaller.appspot.com/upstream/s/ocfs2 > has currently 27 open "possible deadlock" reports). > > Do you want me to try this minimal change in linux-next via my tree for a while? > Or do you want to just apply this minimal change first? Please just submit the minimal change and we'll merge it through ext4 tree. If syzbot manages to trigger more reports, we can always add more complications :). Thanks! Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
syzbot is reporting possibility of deadlock due to sharing lock_class_key
for jbd2_handle across ext4 and ocfs2. But this is a false positive, for
one disk partition can't have two filesystems at the same time.
Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
---
fs/jbd2/journal.c | 6 ++++--
include/linux/jbd2.h | 6 ++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index d480b94117cd..f43474002f50 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1521,7 +1521,6 @@ static journal_t *journal_init_common(struct block_device *bdev,
struct block_device *fs_dev,
unsigned long long start, int len, int blocksize)
{
- static struct lock_class_key jbd2_trans_commit_key;
journal_t *journal;
int err;
int n;
@@ -1530,6 +1529,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
if (!journal)
return ERR_PTR(-ENOMEM);
+ lockdep_register_key(&journal->jbd2_trans_commit_key);
journal->j_blocksize = blocksize;
journal->j_dev = bdev;
journal->j_fs_dev = fs_dev;
@@ -1560,7 +1560,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
journal->j_max_batch_time = 15000; /* 15ms */
atomic_set(&journal->j_reserved_credits, 0);
lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
- &jbd2_trans_commit_key, 0);
+ &journal->jbd2_trans_commit_key, 0);
/* The journal is marked for error until we succeed with recovery! */
journal->j_flags = JBD2_ABORT;
@@ -1611,6 +1611,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
kfree(journal->j_wbuf);
jbd2_journal_destroy_revoke(journal);
journal_fail_superblock(journal);
+ lockdep_unregister_key(&journal->jbd2_trans_commit_key);
kfree(journal);
return ERR_PTR(err);
}
@@ -2187,6 +2188,7 @@ int jbd2_journal_destroy(journal_t *journal)
jbd2_journal_destroy_revoke(journal);
kfree(journal->j_fc_wbuf);
kfree(journal->j_wbuf);
+ lockdep_unregister_key(&journal->jbd2_trans_commit_key);
kfree(journal);
return err;
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 43b9297fe8a7..f5eaf76198f3 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1253,6 +1253,12 @@ struct journal_s
*/
struct lockdep_map j_trans_commit_map;
#endif
+ /**
+ * @jbd2_trans_commit_key:
+ *
+ * "struct lock_class_key" for @j_trans_commit_map
+ */
+ struct lock_class_key jbd2_trans_commit_key;
/**
* @j_fc_cleanup_callback:
--
2.47.3
On Wed, 22 Oct 2025 20:11:37 +0900, Tetsuo Handa wrote:
> syzbot is reporting possibility of deadlock due to sharing lock_class_key
> for jbd2_handle across ext4 and ocfs2. But this is a false positive, for
> one disk partition can't have two filesystems at the same time.
>
>
Applied, thanks!
[1/1] jbd2: allocate lock_class_key for jbd2_handle dynamically
commit: 524c3853831cf4f7e1db579e487c757c3065165c
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
On Wed 22-10-25 20:11:37, Tetsuo Handa wrote:
> syzbot is reporting possibility of deadlock due to sharing lock_class_key
> for jbd2_handle across ext4 and ocfs2. But this is a false positive, for
> one disk partition can't have two filesystems at the same time.
>
> Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
Thanks! Tha patch looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/jbd2/journal.c | 6 ++++--
> include/linux/jbd2.h | 6 ++++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index d480b94117cd..f43474002f50 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1521,7 +1521,6 @@ static journal_t *journal_init_common(struct block_device *bdev,
> struct block_device *fs_dev,
> unsigned long long start, int len, int blocksize)
> {
> - static struct lock_class_key jbd2_trans_commit_key;
> journal_t *journal;
> int err;
> int n;
> @@ -1530,6 +1529,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> if (!journal)
> return ERR_PTR(-ENOMEM);
>
> + lockdep_register_key(&journal->jbd2_trans_commit_key);
> journal->j_blocksize = blocksize;
> journal->j_dev = bdev;
> journal->j_fs_dev = fs_dev;
> @@ -1560,7 +1560,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> journal->j_max_batch_time = 15000; /* 15ms */
> atomic_set(&journal->j_reserved_credits, 0);
> lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
> - &jbd2_trans_commit_key, 0);
> + &journal->jbd2_trans_commit_key, 0);
>
> /* The journal is marked for error until we succeed with recovery! */
> journal->j_flags = JBD2_ABORT;
> @@ -1611,6 +1611,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> kfree(journal->j_wbuf);
> jbd2_journal_destroy_revoke(journal);
> journal_fail_superblock(journal);
> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
> kfree(journal);
> return ERR_PTR(err);
> }
> @@ -2187,6 +2188,7 @@ int jbd2_journal_destroy(journal_t *journal)
> jbd2_journal_destroy_revoke(journal);
> kfree(journal->j_fc_wbuf);
> kfree(journal->j_wbuf);
> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
> kfree(journal);
>
> return err;
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 43b9297fe8a7..f5eaf76198f3 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1253,6 +1253,12 @@ struct journal_s
> */
> struct lockdep_map j_trans_commit_map;
> #endif
> + /**
> + * @jbd2_trans_commit_key:
> + *
> + * "struct lock_class_key" for @j_trans_commit_map
> + */
> + struct lock_class_key jbd2_trans_commit_key;
>
> /**
> * @j_fc_cleanup_callback:
> --
> 2.47.3
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
Who can take this patch?
On 2025/10/23 16:59, Jan Kara wrote:
> On Wed 22-10-25 20:11:37, Tetsuo Handa wrote:
>> syzbot is reporting possibility of deadlock due to sharing lock_class_key
>> for jbd2_handle across ext4 and ocfs2. But this is a false positive, for
>> one disk partition can't have two filesystems at the same time.
>>
>> Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
>
> Thanks! Tha patch looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Honza
>
>> ---
>> fs/jbd2/journal.c | 6 ++++--
>> include/linux/jbd2.h | 6 ++++++
>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index d480b94117cd..f43474002f50 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -1521,7 +1521,6 @@ static journal_t *journal_init_common(struct block_device *bdev,
>> struct block_device *fs_dev,
>> unsigned long long start, int len, int blocksize)
>> {
>> - static struct lock_class_key jbd2_trans_commit_key;
>> journal_t *journal;
>> int err;
>> int n;
>> @@ -1530,6 +1529,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>> if (!journal)
>> return ERR_PTR(-ENOMEM);
>>
>> + lockdep_register_key(&journal->jbd2_trans_commit_key);
>> journal->j_blocksize = blocksize;
>> journal->j_dev = bdev;
>> journal->j_fs_dev = fs_dev;
>> @@ -1560,7 +1560,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>> journal->j_max_batch_time = 15000; /* 15ms */
>> atomic_set(&journal->j_reserved_credits, 0);
>> lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
>> - &jbd2_trans_commit_key, 0);
>> + &journal->jbd2_trans_commit_key, 0);
>>
>> /* The journal is marked for error until we succeed with recovery! */
>> journal->j_flags = JBD2_ABORT;
>> @@ -1611,6 +1611,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>> kfree(journal->j_wbuf);
>> jbd2_journal_destroy_revoke(journal);
>> journal_fail_superblock(journal);
>> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
>> kfree(journal);
>> return ERR_PTR(err);
>> }
>> @@ -2187,6 +2188,7 @@ int jbd2_journal_destroy(journal_t *journal)
>> jbd2_journal_destroy_revoke(journal);
>> kfree(journal->j_fc_wbuf);
>> kfree(journal->j_wbuf);
>> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
>> kfree(journal);
>>
>> return err;
>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>> index 43b9297fe8a7..f5eaf76198f3 100644
>> --- a/include/linux/jbd2.h
>> +++ b/include/linux/jbd2.h
>> @@ -1253,6 +1253,12 @@ struct journal_s
>> */
>> struct lockdep_map j_trans_commit_map;
>> #endif
>> + /**
>> + * @jbd2_trans_commit_key:
>> + *
>> + * "struct lock_class_key" for @j_trans_commit_map
>> + */
>> + struct lock_class_key jbd2_trans_commit_key;
>>
>> /**
>> * @j_fc_cleanup_callback:
>> --
>> 2.47.3
>>
>>
On Fri 31-10-25 20:28:34, Tetsuo Handa wrote:
> Who can take this patch?
Ted should pick it up. He's in CC so I'm sure he'll eventually get to it
when gathering patches for the next merge window.
Honza
>
> On 2025/10/23 16:59, Jan Kara wrote:
> > On Wed 22-10-25 20:11:37, Tetsuo Handa wrote:
> >> syzbot is reporting possibility of deadlock due to sharing lock_class_key
> >> for jbd2_handle across ext4 and ocfs2. But this is a false positive, for
> >> one disk partition can't have two filesystems at the same time.
> >>
> >> Reported-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
> >> Closes: https://syzkaller.appspot.com/bug?extid=6e493c165d26d6fcbf72
> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >> Tested-by: syzbot+6e493c165d26d6fcbf72@syzkaller.appspotmail.com
> >
> > Thanks! Tha patch looks good. Feel free to add:
> >
> > Reviewed-by: Jan Kara <jack@suse.cz>
> >
> > Honza
> >
> >> ---
> >> fs/jbd2/journal.c | 6 ++++--
> >> include/linux/jbd2.h | 6 ++++++
> >> 2 files changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> >> index d480b94117cd..f43474002f50 100644
> >> --- a/fs/jbd2/journal.c
> >> +++ b/fs/jbd2/journal.c
> >> @@ -1521,7 +1521,6 @@ static journal_t *journal_init_common(struct block_device *bdev,
> >> struct block_device *fs_dev,
> >> unsigned long long start, int len, int blocksize)
> >> {
> >> - static struct lock_class_key jbd2_trans_commit_key;
> >> journal_t *journal;
> >> int err;
> >> int n;
> >> @@ -1530,6 +1529,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> >> if (!journal)
> >> return ERR_PTR(-ENOMEM);
> >>
> >> + lockdep_register_key(&journal->jbd2_trans_commit_key);
> >> journal->j_blocksize = blocksize;
> >> journal->j_dev = bdev;
> >> journal->j_fs_dev = fs_dev;
> >> @@ -1560,7 +1560,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> >> journal->j_max_batch_time = 15000; /* 15ms */
> >> atomic_set(&journal->j_reserved_credits, 0);
> >> lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
> >> - &jbd2_trans_commit_key, 0);
> >> + &journal->jbd2_trans_commit_key, 0);
> >>
> >> /* The journal is marked for error until we succeed with recovery! */
> >> journal->j_flags = JBD2_ABORT;
> >> @@ -1611,6 +1611,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> >> kfree(journal->j_wbuf);
> >> jbd2_journal_destroy_revoke(journal);
> >> journal_fail_superblock(journal);
> >> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
> >> kfree(journal);
> >> return ERR_PTR(err);
> >> }
> >> @@ -2187,6 +2188,7 @@ int jbd2_journal_destroy(journal_t *journal)
> >> jbd2_journal_destroy_revoke(journal);
> >> kfree(journal->j_fc_wbuf);
> >> kfree(journal->j_wbuf);
> >> + lockdep_unregister_key(&journal->jbd2_trans_commit_key);
> >> kfree(journal);
> >>
> >> return err;
> >> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> >> index 43b9297fe8a7..f5eaf76198f3 100644
> >> --- a/include/linux/jbd2.h
> >> +++ b/include/linux/jbd2.h
> >> @@ -1253,6 +1253,12 @@ struct journal_s
> >> */
> >> struct lockdep_map j_trans_commit_map;
> >> #endif
> >> + /**
> >> + * @jbd2_trans_commit_key:
> >> + *
> >> + * "struct lock_class_key" for @j_trans_commit_map
> >> + */
> >> + struct lock_class_key jbd2_trans_commit_key;
> >>
> >> /**
> >> * @j_fc_cleanup_callback:
> >> --
> >> 2.47.3
> >>
> >>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
Hi Tetsuo,
kernel test robot noticed the following build errors:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master brauner-vfs/vfs.all v6.18-rc1 next-20251017]
[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/Tetsuo-Handa/jbd2-assign-different-lock_class_key-for-different-filesystem/20251019-185450
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/e42f1471-a88a-4938-8743-1d5b171c47ec%40I-love.SAKURA.ne.jp
patch subject: [PATCH] jbd2: assign different lock_class_key for different filesystem
config: x86_64-buildonly-randconfig-001-20251019 (https://download.01.org/0day-ci/archive/20251019/202510192128.u69Bbmpz-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/20251019/202510192128.u69Bbmpz-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/202510192128.u69Bbmpz-lkp@intel.com/
All errors (new ones prefixed by >>):
>> fs/jbd2/journal.c:1553:17: error: use of undeclared identifier 'EXT4_SUPER_MAGIC'
1553 | fsmagic == EXT4_SUPER_MAGIC) {
| ^
>> fs/jbd2/journal.c:1562:17: error: use of undeclared identifier 'OCFS2_SUPER_MAGIC'
1562 | fsmagic == OCFS2_SUPER_MAGIC) {
| ^
fs/jbd2/journal.c:1585:17: error: use of undeclared identifier 'EXT4_SUPER_MAGIC'
1585 | fsmagic == EXT4_SUPER_MAGIC)
| ^
fs/jbd2/journal.c:1589:15: error: use of undeclared identifier 'OCFS2_SUPER_MAGIC'
1589 | fsmagic == OCFS2_SUPER_MAGIC)
| ^
4 errors generated.
vim +/EXT4_SUPER_MAGIC +1553 fs/jbd2/journal.c
1508
1509
1510 /*
1511 * Management for journal control blocks: functions to create and
1512 * destroy journal_t structures, and to initialise and read existing
1513 * journal blocks from disk. */
1514
1515 /* The journal_init_common() function creates and fills a journal_t object
1516 * in memory. It calls journal_load_superblock() to load the on-disk journal
1517 * superblock and initialize the journal_t object.
1518 */
1519
1520 static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
1521 unsigned long long start, int len, int blocksize,
1522 unsigned long fsmagic)
1523 {
1524 static struct lock_class_key jbd2_trans_commit_key_ext4;
1525 static struct lock_class_key jbd2_trans_commit_key_ocfs2;
1526 static struct lock_class_key jbd2_trans_commit_key_unknown;
1527 journal_t *journal;
1528 int err;
1529 int n;
1530
1531 journal = kzalloc(sizeof(*journal), GFP_KERNEL);
1532 if (!journal)
1533 return ERR_PTR(-ENOMEM);
1534
1535 journal->j_blocksize = blocksize;
1536 journal->j_dev = bdev;
1537 journal->j_fs_dev = fs_dev;
1538 journal->j_blk_offset = start;
1539 journal->j_total_len = len;
1540 jbd2_init_fs_dev_write_error(journal);
1541
1542 err = journal_load_superblock(journal);
1543 if (err)
1544 goto err_cleanup;
1545
1546 init_waitqueue_head(&journal->j_wait_transaction_locked);
1547 init_waitqueue_head(&journal->j_wait_done_commit);
1548 init_waitqueue_head(&journal->j_wait_commit);
1549 init_waitqueue_head(&journal->j_wait_updates);
1550 init_waitqueue_head(&journal->j_wait_reserved);
1551 init_waitqueue_head(&journal->j_fc_wait);
1552 if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
> 1553 fsmagic == EXT4_SUPER_MAGIC) {
1554 mutex_init(&journal->j_abort_mutex);
1555 mutex_init(&journal->j_barrier);
1556 mutex_init(&journal->j_checkpoint_mutex);
1557 spin_lock_init(&journal->j_revoke_lock);
1558 spin_lock_init(&journal->j_list_lock);
1559 spin_lock_init(&journal->j_history_lock);
1560 rwlock_init(&journal->j_state_lock);
1561 } else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
> 1562 fsmagic == OCFS2_SUPER_MAGIC) {
1563 mutex_init(&journal->j_abort_mutex);
1564 mutex_init(&journal->j_barrier);
1565 mutex_init(&journal->j_checkpoint_mutex);
1566 spin_lock_init(&journal->j_revoke_lock);
1567 spin_lock_init(&journal->j_list_lock);
1568 spin_lock_init(&journal->j_history_lock);
1569 rwlock_init(&journal->j_state_lock);
1570 } else {
1571 mutex_init(&journal->j_abort_mutex);
1572 mutex_init(&journal->j_barrier);
1573 mutex_init(&journal->j_checkpoint_mutex);
1574 spin_lock_init(&journal->j_revoke_lock);
1575 spin_lock_init(&journal->j_list_lock);
1576 spin_lock_init(&journal->j_history_lock);
1577 rwlock_init(&journal->j_state_lock);
1578 }
1579
1580 journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
1581 journal->j_min_batch_time = 0;
1582 journal->j_max_batch_time = 15000; /* 15ms */
1583 atomic_set(&journal->j_reserved_credits, 0);
1584 if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
1585 fsmagic == EXT4_SUPER_MAGIC)
1586 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ext4",
1587 &jbd2_trans_commit_key_ext4, 0);
1588 else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
1589 fsmagic == OCFS2_SUPER_MAGIC)
1590 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ocfs2",
1591 &jbd2_trans_commit_key_ocfs2, 0);
1592 else
1593 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_unknown",
1594 &jbd2_trans_commit_key_unknown, 0);
1595
1596 /* The journal is marked for error until we succeed with recovery! */
1597 journal->j_flags = JBD2_ABORT;
1598
1599 /* Set up a default-sized revoke table for the new mount. */
1600 err = jbd2_journal_init_revoke(journal, JOURNAL_REVOKE_DEFAULT_HASH);
1601 if (err)
1602 goto err_cleanup;
1603
1604 /*
1605 * journal descriptor can store up to n blocks, we need enough
1606 * buffers to write out full descriptor block.
1607 */
1608 err = -ENOMEM;
1609 n = journal->j_blocksize / jbd2_min_tag_size();
1610 journal->j_wbufsize = n;
1611 journal->j_fc_wbuf = NULL;
1612 journal->j_wbuf = kmalloc_array(n, sizeof(struct buffer_head *),
1613 GFP_KERNEL);
1614 if (!journal->j_wbuf)
1615 goto err_cleanup;
1616
1617 err = percpu_counter_init(&journal->j_checkpoint_jh_count, 0,
1618 GFP_KERNEL);
1619 if (err)
1620 goto err_cleanup;
1621
1622 journal->j_shrink_transaction = NULL;
1623
1624 journal->j_shrinker = shrinker_alloc(0, "jbd2-journal:(%u:%u)",
1625 MAJOR(bdev->bd_dev),
1626 MINOR(bdev->bd_dev));
1627 if (!journal->j_shrinker) {
1628 err = -ENOMEM;
1629 goto err_cleanup;
1630 }
1631
1632 journal->j_shrinker->scan_objects = jbd2_journal_shrink_scan;
1633 journal->j_shrinker->count_objects = jbd2_journal_shrink_count;
1634 journal->j_shrinker->private_data = journal;
1635
1636 shrinker_register(journal->j_shrinker);
1637
1638 return journal;
1639
1640 err_cleanup:
1641 percpu_counter_destroy(&journal->j_checkpoint_jh_count);
1642 kfree(journal->j_wbuf);
1643 jbd2_journal_destroy_revoke(journal);
1644 journal_fail_superblock(journal);
1645 kfree(journal);
1646 return ERR_PTR(err);
1647 }
1648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Tetsuo,
kernel test robot noticed the following build errors:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master brauner-vfs/vfs.all v6.18-rc1 next-20251017]
[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/Tetsuo-Handa/jbd2-assign-different-lock_class_key-for-different-filesystem/20251019-185450
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/e42f1471-a88a-4938-8743-1d5b171c47ec%40I-love.SAKURA.ne.jp
patch subject: [PATCH] jbd2: assign different lock_class_key for different filesystem
config: arm-randconfig-002-20251019 (https://download.01.org/0day-ci/archive/20251019/202510192155.eZoaLgzE-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251019/202510192155.eZoaLgzE-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/202510192155.eZoaLgzE-lkp@intel.com/
All errors (new ones prefixed by >>):
fs/jbd2/journal.c: In function 'journal_init_common':
>> fs/jbd2/journal.c:1553:24: error: 'EXT4_SUPER_MAGIC' undeclared (first use in this function)
1553 | fsmagic == EXT4_SUPER_MAGIC) {
| ^~~~~~~~~~~~~~~~
fs/jbd2/journal.c:1553:24: note: each undeclared identifier is reported only once for each function it appears in
>> fs/jbd2/journal.c:1562:31: error: 'OCFS2_SUPER_MAGIC' undeclared (first use in this function)
1562 | fsmagic == OCFS2_SUPER_MAGIC) {
| ^~~~~~~~~~~~~~~~~
vim +/EXT4_SUPER_MAGIC +1553 fs/jbd2/journal.c
1508
1509
1510 /*
1511 * Management for journal control blocks: functions to create and
1512 * destroy journal_t structures, and to initialise and read existing
1513 * journal blocks from disk. */
1514
1515 /* The journal_init_common() function creates and fills a journal_t object
1516 * in memory. It calls journal_load_superblock() to load the on-disk journal
1517 * superblock and initialize the journal_t object.
1518 */
1519
1520 static journal_t *journal_init_common(struct block_device *bdev, struct block_device *fs_dev,
1521 unsigned long long start, int len, int blocksize,
1522 unsigned long fsmagic)
1523 {
1524 static struct lock_class_key jbd2_trans_commit_key_ext4;
1525 static struct lock_class_key jbd2_trans_commit_key_ocfs2;
1526 static struct lock_class_key jbd2_trans_commit_key_unknown;
1527 journal_t *journal;
1528 int err;
1529 int n;
1530
1531 journal = kzalloc(sizeof(*journal), GFP_KERNEL);
1532 if (!journal)
1533 return ERR_PTR(-ENOMEM);
1534
1535 journal->j_blocksize = blocksize;
1536 journal->j_dev = bdev;
1537 journal->j_fs_dev = fs_dev;
1538 journal->j_blk_offset = start;
1539 journal->j_total_len = len;
1540 jbd2_init_fs_dev_write_error(journal);
1541
1542 err = journal_load_superblock(journal);
1543 if (err)
1544 goto err_cleanup;
1545
1546 init_waitqueue_head(&journal->j_wait_transaction_locked);
1547 init_waitqueue_head(&journal->j_wait_done_commit);
1548 init_waitqueue_head(&journal->j_wait_commit);
1549 init_waitqueue_head(&journal->j_wait_updates);
1550 init_waitqueue_head(&journal->j_wait_reserved);
1551 init_waitqueue_head(&journal->j_fc_wait);
1552 if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
> 1553 fsmagic == EXT4_SUPER_MAGIC) {
1554 mutex_init(&journal->j_abort_mutex);
1555 mutex_init(&journal->j_barrier);
1556 mutex_init(&journal->j_checkpoint_mutex);
1557 spin_lock_init(&journal->j_revoke_lock);
1558 spin_lock_init(&journal->j_list_lock);
1559 spin_lock_init(&journal->j_history_lock);
1560 rwlock_init(&journal->j_state_lock);
1561 } else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
> 1562 fsmagic == OCFS2_SUPER_MAGIC) {
1563 mutex_init(&journal->j_abort_mutex);
1564 mutex_init(&journal->j_barrier);
1565 mutex_init(&journal->j_checkpoint_mutex);
1566 spin_lock_init(&journal->j_revoke_lock);
1567 spin_lock_init(&journal->j_list_lock);
1568 spin_lock_init(&journal->j_history_lock);
1569 rwlock_init(&journal->j_state_lock);
1570 } else {
1571 mutex_init(&journal->j_abort_mutex);
1572 mutex_init(&journal->j_barrier);
1573 mutex_init(&journal->j_checkpoint_mutex);
1574 spin_lock_init(&journal->j_revoke_lock);
1575 spin_lock_init(&journal->j_list_lock);
1576 spin_lock_init(&journal->j_history_lock);
1577 rwlock_init(&journal->j_state_lock);
1578 }
1579
1580 journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
1581 journal->j_min_batch_time = 0;
1582 journal->j_max_batch_time = 15000; /* 15ms */
1583 atomic_set(&journal->j_reserved_credits, 0);
1584 if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_EXT4_FS) &&
1585 fsmagic == EXT4_SUPER_MAGIC)
1586 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ext4",
1587 &jbd2_trans_commit_key_ext4, 0);
1588 else if (IS_ENABLED(CONFIG_LOCKDEP) && IS_ENABLED(CONFIG_OCFS2_FS) &&
1589 fsmagic == OCFS2_SUPER_MAGIC)
1590 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_ocfs2",
1591 &jbd2_trans_commit_key_ocfs2, 0);
1592 else
1593 lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle_unknown",
1594 &jbd2_trans_commit_key_unknown, 0);
1595
1596 /* The journal is marked for error until we succeed with recovery! */
1597 journal->j_flags = JBD2_ABORT;
1598
1599 /* Set up a default-sized revoke table for the new mount. */
1600 err = jbd2_journal_init_revoke(journal, JOURNAL_REVOKE_DEFAULT_HASH);
1601 if (err)
1602 goto err_cleanup;
1603
1604 /*
1605 * journal descriptor can store up to n blocks, we need enough
1606 * buffers to write out full descriptor block.
1607 */
1608 err = -ENOMEM;
1609 n = journal->j_blocksize / jbd2_min_tag_size();
1610 journal->j_wbufsize = n;
1611 journal->j_fc_wbuf = NULL;
1612 journal->j_wbuf = kmalloc_array(n, sizeof(struct buffer_head *),
1613 GFP_KERNEL);
1614 if (!journal->j_wbuf)
1615 goto err_cleanup;
1616
1617 err = percpu_counter_init(&journal->j_checkpoint_jh_count, 0,
1618 GFP_KERNEL);
1619 if (err)
1620 goto err_cleanup;
1621
1622 journal->j_shrink_transaction = NULL;
1623
1624 journal->j_shrinker = shrinker_alloc(0, "jbd2-journal:(%u:%u)",
1625 MAJOR(bdev->bd_dev),
1626 MINOR(bdev->bd_dev));
1627 if (!journal->j_shrinker) {
1628 err = -ENOMEM;
1629 goto err_cleanup;
1630 }
1631
1632 journal->j_shrinker->scan_objects = jbd2_journal_shrink_scan;
1633 journal->j_shrinker->count_objects = jbd2_journal_shrink_count;
1634 journal->j_shrinker->private_data = journal;
1635
1636 shrinker_register(journal->j_shrinker);
1637
1638 return journal;
1639
1640 err_cleanup:
1641 percpu_counter_destroy(&journal->j_checkpoint_jh_count);
1642 kfree(journal->j_wbuf);
1643 jbd2_journal_destroy_revoke(journal);
1644 journal_fail_superblock(journal);
1645 kfree(journal);
1646 return ERR_PTR(err);
1647 }
1648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.