[PATCH 5.15] xfs: allow inode inactivation during a ro mount log recovery

[PATCH 5.15] xfs: allow inode inactivation during a ro mount log recovery

[Amir Goldstein]

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

[ Upstream commit 76e589013fec672c3587d6314f2d1f0aeddc26d9 ]

In the next patch, we're going to prohibit log recovery if the primary
superblock contains an unrecognized rocompat feature bit even on
readonly mounts.  This requires removing all the code in the log
mounting process that temporarily disables the readonly state.

Unfortunately, inode inactivation disables itself on readonly mounts.
Clearing the iunlinked lists after log recovery needs inactivation to
run to free the unreferenced inodes, which (AFAICT) is the only reason
why log mounting plays games with the readonly state in the first place.

Therefore, change the inactivation predicates to allow inactivation
during log recovery of a readonly mount.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Stable-dep-of: 74ad4693b647 ("xfs: fix log recovery when unknown rocompat bits are set")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Sasha,

This 5.15 backport is needed to fix a regression introduced to
test generic/417 in kernel v5.15.176.
With this backport, kernel v5.15.185 passed the fstests quick run.

As you may have noticed, 5.15.y (and 5.10.y) are not being actively
maintained by xfs stable maintainer who moved their focus to 6.*.y
LTS kernels.

The $SUBJECT commit is a dependency of commit 74ad4693b647, as hinted by
the wording: "In the next patch, we're going to... This requires...".

Indeed, Leah has backported commit 74ad4693b647 to 6.1.y along with its
dependency, yet somehow, commit 74ad4693b647 found its way to v5.15.176,
without the dependency and without the xfs stable review process.

Judging by the line: Stable-dep-of: 652f03db897b ("xfs: remove unknown
compat feature check in superblock write validation") that exists only
in the 5.15.y tree, I deduce that your bot has auto selected this
patch in the process of backporting the commit 652f03db897b, which was
explicitly marked for stable v4.19+ [1].

I don't know if there is a lesson to be learned from this incident.
Applying xfs backports without running fstests regression is always
going to be a gamble. I will leave it up to you to decide if anything
in the process of applying xfs patches to <= v5.15.y needs to change.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-xfs/ZzFon-0VbKscbGMT@localhost.localdomain/

 fs/xfs/xfs_inode.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3b36d5569d15..98955cd0de40 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -32,6 +32,7 @@
 #include "xfs_symlink.h"
 #include "xfs_trans_priv.h"
 #include "xfs_log.h"
+#include "xfs_log_priv.h"
 #include "xfs_bmap_btree.h"
 #include "xfs_reflink.h"
 #include "xfs_ag.h"
@@ -1678,8 +1679,11 @@ xfs_inode_needs_inactive(
 	if (VFS_I(ip)->i_mode == 0)
 		return false;
 
-	/* If this is a read-only mount, don't do this (would generate I/O) */
-	if (xfs_is_readonly(mp))
+	/*
+	 * If this is a read-only mount, don't do this (would generate I/O)
+	 * unless we're in log recovery and cleaning the iunlinked list.
+	 */
+	if (xfs_is_readonly(mp) && !xlog_recovery_needed(mp->m_log))
 		return false;
 
 	/* If the log isn't running, push inodes straight to reclaim. */
@@ -1739,8 +1743,11 @@ xfs_inactive(
 	mp = ip->i_mount;
 	ASSERT(!xfs_iflags_test(ip, XFS_IRECOVERY));
 
-	/* If this is a read-only mount, don't do this (would generate I/O) */
-	if (xfs_is_readonly(mp))
+	/*
+	 * If this is a read-only mount, don't do this (would generate I/O)
+	 * unless we're in log recovery and cleaning the iunlinked list.
+	 */
+	if (xfs_is_readonly(mp) && !xlog_recovery_needed(mp->m_log))
 		goto out;
 
 	/* Metadata inodes require explicit resource cleanup. */
-- 
2.47.1

Re: [PATCH 5.15] xfs: allow inode inactivation during a ro mount log recovery

[Sasha Levin]

On Tue, Jun 17, 2025 at 10:55:33PM +0200, Amir Goldstein wrote:
>Sasha,
>
>This 5.15 backport is needed to fix a regression introduced to
>test generic/417 in kernel v5.15.176.
>With this backport, kernel v5.15.185 passed the fstests quick run.
>
>As you may have noticed, 5.15.y (and 5.10.y) are not being actively
>maintained by xfs stable maintainer who moved their focus to 6.*.y
>LTS kernels.
>
>The $SUBJECT commit is a dependency of commit 74ad4693b647, as hinted by
>the wording: "In the next patch, we're going to... This requires...".
>
>Indeed, Leah has backported commit 74ad4693b647 to 6.1.y along with its
>dependency, yet somehow, commit 74ad4693b647 found its way to v5.15.176,
>without the dependency and without the xfs stable review process.
>
>Judging by the line: Stable-dep-of: 652f03db897b ("xfs: remove unknown
>compat feature check in superblock write validation") that exists only
>in the 5.15.y tree, I deduce that your bot has auto selected this
>patch in the process of backporting the commit 652f03db897b, which was
>explicitly marked for stable v4.19+ [1].
>
>I don't know if there is a lesson to be learned from this incident.
>Applying xfs backports without running fstests regression is always
>going to be a gamble. I will leave it up to you to decide if anything
>in the process of applying xfs patches to <= v5.15.y needs to change.

My scripts don't filter out commits with a stable tag like they do for
commits without. This was safe until they also started pulling in
more commits as dependencies to resolve conflicts.

I'll fix it on my end by adding the same filter for stable tagged
commits as well.

Thanks for the fix! I'll queue it up for this upcoming release.

-- 
Thanks,
Sasha