[PATCH v3] bcachefs: fix deadlock in journal_entry_open()
From: Jeongjun Park
Date: Sat Feb 01 2025 - 14:51:10 EST
In the previous commit b3d82c2f2761, code was added to prevent journal sequence
overflow. Among them, the code added to journal_entry_open() uses the
bch2_fs_fatal_err_on() function to handle errors.
However, __journal_res_get() , which calls journal_entry_open() , calls
journal_entry_open() while holding journal->lock , but bch2_fs_fatal_err_on()
internally tries to acquire journal->lock , which results in a deadlock.
So we need to add a locked helper like bch2_journal_halt_locked() to handle
fatal errors even when the journal->lock is held.
Fixes: b3d82c2f2761 ("bcachefs: Guard against journal seq overflow")
Signed-off-by: Jeongjun Park <aha310510@xxxxxxxxx>
---
fs/bcachefs/error.c | 6 +++---
fs/bcachefs/fs-ioctl.c | 4 ++--
fs/bcachefs/journal.c | 22 +++++++++++++++++-----
fs/bcachefs/journal.h | 3 ++-
fs/bcachefs/recovery.c | 2 +-
fs/bcachefs/super.c | 4 ++--
fs/bcachefs/super.h | 2 +-
7 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
index 038da6a61f6b..2482eb33b1ec 100644
--- a/fs/bcachefs/error.c
+++ b/fs/bcachefs/error.c
@@ -20,7 +20,7 @@ bool bch2_inconsistent_error(struct bch_fs *c)
return false;
case BCH_ON_ERROR_fix_safe:
case BCH_ON_ERROR_ro:
- if (bch2_fs_emergency_read_only(c))
+ if (bch2_fs_emergency_read_only(c, false))
bch_err(c, "inconsistency detected - emergency read only at journal seq %llu",
journal_cur_seq(&c->journal));
return true;
@@ -46,7 +46,7 @@ int bch2_topology_error(struct bch_fs *c)
void bch2_fatal_error(struct bch_fs *c)
{
- if (bch2_fs_emergency_read_only(c))
+ if (bch2_fs_emergency_read_only(c, false))
bch_err(c, "fatal error - emergency read only");
}
@@ -62,7 +62,7 @@ void bch2_io_error_work(struct work_struct *work)
if (dev
? __bch2_dev_set_state(c, ca, BCH_MEMBER_STATE_ro,
BCH_FORCE_IF_DEGRADED)
- : bch2_fs_emergency_read_only(c))
+ : bch2_fs_emergency_read_only(c, false))
bch_err(ca,
"too many IO errors, setting %s RO",
dev ? "device" : "filesystem");
diff --git a/fs/bcachefs/fs-ioctl.c b/fs/bcachefs/fs-ioctl.c
index 15725b4ce393..92becb6ae5aa 100644
--- a/fs/bcachefs/fs-ioctl.c
+++ b/fs/bcachefs/fs-ioctl.c
@@ -354,14 +354,14 @@ static int bch2_ioc_goingdown(struct bch_fs *c, u32 __user *arg)
if (ret)
break;
bch2_journal_flush(&c->journal);
- bch2_fs_emergency_read_only(c);
+ bch2_fs_emergency_read_only(c, false);
bdev_thaw(c->vfs_sb->s_bdev);
break;
case FSOP_GOING_FLAGS_LOGFLUSH:
bch2_journal_flush(&c->journal);
fallthrough;
case FSOP_GOING_FLAGS_NOLOGFLUSH:
- bch2_fs_emergency_read_only(c);
+ bch2_fs_emergency_read_only(c, false);
break;
default:
ret = -EINVAL;
diff --git a/fs/bcachefs/journal.c b/fs/bcachefs/journal.c
index 2cd20114b74b..4b3bdbfd7782 100644
--- a/fs/bcachefs/journal.c
+++ b/fs/bcachefs/journal.c
@@ -310,14 +310,23 @@ static void __journal_entry_close(struct journal *j, unsigned closed_val, bool t
__bch2_journal_buf_put(j, old.idx, le64_to_cpu(buf->data->seq));
}
-void bch2_journal_halt(struct journal *j)
+void bch2_journal_halt_locked(struct journal *j)
{
- spin_lock(&j->lock);
+ lockdep_assert_held(&j->lock);
+
__journal_entry_close(j, JOURNAL_ENTRY_ERROR_VAL, true);
if (!j->err_seq)
j->err_seq = journal_cur_seq(j);
journal_wake(j);
- spin_unlock(&j->lock);
+}
+
+void bch2_journal_halt(struct journal *j, bool locked)
+{
+ if (!locked)
+ spin_lock(&j->lock);
+ bch2_journal_halt_locked(j);
+ if (!locked)
+ spin_unlock(&j->lock);
}
static bool journal_entry_want_write(struct journal *j)
@@ -382,9 +391,12 @@ static int journal_entry_open(struct journal *j)
if (nr_unwritten_journal_entries(j) == ARRAY_SIZE(j->buf))
return JOURNAL_ERR_max_in_flight;
- if (bch2_fs_fatal_err_on(journal_cur_seq(j) >= JOURNAL_SEQ_MAX,
- c, "cannot start: journal seq overflow"))
+ if (journal_cur_seq(j) >= JOURNAL_SEQ_MAX) {
+ bch_err(c, "cannot start: journal seq overflow");
+ if (bch2_fs_emergency_read_only(c, true))
+ bch_err(c, "fatal error - emergency read only");
return JOURNAL_ERR_insufficient_devices; /* -EROFS */
+ }
BUG_ON(!j->cur_entry_sectors);
diff --git a/fs/bcachefs/journal.h b/fs/bcachefs/journal.h
index cb0df0663946..23f5fd970b7d 100644
--- a/fs/bcachefs/journal.h
+++ b/fs/bcachefs/journal.h
@@ -407,7 +407,8 @@ int bch2_journal_flush(struct journal *);
bool bch2_journal_noflush_seq(struct journal *, u64, u64);
int bch2_journal_meta(struct journal *);
-void bch2_journal_halt(struct journal *);
+void bch2_journal_halt(struct journal *, bool);
+void bch2_journal_halt_locked(struct journal *);
static inline int bch2_journal_error(struct journal *j)
{
diff --git a/fs/bcachefs/recovery.c b/fs/bcachefs/recovery.c
index 98825437381c..31549181888c 100644
--- a/fs/bcachefs/recovery.c
+++ b/fs/bcachefs/recovery.c
@@ -1076,7 +1076,7 @@ int bch2_fs_recovery(struct bch_fs *c)
return ret;
err:
fsck_err:
- bch2_fs_emergency_read_only(c);
+ bch2_fs_emergency_read_only(c, false);
goto out;
}
diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index d97ea7bd1171..d3d11fb39632 100644
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -400,11 +400,11 @@ static void bch2_fs_read_only_async(struct bch_fs *c)
queue_work(system_long_wq, &c->read_only_work);
}
-bool bch2_fs_emergency_read_only(struct bch_fs *c)
+bool bch2_fs_emergency_read_only(struct bch_fs *c, bool locked)
{
bool ret = !test_and_set_bit(BCH_FS_emergency_ro, &c->flags);
- bch2_journal_halt(&c->journal);
+ bch2_journal_halt(&c->journal, locked);
bch2_fs_read_only_async(c);
wake_up(&bch2_read_only_wait);
diff --git a/fs/bcachefs/super.h b/fs/bcachefs/super.h
index fa6d52216510..e9ff83e06ca2 100644
--- a/fs/bcachefs/super.h
+++ b/fs/bcachefs/super.h
@@ -28,7 +28,7 @@ int bch2_dev_offline(struct bch_fs *, struct bch_dev *, int);
int bch2_dev_resize(struct bch_fs *, struct bch_dev *, u64);
struct bch_dev *bch2_dev_lookup(struct bch_fs *, const char *);
-bool bch2_fs_emergency_read_only(struct bch_fs *);
+bool bch2_fs_emergency_read_only(struct bch_fs *, bool);
void bch2_fs_read_only(struct bch_fs *);
int bch2_fs_read_write(struct bch_fs *);
--