* [PATCH 1/3] xfs_io: support passing the FORCE_REBUILD flag to online repair
2023-09-25 21:58 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Darrick J. Wong
@ 2023-09-25 21:58 ` Darrick J. Wong
2023-09-25 21:58 ` [PATCH 2/3] xfs_scrub: don't retry unsupported optimizations Darrick J. Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2023-09-25 21:58 UTC (permalink / raw)
To: djwong, cem; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
Add CLI options to the scrubv and repair commands so that the user can
pass FORCE_REBUILD to force the kernel to rebuild metadata.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
io/scrub.c | 24 ++++++++++++++++--------
man/man8/xfs_io.8 | 3 +++
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/io/scrub.c b/io/scrub.c
index fc22ba49f8b..dbdedf10099 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -46,7 +46,8 @@ scrub_ioctl(
int fd,
int type,
uint64_t control,
- uint32_t control2)
+ uint32_t control2,
+ uint32_t flags)
{
struct xfs_scrub_metadata meta;
const struct xfrog_scrub_descr *sc;
@@ -69,7 +70,7 @@ scrub_ioctl(
/* no control parameters */
break;
}
- meta.sm_flags = 0;
+ meta.sm_flags = flags;
error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
if (error)
@@ -91,17 +92,21 @@ parse_args(
int argc,
char **argv,
struct cmdinfo *cmdinfo,
- void (*fn)(int, int, uint64_t, uint32_t))
+ void (*fn)(int, int, uint64_t, uint32_t, uint32_t))
{
char *p;
int type = -1;
int i, c;
uint64_t control = 0;
uint32_t control2 = 0;
+ uint32_t flags = 0;
const struct xfrog_scrub_descr *d = NULL;
- while ((c = getopt(argc, argv, "")) != EOF) {
+ while ((c = getopt(argc, argv, "R")) != EOF) {
switch (c) {
+ case 'R':
+ flags |= XFS_SCRUB_IFLAG_FORCE_REBUILD;
+ break;
default:
return command_usage(cmdinfo);
}
@@ -173,7 +178,7 @@ parse_args(
ASSERT(0);
break;
}
- fn(file->fd, type, control, control2);
+ fn(file->fd, type, control, control2, flags);
return 0;
}
@@ -216,11 +221,13 @@ repair_help(void)
" or (optionally) take an inode number and generation number to act upon as\n"
" the second and third parameters.\n"
"\n"
+" Flags are -R to force rebuilding metadata.\n"
+"\n"
" Example:\n"
" 'repair inobt 3' - repairs the inode btree in AG 3.\n"
" 'repair bmapbtd 128 13525' - repairs the extent map of inode 128 gen 13525.\n"
"\n"
-" Known metadata repairs types are:"));
+" Known metadata repair types are:"));
for (i = 0, d = xfrog_scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
printf(" %s", d->name);
printf("\n");
@@ -231,7 +238,8 @@ repair_ioctl(
int fd,
int type,
uint64_t control,
- uint32_t control2)
+ uint32_t control2,
+ uint32_t flags)
{
struct xfs_scrub_metadata meta;
const struct xfrog_scrub_descr *sc;
@@ -254,7 +262,7 @@ repair_ioctl(
/* no control parameters */
break;
}
- meta.sm_flags = XFS_SCRUB_IFLAG_REPAIR;
+ meta.sm_flags = flags | XFS_SCRUB_IFLAG_REPAIR;
error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
if (error)
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index ef7087b3d09..d46dc369a06 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1340,6 +1340,9 @@ parameter specifies which type of metadata to repair.
For AG metadata, one AG number must be specified.
For file metadata, the repair is applied to the open file unless the
inode number and generation number are specified.
+The
+.B -R
+option can be specified to force rebuilding of a metadata structure.
.TP
.BI "label" " " "[ -c | -s " label " ] "
On filesystems that support online label manipulation, get, set, or clear the
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] xfs_scrub: don't retry unsupported optimizations
2023-09-25 21:58 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Darrick J. Wong
2023-09-25 21:58 ` [PATCH 1/3] xfs_io: support passing the FORCE_REBUILD flag to online repair Darrick J. Wong
@ 2023-09-25 21:58 ` Darrick J. Wong
2023-09-25 21:59 ` [PATCH 3/3] xfs_scrub: try to use XFS_SCRUB_IFLAG_FORCE_REBUILD Darrick J. Wong
2023-10-03 11:33 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Carlos Maiolino
3 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2023-09-25 21:58 UTC (permalink / raw)
To: djwong, cem; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
If the kernel says it doesn't support optimizing a data structure, we
should mark it done and move on. This is much better than requeuing the
repair, in which case it will likely keep failing. Eventually these
requeued repairs end up in the single-threaded last resort at the end of
phase 4, which makes things /very/ slow.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
scrub/scrub.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/scrub/scrub.c b/scrub/scrub.c
index e83d0d9ce99..1a4506875f7 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -668,6 +668,15 @@ _("Filesystem is shut down, aborting."));
return CHECK_ABORT;
case ENOTTY:
case EOPNOTSUPP:
+ /*
+ * If the kernel cannot perform the optimization that we
+ * requested; or we forced a repair but the kernel doesn't know
+ * how to perform the repair, don't requeue the request. Mark
+ * it done and move on.
+ */
+ if (is_unoptimized(&oldm) ||
+ debug_tweak_on("XFS_SCRUB_FORCE_REPAIR"))
+ return CHECK_DONE;
/*
* If we're in no-complain mode, requeue the check for
* later. It's possible that an error in another
@@ -678,13 +687,6 @@ _("Filesystem is shut down, aborting."));
*/
if (!(repair_flags & XRM_COMPLAIN_IF_UNFIXED))
return CHECK_RETRY;
- /*
- * If we forced repairs or this is a preen, don't
- * error out if the kernel doesn't know how to fix.
- */
- if (is_unoptimized(&oldm) ||
- debug_tweak_on("XFS_SCRUB_FORCE_REPAIR"))
- return CHECK_DONE;
fallthrough;
case EINVAL:
/* Kernel doesn't know how to repair this? */
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] xfs_scrub: try to use XFS_SCRUB_IFLAG_FORCE_REBUILD
2023-09-25 21:58 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Darrick J. Wong
2023-09-25 21:58 ` [PATCH 1/3] xfs_io: support passing the FORCE_REBUILD flag to online repair Darrick J. Wong
2023-09-25 21:58 ` [PATCH 2/3] xfs_scrub: don't retry unsupported optimizations Darrick J. Wong
@ 2023-09-25 21:59 ` Darrick J. Wong
2023-10-03 11:33 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Carlos Maiolino
3 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2023-09-25 21:59 UTC (permalink / raw)
To: djwong, cem; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
Now that we have a FORCE_REBUILD flag to the scrub ioctl, try to use
that over the (much noisier) error injection knob, which may or may not
even be enabled in the kernel config.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
scrub/phase1.c | 28 ++++++++++++++++++++++++++++
scrub/scrub.c | 45 +++++++++++++++++++++++----------------------
scrub/scrub.h | 1 +
scrub/xfs_scrub.c | 3 +++
scrub/xfs_scrub.h | 1 +
5 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/scrub/phase1.c b/scrub/phase1.c
index fd1050c9202..2daf5c7bb38 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -27,6 +27,7 @@
#include "scrub.h"
#include "repair.h"
#include "libfrog/fsgeom.h"
+#include "xfs_errortag.h"
/* Phase 1: Find filesystem geometry (and clean up after) */
@@ -68,6 +69,27 @@ scrub_cleanup(
return error;
}
+/* Decide if we're using FORCE_REBUILD or injecting FORCE_REPAIR. */
+static int
+enable_force_repair(
+ struct scrub_ctx *ctx)
+{
+ struct xfs_error_injection inject = {
+ .fd = ctx->mnt.fd,
+ .errtag = XFS_ERRTAG_FORCE_SCRUB_REPAIR,
+ };
+ int error;
+
+ use_force_rebuild = can_force_rebuild(ctx);
+ if (use_force_rebuild)
+ return 0;
+
+ error = ioctl(ctx->mnt.fd, XFS_IOC_ERROR_INJECTION, &inject);
+ if (error)
+ str_errno(ctx, _("force_repair"));
+ return error;
+}
+
/*
* Bind to the mountpoint, read the XFS geometry, bind to the block devices.
* Anything we've already built will be cleaned up by scrub_cleanup.
@@ -156,6 +178,12 @@ _("Kernel metadata repair facility is not available. Use -n to scrub."));
return ECANCELED;
}
+ if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR")) {
+ error = enable_force_repair(ctx);
+ if (error)
+ return error;
+ }
+
/* Did we find the log and rt devices, if they're present? */
if (ctx->mnt.fsgeom.logstart == 0 && ctx->fsinfo.fs_log == NULL) {
str_error(ctx, ctx->mntpoint,
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 1a4506875f7..1469058bd23 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -18,7 +18,6 @@
#include "common.h"
#include "progress.h"
#include "scrub.h"
-#include "xfs_errortag.h"
#include "repair.h"
#include "descr.h"
@@ -500,26 +499,16 @@ static bool
__scrub_test(
struct scrub_ctx *ctx,
unsigned int type,
- bool repair)
+ unsigned int flags)
{
struct xfs_scrub_metadata meta = {0};
- struct xfs_error_injection inject;
- static bool injected;
int error;
if (debug_tweak_on("XFS_SCRUB_NO_KERNEL"))
return false;
- if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !injected) {
- inject.fd = ctx->mnt.fd;
- inject.errtag = XFS_ERRTAG_FORCE_SCRUB_REPAIR;
- error = ioctl(ctx->mnt.fd, XFS_IOC_ERROR_INJECTION, &inject);
- if (error == 0)
- injected = true;
- }
meta.sm_type = type;
- if (repair)
- meta.sm_flags |= XFS_SCRUB_IFLAG_REPAIR;
+ meta.sm_flags = flags;
error = -xfrog_scrub_metadata(&ctx->mnt, &meta);
switch (error) {
case 0:
@@ -532,13 +521,15 @@ _("Filesystem is mounted read-only; cannot proceed."));
str_info(ctx, ctx->mntpoint,
_("Filesystem is mounted norecovery; cannot proceed."));
return false;
+ case EINVAL:
case EOPNOTSUPP:
case ENOTTY:
if (debug || verbose)
str_info(ctx, ctx->mntpoint,
_("Kernel %s %s facility not detected."),
_(xfrog_scrubbers[type].descr),
- repair ? _("repair") : _("scrub"));
+ (flags & XFS_SCRUB_IFLAG_REPAIR) ?
+ _("repair") : _("scrub"));
return false;
case ENOENT:
/* Scrubber says not present on this fs; that's fine. */
@@ -553,56 +544,64 @@ bool
can_scrub_fs_metadata(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_PROBE, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_PROBE, 0);
}
bool
can_scrub_inode(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_INODE, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_INODE, 0);
}
bool
can_scrub_bmap(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_BMBTD, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_BMBTD, 0);
}
bool
can_scrub_dir(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_DIR, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_DIR, 0);
}
bool
can_scrub_attr(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_XATTR, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_XATTR, 0);
}
bool
can_scrub_symlink(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_SYMLINK, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_SYMLINK, 0);
}
bool
can_scrub_parent(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_PARENT, false);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_PARENT, 0);
}
bool
xfs_can_repair(
struct scrub_ctx *ctx)
{
- return __scrub_test(ctx, XFS_SCRUB_TYPE_PROBE, true);
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_PROBE, XFS_SCRUB_IFLAG_REPAIR);
+}
+
+bool
+can_force_rebuild(
+ struct scrub_ctx *ctx)
+{
+ return __scrub_test(ctx, XFS_SCRUB_TYPE_PROBE,
+ XFS_SCRUB_IFLAG_REPAIR | XFS_SCRUB_IFLAG_FORCE_REBUILD);
}
/* General repair routines. */
@@ -624,6 +623,8 @@ xfs_repair_metadata(
assert(!debug_tweak_on("XFS_SCRUB_NO_KERNEL"));
meta.sm_type = aitem->type;
meta.sm_flags = aitem->flags | XFS_SCRUB_IFLAG_REPAIR;
+ if (use_force_rebuild)
+ meta.sm_flags |= XFS_SCRUB_IFLAG_FORCE_REBUILD;
switch (xfrog_scrubbers[aitem->type].type) {
case XFROG_SCRUB_TYPE_AGHEADER:
case XFROG_SCRUB_TYPE_PERAG:
diff --git a/scrub/scrub.h b/scrub/scrub.h
index fccd82f2155..023069ee066 100644
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -33,6 +33,7 @@ bool can_scrub_attr(struct scrub_ctx *ctx);
bool can_scrub_symlink(struct scrub_ctx *ctx);
bool can_scrub_parent(struct scrub_ctx *ctx);
bool xfs_can_repair(struct scrub_ctx *ctx);
+bool can_force_rebuild(struct scrub_ctx *ctx);
int scrub_file(struct scrub_ctx *ctx, int fd, const struct xfs_bulkstat *bstat,
unsigned int type, struct action_list *alist);
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index 7a0411b0cc8..597be59f9f9 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -157,6 +157,9 @@ bool stdout_isatty;
*/
bool is_service;
+/* Set to true if the kernel supports XFS_SCRUB_IFLAG_FORCE_REBUILD */
+bool use_force_rebuild;
+
#define SCRUB_RET_SUCCESS (0) /* no problems left behind */
#define SCRUB_RET_CORRUPT (1) /* corruption remains on fs */
#define SCRUB_RET_UNOPTIMIZED (2) /* fs could be optimized */
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index f6712d368c6..0d6b9dad2c9 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -21,6 +21,7 @@ extern bool want_fstrim;
extern bool stderr_isatty;
extern bool stdout_isatty;
extern bool is_service;
+extern bool use_force_rebuild;
enum scrub_mode {
SCRUB_MODE_DRY_RUN,
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata
2023-09-25 21:58 ` [PATCHSET v26.2 0/3] xfsprogs: force rebuilding of metadata Darrick J. Wong
` (2 preceding siblings ...)
2023-09-25 21:59 ` [PATCH 3/3] xfs_scrub: try to use XFS_SCRUB_IFLAG_FORCE_REBUILD Darrick J. Wong
@ 2023-10-03 11:33 ` Carlos Maiolino
3 siblings, 0 replies; 5+ messages in thread
From: Carlos Maiolino @ 2023-10-03 11:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Mon, Sep 25, 2023 at 02:58:44PM -0700, Darrick J. Wong wrote:
> Hi all,
>
> This patchset adds a new IFLAG to the scrub ioctl so that userspace can
> force a rebuild of an otherwise consistent piece of metadata. This will
> eventually enable the use of online repair to relocate metadata during a
> filesystem reorganization (e.g. shrink). For now, it facilitates stress
> testing of online repair without needing the debugging knobs to be
> enabled.
Series look fine, although it requires a libxfs-sync to v6.6 before it can be
applied/tested.
Once I do a libxfs sync for v6.6 I'll apply/test this series.
By now, consider it a conditional RwB :)
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Carlos
>
> If you're going to start using this code, I strongly recommend pulling
> from my git trees, which are linked below.
>
> This has been running on the djcloud for months with no problems. Enjoy!
> Comments and questions are, as always, welcome.
>
> --D
>
> kernel git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=repair-force-rebuild
>
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=repair-force-rebuild
>
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=repair-force-rebuild
> ---
> io/scrub.c | 24 ++++++++++++++-------
> man/man8/xfs_io.8 | 3 +++
> scrub/phase1.c | 28 ++++++++++++++++++++++++
> scrub/scrub.c | 61 ++++++++++++++++++++++++++++-------------------------
> scrub/scrub.h | 1 +
> scrub/xfs_scrub.c | 3 +++
> scrub/xfs_scrub.h | 1 +
> 7 files changed, 84 insertions(+), 37 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread