public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Mahmoud Adam <mngyadam@amazon.com>
To: <gregkh@linuxfoundation.org>
Cc: <djwong@kernel.org>, <stable@vger.kernel.org>
Subject: [PATCH 6.1 2/6] xfs: fix log recovery when unknown rocompat bits are set
Date: Wed, 3 Apr 2024 14:59:49 +0200	[thread overview]
Message-ID: <20240403125949.33676-3-mngyadam@amazon.com> (raw)
In-Reply-To: <20240403125949.33676-1-mngyadam@amazon.com>

From: "Darrick J. Wong" <djwong@kernel.org>

commit 74ad4693b6473950e971b3dc525b5ee7570e05d0 upstream.

Log recovery has always run on read only mounts, even where the primary
superblock advertises unknown rocompat bits.  Due to a misunderstanding
between Eric and Darrick back in 2018, we accidentally changed the
superblock write verifier to shutdown the fs over that exact scenario.
As a result, the log cleaning that occurs at the end of the mounting
process fails if there are unknown rocompat bits set.

As we now allow writing of the superblock if there are unknown rocompat
bits set on a RO mount, we no longer want to turn off RO state to allow
log recovery to succeed on a RO mount.  Hence we also remove all the
(now unnecessary) RO state toggling from the log recovery path.

Fixes: 9e037cb7972f ("xfs: check for unknown v5 feature bits in superblock write verifier"
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/libxfs/xfs_sb.c |  3 ++-
 fs/xfs/xfs_log.c       | 17 -----------------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index b6a584e044be..7fae732486a9 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -266,7 +266,8 @@ xfs_validate_sb_write(
 		return -EFSCORRUPTED;
 	}

-	if (xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
+	if (!xfs_is_readonly(mp) &&
+	    xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
 		xfs_alert(mp,
 "Corruption detected in superblock read-only compatible features (0x%x)!",
 			(sbp->sb_features_ro_compat &
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f02a0dd522b3..83381d38efbe 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -730,15 +730,7 @@ xfs_log_mount(
 	 * just worked.
 	 */
 	if (!xfs_has_norecovery(mp)) {
-		/*
-		 * log recovery ignores readonly state and so we need to clear
-		 * mount-based read only state so it can write to disk.
-		 */
-		bool	readonly = test_and_clear_bit(XFS_OPSTATE_READONLY,
-						&mp->m_opstate);
 		error = xlog_recover(log);
-		if (readonly)
-			set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
 		if (error) {
 			xfs_warn(mp, "log mount/recovery failed: error %d",
 				error);
@@ -787,7 +779,6 @@ xfs_log_mount_finish(
 	struct xfs_mount	*mp)
 {
 	struct xlog		*log = mp->m_log;
-	bool			readonly;
 	int			error = 0;

 	if (xfs_has_norecovery(mp)) {
@@ -795,12 +786,6 @@ xfs_log_mount_finish(
 		return 0;
 	}

-	/*
-	 * log recovery ignores readonly state and so we need to clear
-	 * mount-based read only state so it can write to disk.
-	 */
-	readonly = test_and_clear_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
-
 	/*
 	 * During the second phase of log recovery, we need iget and
 	 * iput to behave like they do for an active filesystem.
@@ -850,8 +835,6 @@ xfs_log_mount_finish(
 	xfs_buftarg_drain(mp->m_ddev_targp);

 	clear_bit(XLOG_RECOVERY_NEEDED, &log->l_opstate);
-	if (readonly)
-		set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);

 	/* Make sure the log is dead if we're returning failure. */
 	ASSERT(!error || xlog_is_shutdown(log));
--
2.40.1

  parent reply	other threads:[~2024-04-03 13:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03 12:59 [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606 Mahmoud Adam
2024-04-03 12:59 ` [PATCH 6.1 1/6] xfs: allow inode inactivation during a ro mount log recovery Mahmoud Adam
2024-04-03 12:59 ` Mahmoud Adam [this message]
2024-04-03 12:59 ` [PATCH 6.1 3/6] xfs: get root inode correctly at bulkstat Mahmoud Adam
2024-04-03 12:59 ` [PATCH 6.1 4/6] xfs: short circuit xfs_growfs_data_private() if delta is zero Mahmoud Adam
2024-04-03 12:59 ` [PATCH 6.1 5/6] xfs: hoist refcount record merge predicates Mahmoud Adam
2024-04-03 12:59 ` [PATCH 6.1 6/6] xfs: estimate post-merge refcounts correctly Mahmoud Adam
2024-04-03 18:18 ` [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606 Darrick J. Wong
2024-04-04  5:45   ` Amir Goldstein
2024-04-04  9:15     ` Mahmoud Adam
2024-04-05  9:27       ` Greg KH
2024-04-05  9:55         ` Amir Goldstein
2024-04-11  7:22           ` Greg KH
2024-04-11 17:27             ` Leah Rumancik

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=20240403125949.33676-3-mngyadam@amazon.com \
    --to=mngyadam@amazon.com \
    --cc=djwong@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=stable@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