mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeongjun Park <aha310510@gmail.com>
To: kent.overstreet@linux.dev
Cc: linux-bcachefs@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jeongjun Park <aha310510@gmail.com>
Subject: [PATCH v3] bcachefs: fix deadlock in journal_entry_open()
Date: Sun,  2 Feb 2025 04:50:44 +0900	[thread overview]
Message-ID: <20250201195044.28007-1-aha310510@gmail.com> (raw)

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@gmail.com>
---
 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 *);
--

             reply	other threads:[~2025-02-01 19:51 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-01 19:50 Jeongjun Park [this message]
2025-02-01 21:24 ` Kent Overstreet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250201195044.28007-1-aha310510@gmail.com \
    --to=aha310510@gmail.com \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-bcachefs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®