Skip to content

Commit 390c9dc

Browse files
committed
xfs: fix online repair probing when CONFIG_XFS_ONLINE_REPAIR=n
JIRA: https://issues.redhat.com/browse/RHEL-85590 commit 66314e9 Author: Darrick J. Wong <djwong@kernel.org> Date: Sun Feb 2 16:50:14 2025 -0800 xfs: fix online repair probing when CONFIG_XFS_ONLINE_REPAIR=n I received a report from the release engineering side of the house that xfs_scrub without the -n flag (aka fix it mode) would try to fix a broken filesystem even on a kernel that doesn't have online repair built into it: # xfs_scrub -dTvn /mnt/test EXPERIMENTAL xfs_scrub program in use! Use at your own risk! Phase 1: Find filesystem geometry. /mnt/test: using 1 threads to scrub. Phase 1: Memory used: 132k/0k (108k/25k), time: 0.00/ 0.00/ 0.00s <snip> Phase 4: Repair filesystem. <snip> Info: /mnt/test/some/victimdir directory entries: Attempting repair. (repair.c line 351) Corruption: /mnt/test/some/victimdir directory entries: Repair unsuccessful; offline repair required. (repair.c line 204) Source: https://blogs.oracle.com/linux/post/xfs-online-filesystem-repair It is strange that xfs_scrub doesn't refuse to run, because the kernel is supposed to return EOPNOTSUPP if we actually needed to run a repair, and xfs_io's repair subcommand will perror that. And yet: # xfs_io -x -c 'repair probe' /mnt/test # The first problem is commit dcb660f (4.15) which should have had xchk_probe set the CORRUPT OFLAG so that any of the repair machinery will get called at all. It turns out that some refactoring that happened in the 6.6-6.8 era broke the operation of this corner case. What we *really* want to happen is that all the predicates that would steer xfs_scrub_metadata() towards calling xrep_attempt() should function the same way that they do when repair is compiled in; and then xrep_attempt gets to return the fatal EOPNOTSUPP error code that causes the probe to fail. Instead, commit 8336a64 (6.6) started the failwhale swimming by hoisting OFLAG checking logic into a helper whose non-repair stub always returns false, causing scrub to return "repair not needed" when in fact the repair is not supported. Prior to that commit, the oflag checking that was open-coded in scrub.c worked correctly. Similarly, in commit 4bdfd7d (6.8) we hoisted the IFLAG_REPAIR and ALREADY_FIXED logic into a helper whose non-repair stub always returns false, so we never enter the if test body that would have called xrep_attempt, let alone fail to decode the OFLAGs correctly. The final insult (yes, we're doing The Naked Gun now) is commit 48a72f6 (6.8) in which we hoisted the "are we going to try a repair?" predicate into yet another function with a non-repair stub always returns false. Fix xchk_probe to trigger xrep_probe if repair is enabled, or return EOPNOTSUPP directly if it is not. For all the other scrub types, we need to fix the header predicates so that the ->repair functions (which are all xrep_notsupported) get called to return EOPNOTSUPP. Commit 48a72 is tagged here because the scrub code prior to LTS 6.12 are incomplete and not worth patching. Reported-by: David Flynn <david.flynn@oracle.com> Cc: <stable@vger.kernel.org> # v6.8 Fixes: 8336a64 ("xfs: don't complain about unfixed metadata when repairs were injected") Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Carlos Maiolino <cem@kernel.org> Signed-off-by: Bill O'Donnell <bodonnel@redhat.com> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
1 parent 859d228 commit 390c9dc

File tree

3 files changed

+22
-6
lines changed

3 files changed

+22
-6
lines changed

fs/xfs/scrub/common.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,6 @@ static inline bool xchk_skip_xref(struct xfs_scrub_metadata *sm)
179179
bool xchk_dir_looks_zapped(struct xfs_inode *dp);
180180
bool xchk_pptr_looks_zapped(struct xfs_inode *ip);
181181

182-
#ifdef CONFIG_XFS_ONLINE_REPAIR
183182
/* Decide if a repair is required. */
184183
static inline bool xchk_needs_repair(const struct xfs_scrub_metadata *sm)
185184
{
@@ -199,10 +198,6 @@ static inline bool xchk_could_repair(const struct xfs_scrub *sc)
199198
return (sc->sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR) &&
200199
!(sc->flags & XREP_ALREADY_FIXED);
201200
}
202-
#else
203-
# define xchk_needs_repair(sc) (false)
204-
# define xchk_could_repair(sc) (false)
205-
#endif /* CONFIG_XFS_ONLINE_REPAIR */
206201

207202
int xchk_metadata_inode_forks(struct xfs_scrub *sc);
208203

fs/xfs/scrub/repair.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,16 @@ bool xrep_buf_verify_struct(struct xfs_buf *bp, const struct xfs_buf_ops *ops);
163163
#else
164164

165165
#define xrep_ino_dqattach(sc) (0)
166-
#define xrep_will_attempt(sc) (false)
166+
167+
/*
168+
* When online repair is not built into the kernel, we still want to attempt
169+
* the repair so that the stub xrep_attempt below will return EOPNOTSUPP.
170+
*/
171+
static inline bool xrep_will_attempt(const struct xfs_scrub *sc)
172+
{
173+
return (sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
174+
xchk_needs_repair(sc->sm);
175+
}
167176

168177
static inline int
169178
xrep_attempt(

fs/xfs/scrub/scrub.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,18 @@ xchk_probe(
149149
if (xchk_should_terminate(sc, &error))
150150
return error;
151151

152+
/*
153+
* If the caller is probing to see if repair works but repair isn't
154+
* built into the kernel, return EOPNOTSUPP because that's the signal
155+
* that userspace expects. If online repair is built in, set the
156+
* CORRUPT flag (without any of the usual tracing/logging) to force us
157+
* into xrep_probe.
158+
*/
159+
if (xchk_could_repair(sc)) {
160+
if (!IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR))
161+
return -EOPNOTSUPP;
162+
sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
163+
}
152164
return 0;
153165
}
154166

0 commit comments

Comments
 (0)