mirror of
https://github.com/raspberrypi/linux.git
synced 2025-12-06 01:49:46 +00:00
xfs: fix online repair probing when CONFIG_XFS_ONLINE_REPAIR=n
commit66314e9a57upstream. 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 commitdcb660f922(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, commit8336a64eb7(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 commit4bdfd7d157(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 commit48a72f6086(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:8336a64eb7("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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
46017a71f8
commit
943ed76ef0
@@ -212,7 +212,6 @@ static inline bool xchk_skip_xref(struct xfs_scrub_metadata *sm)
|
||||
bool xchk_dir_looks_zapped(struct xfs_inode *dp);
|
||||
bool xchk_pptr_looks_zapped(struct xfs_inode *ip);
|
||||
|
||||
#ifdef CONFIG_XFS_ONLINE_REPAIR
|
||||
/* Decide if a repair is required. */
|
||||
static inline bool xchk_needs_repair(const struct xfs_scrub_metadata *sm)
|
||||
{
|
||||
@@ -232,10 +231,6 @@ static inline bool xchk_could_repair(const struct xfs_scrub *sc)
|
||||
return (sc->sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR) &&
|
||||
!(sc->flags & XREP_ALREADY_FIXED);
|
||||
}
|
||||
#else
|
||||
# define xchk_needs_repair(sc) (false)
|
||||
# define xchk_could_repair(sc) (false)
|
||||
#endif /* CONFIG_XFS_ONLINE_REPAIR */
|
||||
|
||||
int xchk_metadata_inode_forks(struct xfs_scrub *sc);
|
||||
|
||||
|
||||
@@ -173,7 +173,16 @@ bool xrep_buf_verify_struct(struct xfs_buf *bp, const struct xfs_buf_ops *ops);
|
||||
#else
|
||||
|
||||
#define xrep_ino_dqattach(sc) (0)
|
||||
#define xrep_will_attempt(sc) (false)
|
||||
|
||||
/*
|
||||
* When online repair is not built into the kernel, we still want to attempt
|
||||
* the repair so that the stub xrep_attempt below will return EOPNOTSUPP.
|
||||
*/
|
||||
static inline bool xrep_will_attempt(const struct xfs_scrub *sc)
|
||||
{
|
||||
return (sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
|
||||
xchk_needs_repair(sc->sm);
|
||||
}
|
||||
|
||||
static inline int
|
||||
xrep_attempt(
|
||||
|
||||
@@ -149,6 +149,18 @@ xchk_probe(
|
||||
if (xchk_should_terminate(sc, &error))
|
||||
return error;
|
||||
|
||||
/*
|
||||
* If the caller is probing to see if repair works but repair isn't
|
||||
* built into the kernel, return EOPNOTSUPP because that's the signal
|
||||
* that userspace expects. If online repair is built in, set the
|
||||
* CORRUPT flag (without any of the usual tracing/logging) to force us
|
||||
* into xrep_probe.
|
||||
*/
|
||||
if (xchk_could_repair(sc)) {
|
||||
if (!IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR))
|
||||
return -EOPNOTSUPP;
|
||||
sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user