[PATCH 0/5] xfsprogs: scrub filesystem summary counters

[PATCH 0/5] xfsprogs: scrub filesystem summary counters

[Darrick J. Wong]

Hi all,

This patch series introduces a totally new filesystem summary counter
online scrub feature.  Whereas previous iterations froze the filesystem
to count inodes and free blocks, this one drastically reduces overhead
by loosening its precision somewhat.  Instead of freezing the fs, we
race the expected summary counter calculation with normal fs operations
and use thresholding to check that the counters are in the ballpark,
where ballpark means "off by less than 6% or so".

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  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=scrub-summary-counters

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=scrub-summary-counters

[PATCH 1/5] xfs_scrub: remove unnecessary fd parameter from file scrubbers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

xfs_scrub's scrub ioctl wrapper functions always take a scrub_ctx and an
fd, but we always set the fd to ctx->mnt.fd.  Remove the redundant
parameter.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/phase3.c |   10 +++++-----
 scrub/scrub.c  |   34 ++++++++++++----------------------
 scrub/scrub.h  |   16 ++++++++--------
 3 files changed, 25 insertions(+), 35 deletions(-)


diff --git a/scrub/phase3.c b/scrub/phase3.c
index 5eff7907..81c64cd1 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -28,12 +28,12 @@
 static bool
 xfs_scrub_fd(
 	struct scrub_ctx	*ctx,
-	bool			(*fn)(struct scrub_ctx *, uint64_t,
-				      uint32_t, int, struct xfs_action_list *),
+	bool			(*fn)(struct scrub_ctx *ctx, uint64_t ino,
+				      uint32_t gen, struct xfs_action_list *a),
 	struct xfs_bstat	*bs,
 	struct xfs_action_list	*alist)
 {
-	return fn(ctx, bs->bs_ino, bs->bs_gen, ctx->mnt.fd, alist);
+	return fn(ctx, bs->bs_ino, bs->bs_gen, alist);
 }
 
 struct scrub_inode_ctx {
@@ -114,8 +114,8 @@ xfs_scrub_inode(
 
 	if (S_ISLNK(bstat->bs_mode)) {
 		/* Check symlink contents. */
-		moveon = xfs_scrub_symlink(ctx, bstat->bs_ino,
-				bstat->bs_gen, ctx->mnt.fd, &alist);
+		moveon = xfs_scrub_symlink(ctx, bstat->bs_ino, bstat->bs_gen,
+				&alist);
 	} else if (S_ISDIR(bstat->bs_mode)) {
 		/* Check the directory entries. */
 		moveon = xfs_scrub_fd(ctx, xfs_scrub_dir, bstat, &alist);
diff --git a/scrub/scrub.c b/scrub/scrub.c
index ac67f8ec..62edc361 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -176,7 +176,6 @@ xfs_scrub_warn_incomplete_scrub(
 static enum check_outcome
 xfs_check_metadata(
 	struct scrub_ctx		*ctx,
-	int				fd,
 	struct xfs_scrub_metadata	*meta,
 	bool				is_inode)
 {
@@ -191,7 +190,7 @@ xfs_check_metadata(
 
 	dbg_printf("check %s flags %xh\n", buf, meta->sm_flags);
 retry:
-	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, meta);
+	error = ioctl(ctx->mnt.fd, XFS_IOC_SCRUB_METADATA, meta);
 	if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !error)
 		meta->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
 	if (error) {
@@ -363,7 +362,7 @@ xfs_scrub_metadata(
 		background_sleep();
 
 		/* Check the item. */
-		fix = xfs_check_metadata(ctx, ctx->mnt.fd, &meta, false);
+		fix = xfs_check_metadata(ctx, &meta, false);
 		progress_add(1);
 		switch (fix) {
 		case CHECK_ABORT:
@@ -399,7 +398,7 @@ xfs_scrub_primary_super(
 	enum check_outcome		fix;
 
 	/* Check the item. */
-	fix = xfs_check_metadata(ctx, ctx->mnt.fd, &meta, false);
+	fix = xfs_check_metadata(ctx, &meta, false);
 	switch (fix) {
 	case CHECK_ABORT:
 		return false;
@@ -478,7 +477,6 @@ __xfs_scrub_file(
 	struct scrub_ctx		*ctx,
 	uint64_t			ino,
 	uint32_t			gen,
-	int				fd,
 	unsigned int			type,
 	struct xfs_action_list		*alist)
 {
@@ -493,7 +491,7 @@ __xfs_scrub_file(
 	meta.sm_gen = gen;
 
 	/* Scrub the piece of metadata. */
-	fix = xfs_check_metadata(ctx, fd, &meta, true);
+	fix = xfs_check_metadata(ctx, &meta, true);
 	if (fix == CHECK_ABORT)
 		return false;
 	if (fix == CHECK_DONE)
@@ -507,10 +505,9 @@ xfs_scrub_inode_fields(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_INODE, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_INODE, alist);
 }
 
 bool
@@ -518,10 +515,9 @@ xfs_scrub_data_fork(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_BMBTD, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTD, alist);
 }
 
 bool
@@ -529,10 +525,9 @@ xfs_scrub_attr_fork(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_BMBTA, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTA, alist);
 }
 
 bool
@@ -540,10 +535,9 @@ xfs_scrub_cow_fork(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_BMBTC, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTC, alist);
 }
 
 bool
@@ -551,10 +545,9 @@ xfs_scrub_dir(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_DIR, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_DIR, alist);
 }
 
 bool
@@ -562,10 +555,9 @@ xfs_scrub_attr(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_XATTR, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_XATTR, alist);
 }
 
 bool
@@ -573,10 +565,9 @@ xfs_scrub_symlink(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_SYMLINK, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_SYMLINK, alist);
 }
 
 bool
@@ -584,10 +575,9 @@ xfs_scrub_parent(
 	struct scrub_ctx	*ctx,
 	uint64_t		ino,
 	uint32_t		gen,
-	int			fd,
 	struct xfs_action_list	*alist)
 {
-	return __xfs_scrub_file(ctx, ino, gen, fd, XFS_SCRUB_TYPE_PARENT, alist);
+	return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_PARENT, alist);
 }
 
 /* Test the availability of a kernel scrub command. */
diff --git a/scrub/scrub.h b/scrub/scrub.h
index e6e3f16f..7e28b522 100644
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -36,21 +36,21 @@ bool xfs_can_scrub_parent(struct scrub_ctx *ctx);
 bool xfs_can_repair(struct scrub_ctx *ctx);
 
 bool xfs_scrub_inode_fields(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_data_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_attr_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_cow_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_dir(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_attr(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_symlink(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 bool xfs_scrub_parent(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
-		int fd, struct xfs_action_list *alist);
+		struct xfs_action_list *alist);
 
 /* Repair parameters are the scrub inputs and retry count. */
 struct action_item {

[PATCH 2/5] libfrog: share scrub headers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

xfs_io and xfs_scrub have nearly identical structures to describe scrub
types.  Combine them into a single header file.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/scrub.c       |   89 ++++++++++------------------------
 libfrog/Makefile |    2 +
 libfrog/scrub.c  |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfrog/scrub.h  |   29 +++++++++++
 scrub/scrub.c    |  141 +++++++++++++++---------------------------------------
 5 files changed, 238 insertions(+), 163 deletions(-)
 create mode 100644 libfrog/scrub.c
 create mode 100644 libfrog/scrub.h


diff --git a/io/scrub.c b/io/scrub.c
index 9d1c62b5..fc22ba49 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -10,55 +10,17 @@
 #include "input.h"
 #include "init.h"
 #include "libfrog/paths.h"
+#include "libfrog/fsgeom.h"
+#include "libfrog/scrub.h"
 #include "io.h"
 
 static struct cmdinfo scrub_cmd;
 static struct cmdinfo repair_cmd;
 
-/* Type info and names for the scrub types. */
-enum scrub_type {
-	ST_NONE,	/* disabled */
-	ST_PERAG,	/* per-AG metadata */
-	ST_FS,		/* per-FS metadata */
-	ST_INODE,	/* per-inode metadata */
-};
-
-struct scrub_descr {
-	const char	*name;
-	enum scrub_type	type;
-};
-
-static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
-	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
-	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
-	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
-	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
-	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
-	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
-	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
-	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
-	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
-	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
-	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
-	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
-	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
-	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
-	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
-	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
-	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
-	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
-	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
-	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
-	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
-};
-
 static void
 scrub_help(void)
 {
-	const struct scrub_descr	*d;
+	const struct xfrog_scrub_descr	*d;
 	int				i;
 
 	printf(_(
@@ -74,7 +36,7 @@ scrub_help(void)
 " 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n"
 "\n"
 " Known metadata scrub types are:"));
-	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
+	for (i = 0, d = xfrog_scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
 		printf(" %s", d->name);
 	printf("\n");
 }
@@ -87,22 +49,23 @@ scrub_ioctl(
 	uint32_t			control2)
 {
 	struct xfs_scrub_metadata	meta;
-	const struct scrub_descr	*sc;
+	const struct xfrog_scrub_descr	*sc;
 	int				error;
 
-	sc = &scrubbers[type];
+	sc = &xfrog_scrubbers[type];
 	memset(&meta, 0, sizeof(meta));
 	meta.sm_type = type;
 	switch (sc->type) {
-	case ST_PERAG:
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		meta.sm_agno = control;
 		break;
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		meta.sm_ino = control;
 		meta.sm_gen = control2;
 		break;
-	case ST_NONE:
-	case ST_FS:
+	case XFROG_SCRUB_TYPE_NONE:
+	case XFROG_SCRUB_TYPE_FS:
 		/* no control parameters */
 		break;
 	}
@@ -135,7 +98,7 @@ parse_args(
 	int				i, c;
 	uint64_t			control = 0;
 	uint32_t			control2 = 0;
-	const struct scrub_descr	*d = NULL;
+	const struct xfrog_scrub_descr	*d = NULL;
 
 	while ((c = getopt(argc, argv, "")) != EOF) {
 		switch (c) {
@@ -146,7 +109,7 @@ parse_args(
 	if (optind > argc - 1)
 		return command_usage(cmdinfo);
 
-	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
+	for (i = 0, d = xfrog_scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
 		if (strcmp(d->name, argv[optind]) == 0) {
 			type = i;
 			break;
@@ -159,7 +122,7 @@ parse_args(
 	optind++;
 
 	switch (d->type) {
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		if (optind == argc) {
 			control = 0;
 			control2 = 0;
@@ -184,7 +147,8 @@ parse_args(
 			return 0;
 		}
 		break;
-	case ST_PERAG:
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		if (optind != argc - 1) {
 			fprintf(stderr,
 				_("Must specify one AG number.\n"));
@@ -197,8 +161,8 @@ parse_args(
 			return 0;
 		}
 		break;
-	case ST_FS:
-	case ST_NONE:
+	case XFROG_SCRUB_TYPE_FS:
+	case XFROG_SCRUB_TYPE_NONE:
 		if (optind != argc) {
 			fprintf(stderr,
 				_("No parameters allowed.\n"));
@@ -241,7 +205,7 @@ scrub_init(void)
 static void
 repair_help(void)
 {
-	const struct scrub_descr	*d;
+	const struct xfrog_scrub_descr	*d;
 	int				i;
 
 	printf(_(
@@ -257,7 +221,7 @@ repair_help(void)
 " 'repair bmapbtd 128 13525' - repairs the extent map of inode 128 gen 13525.\n"
 "\n"
 " Known metadata repairs types are:"));
-	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
+	for (i = 0, d = xfrog_scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
 		printf(" %s", d->name);
 	printf("\n");
 }
@@ -270,22 +234,23 @@ repair_ioctl(
 	uint32_t			control2)
 {
 	struct xfs_scrub_metadata	meta;
-	const struct scrub_descr	*sc;
+	const struct xfrog_scrub_descr	*sc;
 	int				error;
 
-	sc = &scrubbers[type];
+	sc = &xfrog_scrubbers[type];
 	memset(&meta, 0, sizeof(meta));
 	meta.sm_type = type;
 	switch (sc->type) {
-	case ST_PERAG:
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		meta.sm_agno = control;
 		break;
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		meta.sm_ino = control;
 		meta.sm_gen = control2;
 		break;
-	case ST_NONE:
-	case ST_FS:
+	case XFROG_SCRUB_TYPE_NONE:
+	case XFROG_SCRUB_TYPE_FS:
 		/* no control parameters */
 		break;
 	}
diff --git a/libfrog/Makefile b/libfrog/Makefile
index 25b5a03c..de67bf00 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -23,6 +23,7 @@ paths.c \
 projects.c \
 ptvar.c \
 radix-tree.c \
+scrub.c \
 topology.c \
 util.c \
 workqueue.c
@@ -41,6 +42,7 @@ paths.h \
 projects.h \
 ptvar.h \
 radix-tree.h \
+scrub.h \
 topology.h \
 workqueue.h
 
diff --git a/libfrog/scrub.c b/libfrog/scrub.c
new file mode 100644
index 00000000..228e4339
--- /dev/null
+++ b/libfrog/scrub.c
@@ -0,0 +1,140 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Oracle, Inc.
+ * All Rights Reserved.
+ */
+#include "xfs.h"
+#include "fsgeom.h"
+#include "scrub.h"
+
+/* These must correspond to XFS_SCRUB_TYPE_ */
+const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR] = {
+	[XFS_SCRUB_TYPE_PROBE] = {
+		.name	= "probe",
+		.descr	= "metadata",
+		.type	= XFROG_SCRUB_TYPE_NONE,
+	},
+	[XFS_SCRUB_TYPE_SB] = {
+		.name	= "sb",
+		.descr	= "superblock",
+		.type	= XFROG_SCRUB_TYPE_AGHEADER,
+	},
+	[XFS_SCRUB_TYPE_AGF] = {
+		.name	= "agf",
+		.descr	= "free space header",
+		.type	= XFROG_SCRUB_TYPE_AGHEADER,
+	},
+	[XFS_SCRUB_TYPE_AGFL] = {
+		.name	= "agfl",
+		.descr	= "free list",
+		.type	= XFROG_SCRUB_TYPE_AGHEADER,
+	},
+	[XFS_SCRUB_TYPE_AGI] = {
+		.name	= "agi",
+		.descr	= "inode header",
+		.type	= XFROG_SCRUB_TYPE_AGHEADER,
+	},
+	[XFS_SCRUB_TYPE_BNOBT] = {
+		.name	= "bnobt",
+		.descr	= "freesp by block btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_CNTBT] = {
+		.name	= "cntbt",
+		.descr	= "freesp by length btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_INOBT] = {
+		.name	= "inobt",
+		.descr	= "inode btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_FINOBT] = {
+		.name	= "finobt",
+		.descr	= "free inode btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_RMAPBT] = {
+		.name	= "rmapbt",
+		.descr	= "reverse mapping btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_REFCNTBT] = {
+		.name	= "refcountbt",
+		.descr	= "reference count btree",
+		.type	= XFROG_SCRUB_TYPE_PERAG,
+	},
+	[XFS_SCRUB_TYPE_INODE] = {
+		.name	= "inode",
+		.descr	= "inode record",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_BMBTD] = {
+		.name	= "bmapbtd",
+		.descr	= "data block map",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_BMBTA] = {
+		.name	= "bmapbta",
+		.descr	= "attr block map",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_BMBTC] = {
+		.name	= "bmapbtc",
+		.descr	= "CoW block map",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_DIR] = {
+		.name	= "directory",
+		.descr	= "directory entries",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_XATTR] = {
+		.name	= "xattr",
+		.descr	= "extended attributes",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_SYMLINK] = {
+		.name	= "symlink",
+		.descr	= "symbolic link",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_PARENT] = {
+		.name	= "parent",
+		.descr	= "parent pointer",
+		.type	= XFROG_SCRUB_TYPE_INODE,
+	},
+	[XFS_SCRUB_TYPE_RTBITMAP] = {
+		.name	= "rtbitmap",
+		.descr	= "realtime bitmap",
+		.type	= XFROG_SCRUB_TYPE_FS,
+	},
+	[XFS_SCRUB_TYPE_RTSUM] = {
+		.name	= "rtsummary",
+		.descr	= "realtime summary",
+		.type	= XFROG_SCRUB_TYPE_FS,
+	},
+	[XFS_SCRUB_TYPE_UQUOTA] = {
+		.name	= "usrquota",
+		.descr	= "user quotas",
+		.type	= XFROG_SCRUB_TYPE_FS,
+	},
+	[XFS_SCRUB_TYPE_GQUOTA] = {
+		.name	= "grpquota",
+		.descr	= "group quotas",
+		.type	= XFROG_SCRUB_TYPE_FS,
+	},
+	[XFS_SCRUB_TYPE_PQUOTA] = {
+		.name	= "prjquota",
+		.descr	= "project quotas",
+		.type	= XFROG_SCRUB_TYPE_FS,
+	},
+};
+
+int
+xfrog_scrub_metadata(
+	struct xfs_fd			*xfd,
+	struct xfs_scrub_metadata	*meta)
+{
+	return ioctl(xfd->fd, XFS_IOC_SCRUB_METADATA, meta);
+}
diff --git a/libfrog/scrub.h b/libfrog/scrub.h
new file mode 100644
index 00000000..6fda8975
--- /dev/null
+++ b/libfrog/scrub.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Oracle, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __LIBFROG_SCRUB_H__
+#define __LIBFROG_SCRUB_H__
+
+/* Type info and names for the scrub types. */
+enum xfrog_scrub_type {
+	XFROG_SCRUB_TYPE_NONE,		/* not metadata */
+	XFROG_SCRUB_TYPE_AGHEADER,	/* per-AG header */
+	XFROG_SCRUB_TYPE_PERAG,		/* per-AG metadata */
+	XFROG_SCRUB_TYPE_FS,		/* per-FS metadata */
+	XFROG_SCRUB_TYPE_INODE,		/* per-inode metadata */
+};
+
+/* Catalog of scrub types and names, indexed by XFS_SCRUB_TYPE_* */
+struct xfrog_scrub_descr {
+	const char		*name;
+	const char		*descr;
+	enum xfrog_scrub_type	type;
+};
+
+extern const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR];
+
+int xfrog_scrub_metadata(struct xfs_fd *xfd, struct xfs_scrub_metadata *meta);
+
+#endif	/* __LIBFROG_SCRUB_H__ */
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 62edc361..153d29d5 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -12,6 +12,8 @@
 #include <sys/statvfs.h>
 #include "list.h"
 #include "libfrog/paths.h"
+#include "libfrog/fsgeom.h"
+#include "libfrog/scrub.h"
 #include "xfs_scrub.h"
 #include "common.h"
 #include "progress.h"
@@ -21,93 +23,28 @@
 
 /* Online scrub and repair wrappers. */
 
-/* Type info and names for the scrub types. */
-enum scrub_type {
-	ST_NONE,	/* disabled */
-	ST_AGHEADER,	/* per-AG header */
-	ST_PERAG,	/* per-AG metadata */
-	ST_FS,		/* per-FS metadata */
-	ST_INODE,	/* per-inode metadata */
-};
-struct scrub_descr {
-	const char	*name;
-	enum scrub_type	type;
-};
-
-/* These must correspond to XFS_SCRUB_TYPE_ */
-static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
-	[XFS_SCRUB_TYPE_PROBE] =
-		{"metadata",				ST_NONE},
-	[XFS_SCRUB_TYPE_SB] =
-		{"superblock",				ST_AGHEADER},
-	[XFS_SCRUB_TYPE_AGF] =
-		{"free space header",			ST_AGHEADER},
-	[XFS_SCRUB_TYPE_AGFL] =
-		{"free list",				ST_AGHEADER},
-	[XFS_SCRUB_TYPE_AGI] =
-		{"inode header",			ST_AGHEADER},
-	[XFS_SCRUB_TYPE_BNOBT] =
-		{"freesp by block btree",		ST_PERAG},
-	[XFS_SCRUB_TYPE_CNTBT] =
-		{"freesp by length btree",		ST_PERAG},
-	[XFS_SCRUB_TYPE_INOBT] =
-		{"inode btree",				ST_PERAG},
-	[XFS_SCRUB_TYPE_FINOBT] =
-		{"free inode btree",			ST_PERAG},
-	[XFS_SCRUB_TYPE_RMAPBT] =
-		{"reverse mapping btree",		ST_PERAG},
-	[XFS_SCRUB_TYPE_REFCNTBT] =
-		{"reference count btree",		ST_PERAG},
-	[XFS_SCRUB_TYPE_INODE] =
-		{"inode record",			ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTD] =
-		{"data block map",			ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTA] =
-		{"attr block map",			ST_INODE},
-	[XFS_SCRUB_TYPE_BMBTC] =
-		{"CoW block map",			ST_INODE},
-	[XFS_SCRUB_TYPE_DIR] =
-		{"directory entries",			ST_INODE},
-	[XFS_SCRUB_TYPE_XATTR] =
-		{"extended attributes",			ST_INODE},
-	[XFS_SCRUB_TYPE_SYMLINK] =
-		{"symbolic link",			ST_INODE},
-	[XFS_SCRUB_TYPE_PARENT] =
-		{"parent pointer",			ST_INODE},
-	[XFS_SCRUB_TYPE_RTBITMAP] =
-		{"realtime bitmap",			ST_FS},
-	[XFS_SCRUB_TYPE_RTSUM] =
-		{"realtime summary",			ST_FS},
-	[XFS_SCRUB_TYPE_UQUOTA] =
-		{"user quotas",				ST_FS},
-	[XFS_SCRUB_TYPE_GQUOTA] =
-		{"group quotas",			ST_FS},
-	[XFS_SCRUB_TYPE_PQUOTA] =
-		{"project quotas",			ST_FS},
-};
-
 /* Format a scrub description. */
 static void
 format_scrub_descr(
 	char				*buf,
 	size_t				buflen,
 	struct xfs_scrub_metadata	*meta,
-	const struct scrub_descr	*sc)
+	const struct xfrog_scrub_descr	*sc)
 {
 	switch (sc->type) {
-	case ST_AGHEADER:
-	case ST_PERAG:
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		snprintf(buf, buflen, _("AG %u %s"), meta->sm_agno,
-				_(sc->name));
+				_(sc->descr));
 		break;
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		snprintf(buf, buflen, _("Inode %"PRIu64" %s"),
-				(uint64_t)meta->sm_ino, _(sc->name));
+				(uint64_t)meta->sm_ino, _(sc->descr));
 		break;
-	case ST_FS:
-		snprintf(buf, buflen, _("%s"), _(sc->name));
+	case XFROG_SCRUB_TYPE_FS:
+		snprintf(buf, buflen, _("%s"), _(sc->descr));
 		break;
-	case ST_NONE:
+	case XFROG_SCRUB_TYPE_NONE:
 		assert(0);
 		break;
 	}
@@ -186,11 +123,12 @@ xfs_check_metadata(
 
 	assert(!debug_tweak_on("XFS_SCRUB_NO_KERNEL"));
 	assert(meta->sm_type < XFS_SCRUB_TYPE_NR);
-	format_scrub_descr(buf, DESCR_BUFSZ, meta, &scrubbers[meta->sm_type]);
+	format_scrub_descr(buf, DESCR_BUFSZ, meta,
+			&xfrog_scrubbers[meta->sm_type]);
 
 	dbg_printf("check %s flags %xh\n", buf, meta->sm_flags);
 retry:
-	error = ioctl(ctx->mnt.fd, XFS_IOC_SCRUB_METADATA, meta);
+	error = xfrog_scrub_metadata(&ctx->mnt, meta);
 	if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !error)
 		meta->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
 	if (error) {
@@ -296,7 +234,7 @@ xfs_scrub_report_preen_triggers(
 			ctx->preen_triggers[i] = false;
 			pthread_mutex_unlock(&ctx->lock);
 			str_info(ctx, ctx->mntpoint,
-_("Optimizations of %s are possible."), scrubbers[i].name);
+_("Optimizations of %s are possible."), _(xfrog_scrubbers[i].descr));
 		} else {
 			pthread_mutex_unlock(&ctx->lock);
 		}
@@ -321,12 +259,12 @@ xfs_scrub_save_repair(
 	memset(aitem, 0, sizeof(*aitem));
 	aitem->type = meta->sm_type;
 	aitem->flags = meta->sm_flags;
-	switch (scrubbers[meta->sm_type].type) {
-	case ST_AGHEADER:
-	case ST_PERAG:
+	switch (xfrog_scrubbers[meta->sm_type].type) {
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		aitem->agno = meta->sm_agno;
 		break;
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		aitem->ino = meta->sm_ino;
 		aitem->gen = meta->sm_gen;
 		break;
@@ -342,16 +280,16 @@ xfs_scrub_save_repair(
 static bool
 xfs_scrub_metadata(
 	struct scrub_ctx		*ctx,
-	enum scrub_type			scrub_type,
+	enum xfrog_scrub_type		scrub_type,
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
 	struct xfs_scrub_metadata	meta = {0};
-	const struct scrub_descr	*sc;
+	const struct xfrog_scrub_descr	*sc;
 	enum check_outcome		fix;
 	int				type;
 
-	sc = scrubbers;
+	sc = xfrog_scrubbers;
 	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
 		if (sc->type != scrub_type)
 			continue;
@@ -423,7 +361,7 @@ xfs_scrub_ag_headers(
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, ST_AGHEADER, agno, alist);
+	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_AGHEADER, agno, alist);
 }
 
 /* Scrub each AG's metadata btrees. */
@@ -433,7 +371,7 @@ xfs_scrub_ag_metadata(
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, ST_PERAG, agno, alist);
+	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_PERAG, agno, alist);
 }
 
 /* Scrub whole-FS metadata btrees. */
@@ -442,7 +380,7 @@ xfs_scrub_fs_metadata(
 	struct scrub_ctx		*ctx,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, ST_FS, 0, alist);
+	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_FS, 0, alist);
 }
 
 /* How many items do we have to check? */
@@ -450,18 +388,18 @@ unsigned int
 xfs_scrub_estimate_ag_work(
 	struct scrub_ctx		*ctx)
 {
-	const struct scrub_descr	*sc;
+	const struct xfrog_scrub_descr	*sc;
 	int				type;
 	unsigned int			estimate = 0;
 
-	sc = scrubbers;
+	sc = xfrog_scrubbers;
 	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
 		switch (sc->type) {
-		case ST_AGHEADER:
-		case ST_PERAG:
+		case XFROG_SCRUB_TYPE_AGHEADER:
+		case XFROG_SCRUB_TYPE_PERAG:
 			estimate += ctx->mnt.fsgeom.agcount;
 			break;
-		case ST_FS:
+		case XFROG_SCRUB_TYPE_FS:
 			estimate++;
 			break;
 		default:
@@ -484,7 +422,7 @@ __xfs_scrub_file(
 	enum check_outcome		fix;
 
 	assert(type < XFS_SCRUB_TYPE_NR);
-	assert(scrubbers[type].type == ST_INODE);
+	assert(xfrog_scrubbers[type].type == XFROG_SCRUB_TYPE_INODE);
 
 	meta.sm_type = type;
 	meta.sm_ino = ino;
@@ -605,7 +543,7 @@ __xfs_scrub_test(
 	meta.sm_type = type;
 	if (repair)
 		meta.sm_flags |= XFS_SCRUB_IFLAG_REPAIR;
-	error = ioctl(ctx->mnt.fd, XFS_IOC_SCRUB_METADATA, &meta);
+	error = xfrog_scrub_metadata(&ctx->mnt, &meta);
 	if (!error)
 		return true;
 	switch (errno) {
@@ -622,7 +560,7 @@ _("Filesystem is mounted norecovery; cannot proceed."));
 		if (debug || verbose)
 			str_info(ctx, ctx->mntpoint,
 _("Kernel %s %s facility not detected."),
-					_(scrubbers[type].name),
+					_(xfrog_scrubbers[type].descr),
 					repair ? _("repair") : _("scrub"));
 		return false;
 	case ENOENT:
@@ -709,12 +647,12 @@ 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;
-	switch (scrubbers[aitem->type].type) {
-	case ST_AGHEADER:
-	case ST_PERAG:
+	switch (xfrog_scrubbers[aitem->type].type) {
+	case XFROG_SCRUB_TYPE_AGHEADER:
+	case XFROG_SCRUB_TYPE_PERAG:
 		meta.sm_agno = aitem->agno;
 		break;
-	case ST_INODE:
+	case XFROG_SCRUB_TYPE_INODE:
 		meta.sm_ino = aitem->ino;
 		meta.sm_gen = aitem->gen;
 		break;
@@ -726,14 +664,15 @@ xfs_repair_metadata(
 		return CHECK_RETRY;
 
 	memcpy(&oldm, &meta, sizeof(oldm));
-	format_scrub_descr(buf, DESCR_BUFSZ, &meta, &scrubbers[meta.sm_type]);
+	format_scrub_descr(buf, DESCR_BUFSZ, &meta,
+			&xfrog_scrubbers[meta.sm_type]);
 
 	if (needs_repair(&meta))
 		str_info(ctx, buf, _("Attempting repair."));
 	else if (debug || verbose)
 		str_info(ctx, buf, _("Attempting optimization."));
 
-	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
+	error = xfrog_scrub_metadata(&ctx->mnt, &meta);
 	if (error) {
 		switch (errno) {
 		case EDEADLOCK:

[PATCH 3/5] libfrog: add online scrub/repair for superblock counters

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Wire up the new superblock summary counter ioctls.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/scrub.c |    6 ++++++
 libfrog/scrub.h |    7 +++++++
 scrub/scrub.c   |    2 ++
 3 files changed, 15 insertions(+)


diff --git a/libfrog/scrub.c b/libfrog/scrub.c
index 228e4339..04e44565 100644
--- a/libfrog/scrub.c
+++ b/libfrog/scrub.c
@@ -129,6 +129,12 @@ const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR] = {
 		.descr	= "project quotas",
 		.type	= XFROG_SCRUB_TYPE_FS,
 	},
+	[XFS_SCRUB_TYPE_FSCOUNTERS] = {
+		.name	= "fscounters",
+		.descr	= "filesystem summary counters",
+		.type	= XFROG_SCRUB_TYPE_FS,
+		.flags	= XFROG_SCRUB_DESCR_SUMMARY,
+	},
 };
 
 int
diff --git a/libfrog/scrub.h b/libfrog/scrub.h
index 6fda8975..e43d8c24 100644
--- a/libfrog/scrub.h
+++ b/libfrog/scrub.h
@@ -20,8 +20,15 @@ struct xfrog_scrub_descr {
 	const char		*name;
 	const char		*descr;
 	enum xfrog_scrub_type	type;
+	unsigned int		flags;
 };
 
+/*
+ * The type of metadata checked by this scrubber is a summary of other types
+ * of metadata.  This scrubber should be run after all the others.
+ */
+#define XFROG_SCRUB_DESCR_SUMMARY	(1 << 0)
+
 extern const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR];
 
 int xfrog_scrub_metadata(struct xfs_fd *xfd, struct xfs_scrub_metadata *meta);
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 153d29d5..083ed9a1 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -293,6 +293,8 @@ xfs_scrub_metadata(
 	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
 		if (sc->type != scrub_type)
 			continue;
+		if (sc->flags & XFROG_SCRUB_DESCR_SUMMARY)
+			continue;
 
 		meta.sm_type = type;
 		meta.sm_flags = 0;

[PATCH 4/5] xfs_scrub: separate internal metadata scrub functions

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor xfs_scrub_metadata into two functions -- one to make a single
call xfs_check_metadata, and the second retains the loop logic.  The
name is a little easy to confuse with other functions, so rename it to
reflect what it actually does: scrub all internal metadata of a given
class (AG header, AG metadata, FS metadata).  No functional changes.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/scrub.c |   95 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 47 insertions(+), 48 deletions(-)


diff --git a/scrub/scrub.c b/scrub/scrub.c
index 083ed9a1..b1927f38 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -276,47 +276,64 @@ xfs_scrub_save_repair(
 	return true;
 }
 
+/* Scrub non-inode metadata, saving corruption reports for later. */
+static int
+xfs_scrub_meta(
+	struct scrub_ctx		*ctx,
+	unsigned int			type,
+	xfs_agnumber_t			agno,
+	struct xfs_action_list		*alist)
+{
+	struct xfs_scrub_metadata	meta = {
+		.sm_type		= type,
+		.sm_agno		= agno,
+	};
+	enum check_outcome		fix;
+
+	background_sleep();
+
+	/* Check the item. */
+	fix = xfs_check_metadata(ctx, &meta, false);
+	progress_add(1);
+
+	switch (fix) {
+	case CHECK_ABORT:
+		return ECANCELED;
+	case CHECK_REPAIR:
+		if (!xfs_scrub_save_repair(ctx, alist, &meta))
+			return ENOMEM;
+		/* fall through */
+	case CHECK_DONE:
+		return 0;
+	default:
+		/* CHECK_RETRY should never happen. */
+		abort();
+	}
+}
+
 /* Scrub metadata, saving corruption reports for later. */
 static bool
-xfs_scrub_metadata(
+xfs_scrub_meta_type(
 	struct scrub_ctx		*ctx,
 	enum xfrog_scrub_type		scrub_type,
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
-	struct xfs_scrub_metadata	meta = {0};
 	const struct xfrog_scrub_descr	*sc;
-	enum check_outcome		fix;
-	int				type;
+	unsigned int			type;
 
 	sc = xfrog_scrubbers;
 	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
+		int			ret;
+
 		if (sc->type != scrub_type)
 			continue;
 		if (sc->flags & XFROG_SCRUB_DESCR_SUMMARY)
 			continue;
 
-		meta.sm_type = type;
-		meta.sm_flags = 0;
-		meta.sm_agno = agno;
-		background_sleep();
-
-		/* Check the item. */
-		fix = xfs_check_metadata(ctx, &meta, false);
-		progress_add(1);
-		switch (fix) {
-		case CHECK_ABORT:
+		ret = xfs_scrub_meta(ctx, type, agno, alist);
+		if (ret)
 			return false;
-		case CHECK_REPAIR:
-			if (!xfs_scrub_save_repair(ctx, alist, &meta))
-				return false;
-			/* fall through */
-		case CHECK_DONE:
-			continue;
-		case CHECK_RETRY:
-			abort();
-			break;
-		}
 	}
 
 	return true;
@@ -332,28 +349,10 @@ xfs_scrub_primary_super(
 	struct scrub_ctx		*ctx,
 	struct xfs_action_list		*alist)
 {
-	struct xfs_scrub_metadata	meta = {
-		.sm_type = XFS_SCRUB_TYPE_SB,
-	};
-	enum check_outcome		fix;
-
-	/* Check the item. */
-	fix = xfs_check_metadata(ctx, &meta, false);
-	switch (fix) {
-	case CHECK_ABORT:
-		return false;
-	case CHECK_REPAIR:
-		if (!xfs_scrub_save_repair(ctx, alist, &meta))
-			return false;
-		/* fall through */
-	case CHECK_DONE:
-		return true;
-	case CHECK_RETRY:
-		abort();
-		break;
-	}
+	int				ret;
 
-	return true;
+	ret = xfs_scrub_meta(ctx, XFS_SCRUB_TYPE_SB, 0, alist);
+	return ret == 0;
 }
 
 /* Scrub each AG's header blocks. */
@@ -363,7 +362,7 @@ xfs_scrub_ag_headers(
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_AGHEADER, agno, alist);
+	return xfs_scrub_meta_type(ctx, XFROG_SCRUB_TYPE_AGHEADER, agno, alist);
 }
 
 /* Scrub each AG's metadata btrees. */
@@ -373,7 +372,7 @@ xfs_scrub_ag_metadata(
 	xfs_agnumber_t			agno,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_PERAG, agno, alist);
+	return xfs_scrub_meta_type(ctx, XFROG_SCRUB_TYPE_PERAG, agno, alist);
 }
 
 /* Scrub whole-FS metadata btrees. */
@@ -382,7 +381,7 @@ xfs_scrub_fs_metadata(
 	struct scrub_ctx		*ctx,
 	struct xfs_action_list		*alist)
 {
-	return xfs_scrub_metadata(ctx, XFROG_SCRUB_TYPE_FS, 0, alist);
+	return xfs_scrub_meta_type(ctx, XFROG_SCRUB_TYPE_FS, 0, alist);
 }
 
 /* How many items do we have to check? */

[PATCH 5/5] xfs_scrub: check summary counters

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Teach scrub to ask the kernel to check and repair summary counters
during phase 7.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/phase4.c |   12 ++++++++++++
 scrub/phase7.c |   14 ++++++++++++++
 scrub/repair.c |    3 +++
 scrub/scrub.c  |   12 ++++++++++++
 scrub/scrub.h  |    2 ++
 5 files changed, 43 insertions(+)


diff --git a/scrub/phase4.c b/scrub/phase4.c
index 589777f6..25fedc83 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -107,6 +107,18 @@ bool
 xfs_repair_fs(
 	struct scrub_ctx		*ctx)
 {
+	bool				moveon;
+
+	/*
+	 * Check the summary counters early.  Normally we do this during phase
+	 * seven, but some of the cross-referencing requires fairly-accurate
+	 * counters, so counter repairs have to be put on the list now so that
+	 * they get fixed before we stop retrying unfixed metadata repairs.
+	 */
+	moveon = xfs_scrub_fs_summary(ctx, &ctx->action_lists[0]);
+	if (!moveon)
+		return false;
+
 	return xfs_process_action_items(ctx);
 }
 
diff --git a/scrub/phase7.c b/scrub/phase7.c
index f82b60d6..308b8bb3 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -9,10 +9,13 @@
 #include <sys/statvfs.h>
 #include "libfrog/paths.h"
 #include "libfrog/ptvar.h"
+#include "list.h"
 #include "xfs_scrub.h"
 #include "common.h"
+#include "scrub.h"
 #include "fscounters.h"
 #include "spacemap.h"
+#include "repair.h"
 
 /* Phase 7: Check summary counters. */
 
@@ -91,6 +94,7 @@ xfs_scan_summary(
 	struct scrub_ctx	*ctx)
 {
 	struct summary_counts	totalcount = {0};
+	struct xfs_action_list	alist;
 	struct ptvar		*ptvar;
 	unsigned long long	used_data;
 	unsigned long long	used_rt;
@@ -110,6 +114,16 @@ xfs_scan_summary(
 	int			ip;
 	int			error;
 
+	/* Check and fix the fs summary counters. */
+	xfs_action_list_init(&alist);
+	moveon = xfs_scrub_fs_summary(ctx, &alist);
+	if (!moveon)
+		return false;
+	moveon = xfs_action_list_process(ctx, ctx->mnt.fd, &alist,
+			ALP_COMPLAIN_IF_UNFIXED | ALP_NOPROGRESS);
+	if (!moveon)
+		return moveon;
+
 	/* Flush everything out to disk before we start counting. */
 	error = syncfs(ctx->mnt.fd);
 	if (error) {
diff --git a/scrub/repair.c b/scrub/repair.c
index 0e5afb20..04a9dccf 100644
--- a/scrub/repair.c
+++ b/scrub/repair.c
@@ -84,6 +84,9 @@ xfs_action_item_priority(
 	case XFS_SCRUB_TYPE_GQUOTA:
 	case XFS_SCRUB_TYPE_PQUOTA:
 		return PRIO(aitem, XFS_SCRUB_TYPE_UQUOTA);
+	case XFS_SCRUB_TYPE_FSCOUNTERS:
+		/* This should always go after AG headers no matter what. */
+		return PRIO(aitem, INT_MAX);
 	}
 	abort();
 }
diff --git a/scrub/scrub.c b/scrub/scrub.c
index b1927f38..0e30bb2f 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -384,6 +384,18 @@ xfs_scrub_fs_metadata(
 	return xfs_scrub_meta_type(ctx, XFROG_SCRUB_TYPE_FS, 0, alist);
 }
 
+/* Scrub FS summary metadata. */
+bool
+xfs_scrub_fs_summary(
+	struct scrub_ctx		*ctx,
+	struct xfs_action_list		*alist)
+{
+	int				ret;
+
+	ret = xfs_scrub_meta(ctx, XFS_SCRUB_TYPE_FSCOUNTERS, 0, alist);
+	return ret == 0;
+}
+
 /* How many items do we have to check? */
 unsigned int
 xfs_scrub_estimate_ag_work(
diff --git a/scrub/scrub.h b/scrub/scrub.h
index 7e28b522..d407abb0 100644
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -25,6 +25,8 @@ bool xfs_scrub_ag_metadata(struct scrub_ctx *ctx, xfs_agnumber_t agno,
 		struct xfs_action_list *alist);
 bool xfs_scrub_fs_metadata(struct scrub_ctx *ctx,
 		struct xfs_action_list *alist);
+bool xfs_scrub_fs_summary(struct scrub_ctx *ctx,
+		struct xfs_action_list *alist);
 
 bool xfs_can_scrub_fs_metadata(struct scrub_ctx *ctx);
 bool xfs_can_scrub_inode(struct scrub_ctx *ctx);

Re: [PATCH 1/5] xfs_scrub: remove unnecessary fd parameter from file scrubbers

[Dave Chinner]

On Thu, Sep 05, 2019 at 08:33:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> xfs_scrub's scrub ioctl wrapper functions always take a scrub_ctx and an
> fd, but we always set the fd to ctx->mnt.fd.  Remove the redundant
> parameter.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  scrub/phase3.c |   10 +++++-----
>  scrub/scrub.c  |   34 ++++++++++++----------------------
>  scrub/scrub.h  |   16 ++++++++--------
>  3 files changed, 25 insertions(+), 35 deletions(-)

Nice little cleanup.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/5] libfrog: share scrub headers

[Dave Chinner]

On Thu, Sep 05, 2019 at 08:33:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> xfs_io and xfs_scrub have nearly identical structures to describe scrub
> types.  Combine them into a single header file.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  io/scrub.c       |   89 ++++++++++------------------------
>  libfrog/Makefile |    2 +
>  libfrog/scrub.c  |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libfrog/scrub.h  |   29 +++++++++++
>  scrub/scrub.c    |  141 +++++++++++++++---------------------------------------
>  5 files changed, 238 insertions(+), 163 deletions(-)
>  create mode 100644 libfrog/scrub.c
>  create mode 100644 libfrog/scrub.h

Looks fine. Not sure we needed all the xfrog prefixes, but getting
this into common code is still a win.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/5] libfrog: add online scrub/repair for superblock counters

[Dave Chinner]

On Thu, Sep 05, 2019 at 08:33:41PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Wire up the new superblock summary counter ioctls.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  libfrog/scrub.c |    6 ++++++
>  libfrog/scrub.h |    7 +++++++
>  scrub/scrub.c   |    2 ++
>  3 files changed, 15 insertions(+)

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/5] xfs_scrub: separate internal metadata scrub functions

[Dave Chinner]

On Thu, Sep 05, 2019 at 08:33:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor xfs_scrub_metadata into two functions -- one to make a single
> call xfs_check_metadata, and the second retains the loop logic.  The
> name is a little easy to confuse with other functions, so rename it to
> reflect what it actually does: scrub all internal metadata of a given
> class (AG header, AG metadata, FS metadata).  No functional changes.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Minor nit:

> +/* Scrub non-inode metadata, saving corruption reports for later. */
> +static int
> +xfs_scrub_meta(
> +	struct scrub_ctx		*ctx,
> +	unsigned int			type,
> +	xfs_agnumber_t			agno,
> +	struct xfs_action_list		*alist)
> +{
> +	struct xfs_scrub_metadata	meta = {
> +		.sm_type		= type,
> +		.sm_agno		= agno,
> +	};

This should be called xfs_scrub_meta_type() because it only
scrubs the specific type passed into it....

>  /* Scrub metadata, saving corruption reports for later. */
>  static bool
> -xfs_scrub_metadata(
> +xfs_scrub_meta_type(
>  	struct scrub_ctx		*ctx,
>  	enum xfrog_scrub_type		scrub_type,
>  	xfs_agnumber_t			agno,
>  	struct xfs_action_list		*alist)
>  {
> -	struct xfs_scrub_metadata	meta = {0};
>  	const struct xfrog_scrub_descr	*sc;
> -	enum check_outcome		fix;
> -	int				type;
> +	unsigned int			type;
>  
>  	sc = xfrog_scrubbers;
>  	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
> +		int			ret;
> +

And this should be called xfs_scrub_all_metadata() because it
walks across all the metadata types in the filesystem and calls
xfs_scrub_meta_type() for each type to scrub them one by one....

Other than that, it looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 5/5] xfs_scrub: check summary counters

[Dave Chinner]

On Thu, Sep 05, 2019 at 08:33:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Teach scrub to ask the kernel to check and repair summary counters
> during phase 7.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  scrub/phase4.c |   12 ++++++++++++
>  scrub/phase7.c |   14 ++++++++++++++
>  scrub/repair.c |    3 +++
>  scrub/scrub.c  |   12 ++++++++++++
>  scrub/scrub.h  |    2 ++
>  5 files changed, 43 insertions(+)

Looks fine (ignoring the moveon/retval stuff as it all gets cleaned
up later).

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/5] xfs_scrub: separate internal metadata scrub functions

[Darrick J. Wong]

On Tue, Sep 10, 2019 at 10:19:33AM +1000, Dave Chinner wrote:
> On Thu, Sep 05, 2019 at 08:33:47PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Refactor xfs_scrub_metadata into two functions -- one to make a single
> > call xfs_check_metadata, and the second retains the loop logic.  The
> > name is a little easy to confuse with other functions, so rename it to
> > reflect what it actually does: scrub all internal metadata of a given
> > class (AG header, AG metadata, FS metadata).  No functional changes.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Minor nit:
> 
> > +/* Scrub non-inode metadata, saving corruption reports for later. */
> > +static int
> > +xfs_scrub_meta(
> > +	struct scrub_ctx		*ctx,
> > +	unsigned int			type,
> > +	xfs_agnumber_t			agno,
> > +	struct xfs_action_list		*alist)
> > +{
> > +	struct xfs_scrub_metadata	meta = {
> > +		.sm_type		= type,
> > +		.sm_agno		= agno,
> > +	};
> 
> This should be called xfs_scrub_meta_type() because it only
> scrubs the specific type passed into it....

Ok.

> >  /* Scrub metadata, saving corruption reports for later. */
> >  static bool
> > -xfs_scrub_metadata(
> > +xfs_scrub_meta_type(
> >  	struct scrub_ctx		*ctx,
> >  	enum xfrog_scrub_type		scrub_type,
> >  	xfs_agnumber_t			agno,
> >  	struct xfs_action_list		*alist)
> >  {
> > -	struct xfs_scrub_metadata	meta = {0};
> >  	const struct xfrog_scrub_descr	*sc;
> > -	enum check_outcome		fix;
> > -	int				type;
> > +	unsigned int			type;
> >  
> >  	sc = xfrog_scrubbers;
> >  	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
> > +		int			ret;
> > +
> 
> And this should be called xfs_scrub_all_metadata() because it
> walks across all the metadata types in the filesystem and calls
> xfs_scrub_meta_type() for each type to scrub them one by one....

Ok.  I think I'll update the comments for both to make it clearer what
"type" means.

--D

> Other than that, it looks fine.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

[PATCH 3/5] libfrog: add online scrub/repair for superblock counters

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Wire up the new superblock summary counter ioctls.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 libfrog/scrub.c |    6 ++++++
 libfrog/scrub.h |    7 +++++++
 scrub/scrub.c   |    2 ++
 3 files changed, 15 insertions(+)


diff --git a/libfrog/scrub.c b/libfrog/scrub.c
index 12c0926f..e9671da2 100644
--- a/libfrog/scrub.c
+++ b/libfrog/scrub.c
@@ -129,6 +129,12 @@ const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR] = {
 		.descr	= "project quotas",
 		.type	= XFROG_SCRUB_TYPE_FS,
 	},
+	[XFS_SCRUB_TYPE_FSCOUNTERS] = {
+		.name	= "fscounters",
+		.descr	= "filesystem summary counters",
+		.type	= XFROG_SCRUB_TYPE_FS,
+		.flags	= XFROG_SCRUB_DESCR_SUMMARY,
+	},
 };
 
 int
diff --git a/libfrog/scrub.h b/libfrog/scrub.h
index 6fda8975..e43d8c24 100644
--- a/libfrog/scrub.h
+++ b/libfrog/scrub.h
@@ -20,8 +20,15 @@ struct xfrog_scrub_descr {
 	const char		*name;
 	const char		*descr;
 	enum xfrog_scrub_type	type;
+	unsigned int		flags;
 };
 
+/*
+ * The type of metadata checked by this scrubber is a summary of other types
+ * of metadata.  This scrubber should be run after all the others.
+ */
+#define XFROG_SCRUB_DESCR_SUMMARY	(1 << 0)
+
 extern const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR];
 
 int xfrog_scrub_metadata(struct xfs_fd *xfd, struct xfs_scrub_metadata *meta);
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 153d29d5..083ed9a1 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -293,6 +293,8 @@ xfs_scrub_metadata(
 	for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
 		if (sc->type != scrub_type)
 			continue;
+		if (sc->flags & XFROG_SCRUB_DESCR_SUMMARY)
+			continue;
 
 		meta.sm_type = type;
 		meta.sm_flags = 0;