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 *);
--

On Sun, Feb 02, 2025 at 04:50:44AM +0900, Jeongjun Park wrote:
> 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))

No, don't add a bool parameter. It'll be a lot less churny, and clearer
reading, if you add a separate

bch2_fs_emergency_read_only_locked()
helper

>  			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 *);
> --