[PATCHBOMB] xfsprogs: pending changes for 6.6

[PATCHBOMB] xfsprogs: pending changes for 6.6

[Darrick J. Wong]

Hi,

Here are all the pending bugfixes and new code that I have staged for
6.6.  Can we get the unreviewed ones through review, and the reviewed
ones merged into for-next, please?

Unreviewed patches:

patchset 1: 3, 4, and 5
patchset 2: 6
patchset 3: 1-4

Fully rewiewed:
patchset 4:

--D

[PATCHSET 1/4] xfsprogs: various bug fixes for 6.6

[Darrick J. Wong]

Hi all,

This series fixes a couple of bugs that I found in the userspace support
libraries.

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.

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-fixes-6.6
---
 copy/xfs_copy.c      |   24 +++++++++----
 db/block.c           |   14 +++++++
 db/io.c              |   35 +++++++++++++++++-
 db/io.h              |    3 ++
 include/libxfs.h     |    1 +
 libfrog/Makefile     |    1 +
 libfrog/div64.h      |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 libxfs/defer_item.c  |    7 ++++
 libxfs/libxfs_priv.h |   77 +---------------------------------------
 9 files changed, 171 insertions(+), 87 deletions(-)
 create mode 100644 libfrog/div64.h

[PATCH 1/5] libfrog: move 64-bit division wrappers to libfrog

[Darrick J. Wong]

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

We want to keep the rtgroup unit conversion functions as static inlines,
so share the div64 functions via libfrog instead of libxfs_priv.h.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 include/libxfs.h     |    1 +
 libfrog/Makefile     |    1 +
 libfrog/div64.h      |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 libxfs/libxfs_priv.h |   77 +---------------------------------------
 4 files changed, 99 insertions(+), 76 deletions(-)
 create mode 100644 libfrog/div64.h


diff --git a/include/libxfs.h b/include/libxfs.h
index b28781d1..a6a5f66f 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -18,6 +18,7 @@
 #include "kmem.h"
 #include "libfrog/radix-tree.h"
 #include "libfrog/bitmask.h"
+#include "libfrog/div64.h"
 #include "atomic.h"
 #include "spinlock.h"
 
diff --git a/libfrog/Makefile b/libfrog/Makefile
index 8cde97d4..dcfd1fb8 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -41,6 +41,7 @@ crc32cselftest.h \
 crc32defs.h \
 crc32table.h \
 dahashselftest.h \
+div64.h \
 fsgeom.h \
 logging.h \
 paths.h \
diff --git a/libfrog/div64.h b/libfrog/div64.h
new file mode 100644
index 00000000..673b01cb
--- /dev/null
+++ b/libfrog/div64.h
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#ifndef LIBFROG_DIV64_H_
+#define LIBFROG_DIV64_H_
+
+static inline int __do_div(unsigned long long *n, unsigned base)
+{
+	int __res;
+	__res = (int)(((unsigned long) *n) % (unsigned) base);
+	*n = ((unsigned long) *n) / (unsigned) base;
+	return __res;
+}
+
+#define do_div(n,base)	(__do_div((unsigned long long *)&(n), (base)))
+#define do_mod(a, b)		((a) % (b))
+#define rol32(x,y)		(((x) << (y)) | ((x) >> (32 - (y))))
+
+/**
+ * div_u64_rem - unsigned 64bit divide with 32bit divisor with remainder
+ * @dividend: unsigned 64bit dividend
+ * @divisor: unsigned 32bit divisor
+ * @remainder: pointer to unsigned 32bit remainder
+ *
+ * Return: sets ``*remainder``, then returns dividend / divisor
+ *
+ * This is commonly provided by 32bit archs to provide an optimized 64bit
+ * divide.
+ */
+static inline uint64_t
+div_u64_rem(uint64_t dividend, uint32_t divisor, uint32_t *remainder)
+{
+	*remainder = dividend % divisor;
+	return dividend / divisor;
+}
+
+/**
+ * div_u64 - unsigned 64bit divide with 32bit divisor
+ * @dividend: unsigned 64bit dividend
+ * @divisor: unsigned 32bit divisor
+ *
+ * This is the most common 64bit divide and should be used if possible,
+ * as many 32bit archs can optimize this variant better than a full 64bit
+ * divide.
+ */
+static inline uint64_t div_u64(uint64_t dividend, uint32_t divisor)
+{
+	uint32_t remainder;
+	return div_u64_rem(dividend, divisor, &remainder);
+}
+
+/**
+ * div64_u64_rem - unsigned 64bit divide with 64bit divisor and remainder
+ * @dividend: unsigned 64bit dividend
+ * @divisor: unsigned 64bit divisor
+ * @remainder: pointer to unsigned 64bit remainder
+ *
+ * Return: sets ``*remainder``, then returns dividend / divisor
+ */
+static inline uint64_t
+div64_u64_rem(uint64_t dividend, uint64_t divisor, uint64_t *remainder)
+{
+	*remainder = dividend % divisor;
+	return dividend / divisor;
+}
+
+static inline uint64_t rounddown_64(uint64_t x, uint32_t y)
+{
+	do_div(x, y);
+	return x * y;
+}
+
+static inline bool isaligned_64(uint64_t x, uint32_t y)
+{
+	return do_div(x, y) == 0;
+}
+
+static inline uint64_t
+roundup_64(uint64_t x, uint32_t y)
+{
+	x += y - 1;
+	do_div(x, y);
+	return x * y;
+}
+
+static inline uint64_t
+howmany_64(uint64_t x, uint32_t y)
+{
+	x += y - 1;
+	do_div(x, y);
+	return x;
+}
+
+#endif /* LIBFROG_DIV64_H_ */
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 2729241b..5a7decf9 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -48,6 +48,7 @@
 #include "kmem.h"
 #include "libfrog/radix-tree.h"
 #include "libfrog/bitmask.h"
+#include "libfrog/div64.h"
 #include "atomic.h"
 #include "spinlock.h"
 #include "linux-err.h"
@@ -215,66 +216,6 @@ static inline bool WARN_ON(bool expr) {
 	(inode)->i_version = (version);	\
 } while (0)
 
-static inline int __do_div(unsigned long long *n, unsigned base)
-{
-	int __res;
-	__res = (int)(((unsigned long) *n) % (unsigned) base);
-	*n = ((unsigned long) *n) / (unsigned) base;
-	return __res;
-}
-
-#define do_div(n,base)	(__do_div((unsigned long long *)&(n), (base)))
-#define do_mod(a, b)		((a) % (b))
-#define rol32(x,y)		(((x) << (y)) | ((x) >> (32 - (y))))
-
-/**
- * div_u64_rem - unsigned 64bit divide with 32bit divisor with remainder
- * @dividend: unsigned 64bit dividend
- * @divisor: unsigned 32bit divisor
- * @remainder: pointer to unsigned 32bit remainder
- *
- * Return: sets ``*remainder``, then returns dividend / divisor
- *
- * This is commonly provided by 32bit archs to provide an optimized 64bit
- * divide.
- */
-static inline uint64_t
-div_u64_rem(uint64_t dividend, uint32_t divisor, uint32_t *remainder)
-{
-	*remainder = dividend % divisor;
-	return dividend / divisor;
-}
-
-/**
- * div_u64 - unsigned 64bit divide with 32bit divisor
- * @dividend: unsigned 64bit dividend
- * @divisor: unsigned 32bit divisor
- *
- * This is the most common 64bit divide and should be used if possible,
- * as many 32bit archs can optimize this variant better than a full 64bit
- * divide.
- */
-static inline uint64_t div_u64(uint64_t dividend, uint32_t divisor)
-{
-	uint32_t remainder;
-	return div_u64_rem(dividend, divisor, &remainder);
-}
-
-/**
- * div64_u64_rem - unsigned 64bit divide with 64bit divisor and remainder
- * @dividend: unsigned 64bit dividend
- * @divisor: unsigned 64bit divisor
- * @remainder: pointer to unsigned 64bit remainder
- *
- * Return: sets ``*remainder``, then returns dividend / divisor
- */
-static inline uint64_t
-div64_u64_rem(uint64_t dividend, uint64_t divisor, uint64_t *remainder)
-{
-	*remainder = dividend % divisor;
-	return dividend / divisor;
-}
-
 #define min_t(type,x,y) \
 	({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
 #define max_t(type,x,y) \
@@ -380,22 +321,6 @@ roundup_pow_of_two(uint v)
 	return 0;
 }
 
-static inline uint64_t
-roundup_64(uint64_t x, uint32_t y)
-{
-	x += y - 1;
-	do_div(x, y);
-	return x * y;
-}
-
-static inline uint64_t
-howmany_64(uint64_t x, uint32_t y)
-{
-	x += y - 1;
-	do_div(x, y);
-	return x;
-}
-
 /* buffer management */
 #define XBF_TRYLOCK			0
 #define XBF_UNMAPPED			0

[PATCH 2/5] libxfs: don't UAF a requeued EFI

[Darrick J. Wong]

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

In the kernel, commit 8ebbf262d4684 ("xfs: don't block in busy flushing
when freeing extents") changed the allocator behavior such that AGFL
fixing can return -EAGAIN in response to detection of a deadlock with
the transaction busy extent list.  If this happens, we're supposed to
requeue the EFI so that we can roll the transaction and try the item
again.

If a requeue happens, we should not free the xefi pointer in
xfs_extent_free_finish_item or else the retry will walk off a dangling
pointer.  There is no extent busy list in userspace so this should
never happen, but let's fix the logic bomb anyway.

We should have ported kernel commit 0853b5de42b47 ("xfs: allow extent
free intents to be retried") to userspace, but neither Carlos nor I
noticed this fine detail. :(

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 libxfs/defer_item.c |    7 +++++++
 1 file changed, 7 insertions(+)


diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index 3f519252..8731d183 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -115,6 +115,13 @@ xfs_extent_free_finish_item(
 	error = xfs_free_extent(tp, xefi->xefi_pag, agbno,
 			xefi->xefi_blockcount, &oinfo, XFS_AG_RESV_NONE);
 
+	/*
+	 * Don't free the XEFI if we need a new transaction to complete
+	 * processing of it.
+	 */
+	if (error == -EAGAIN)
+		return error;
+
 	xfs_extent_free_put_group(xefi);
 	kmem_cache_free(xfs_extfree_item_cache, xefi);
 	return error;

[PATCH 3/5] xfs_copy: distinguish short writes to EOD from runtime errors

[Darrick J. Wong]

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

Detect short writes to the end of the destination device and report
them.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 copy/xfs_copy.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)


diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index 79f65946..d9a14a95 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -889,18 +889,28 @@ main(int argc, char **argv)
 		} else  {
 			char	*lb[XFS_MAX_SECTORSIZE] = { NULL };
 			off64_t	off;
+			ssize_t	len;
 
 			/* ensure device files are sufficiently large */
 
 			off = mp->m_sb.sb_dblocks * source_blocksize;
 			off -= sizeof(lb);
-			if (pwrite(target[i].fd, lb, sizeof(lb), off) < 0)  {
+			len = pwrite(target[i].fd, lb, XFS_MAX_SECTORSIZE, off);
+			if (len < 0) {
 				do_log(_("%s:  failed to write last block\n"),
 					progname);
 				do_log(_("\tIs target \"%s\" too small?\n"),
 					target[i].name);
 				die_perror();
 			}
+			if (len != XFS_MAX_SECTORSIZE) {
+				do_log(
+ _("%s:  short write to last block: %zd bytes, %zu expected\n"),
+					progname, len, XFS_MAX_SECTORSIZE);
+				do_log(_("\tIs target \"%s\" too small?\n"),
+					target[i].name);
+				exit(1);
+			}
 		}
 	}
 

Re: [PATCH 3/5] xfs_copy: distinguish short writes to EOD from runtime errors

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/5] xfs_copy: actually do directio writes to block devices

[Darrick J. Wong]

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

Not sure why block device targets don't get O_DIRECT in !buffered mode,
but it's misleading when the copy completes instantly only to stall
forever due to fsync-on-close.  Adjust the "write last sector" code to
allocate a properly aligned buffer.

In removing the onstack buffer for EOD writes, this also corrects the
buffer being larger than necessary -- the old code declared an array of
32768 pointers, whereas all we really need is an aligned 32768-byte
buffer.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 copy/xfs_copy.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)


diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index d9a14a95..bcc807ed 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -832,13 +832,9 @@ main(int argc, char **argv)
 			do_out(_("Creating file %s\n"), target[i].name);
 
 			open_flags |= O_CREAT;
-			if (!buffered_output)
-				open_flags |= O_DIRECT;
 			write_last_block = 1;
 		} else if (S_ISREG(statbuf.st_mode))  {
 			open_flags |= O_TRUNC;
-			if (!buffered_output)
-				open_flags |= O_DIRECT;
 			write_last_block = 1;
 		} else  {
 			/*
@@ -855,6 +851,8 @@ main(int argc, char **argv)
 				exit(1);
 			}
 		}
+		if (!buffered_output)
+			open_flags |= O_DIRECT;
 
 		target[i].fd = open(target[i].name, open_flags, 0644);
 		if (target[i].fd < 0)  {
@@ -887,14 +885,15 @@ main(int argc, char **argv)
 				}
 			}
 		} else  {
-			char	*lb[XFS_MAX_SECTORSIZE] = { NULL };
+			char	*lb = memalign(wbuf_align, XFS_MAX_SECTORSIZE);
 			off64_t	off;
 			ssize_t	len;
 
 			/* ensure device files are sufficiently large */
+			memset(lb, 0, XFS_MAX_SECTORSIZE);
 
 			off = mp->m_sb.sb_dblocks * source_blocksize;
-			off -= sizeof(lb);
+			off -= XFS_MAX_SECTORSIZE;
 			len = pwrite(target[i].fd, lb, XFS_MAX_SECTORSIZE, off);
 			if (len < 0) {
 				do_log(_("%s:  failed to write last block\n"),
@@ -911,6 +910,7 @@ main(int argc, char **argv)
 					target[i].name);
 				exit(1);
 			}
+			free(lb);
 		}
 	}
 

Re: [PATCH 4/5] xfs_copy: actually do directio writes to block devices

[Christoph Hellwig]

On Wed, Dec 20, 2023 at 09:12:28AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Not sure why block device targets don't get O_DIRECT in !buffered mode,
> but it's misleading when the copy completes instantly only to stall
> forever due to fsync-on-close.  Adjust the "write last sector" code to
> allocate a properly aligned buffer.
> 
> In removing the onstack buffer for EOD writes, this also corrects the
> buffer being larger than necessary -- the old code declared an array of
> 32768 pointers, whereas all we really need is an aligned 32768-byte
> buffer.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 5/5] xfs_db: report the device associated with each io cursor

[Darrick J. Wong]

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

When db is reporting on an io cursor, have it print out the device
that the cursor is pointing to.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 db/block.c |   14 +++++++++++++-
 db/io.c    |   35 ++++++++++++++++++++++++++++++++---
 db/io.h    |    3 +++
 3 files changed, 48 insertions(+), 4 deletions(-)


diff --git a/db/block.c b/db/block.c
index 788337d3..d730c779 100644
--- a/db/block.c
+++ b/db/block.c
@@ -126,7 +126,15 @@ daddr_f(
 	char		*p;
 
 	if (argc == 1) {
-		dbprintf(_("current daddr is %lld\n"), iocur_top->off >> BBSHIFT);
+		xfs_daddr_t	daddr = iocur_top->off >> BBSHIFT;
+
+		if (iocur_is_ddev(iocur_top))
+			dbprintf(_("datadev daddr is %lld\n"), daddr);
+		else if (iocur_is_extlogdev(iocur_top))
+			dbprintf(_("logdev daddr is %lld\n"), daddr);
+		else
+			dbprintf(_("current daddr is %lld\n"), daddr);
+
 		return 0;
 	}
 	d = (int64_t)strtoull(argv[1], &p, 0);
@@ -220,6 +228,10 @@ fsblock_f(
 	char		*p;
 
 	if (argc == 1) {
+		if (!iocur_is_ddev(iocur_top)) {
+			dbprintf(_("cursor does not point to data device\n"));
+			return 0;
+		}
 		dbprintf(_("current fsblock is %lld\n"),
 			XFS_DADDR_TO_FSB(mp, iocur_top->off >> BBSHIFT));
 		return 0;
diff --git a/db/io.c b/db/io.c
index 5ccfe3b5..590dd1f8 100644
--- a/db/io.c
+++ b/db/io.c
@@ -137,18 +137,47 @@ pop_help(void)
 		));
 }
 
+bool
+iocur_is_ddev(const struct iocur *ioc)
+{
+	if (!ioc->bp)
+		return false;
+
+	return ioc->bp->b_target == ioc->bp->b_mount->m_ddev_targp;
+}
+
+bool
+iocur_is_extlogdev(const struct iocur *ioc)
+{
+	struct xfs_buf	*bp = ioc->bp;
+
+	if (!bp)
+		return false;
+	if (bp->b_mount->m_logdev_targp == bp->b_mount->m_ddev_targp)
+		return false;
+
+	return bp->b_target == bp->b_mount->m_logdev_targp;
+}
+
 void
 print_iocur(
 	char	*tag,
 	iocur_t	*ioc)
 {
+	const char	*block_unit = "fsbno?";
 	int	i;
 
+	if (iocur_is_ddev(ioc))
+		block_unit = "fsbno";
+	else if (iocur_is_extlogdev(ioc))
+		block_unit = "logbno";
+
 	dbprintf("%s\n", tag);
 	dbprintf(_("\tbyte offset %lld, length %d\n"), ioc->off, ioc->len);
-	dbprintf(_("\tbuffer block %lld (fsbno %lld), %d bb%s\n"), ioc->bb,
-		(xfs_fsblock_t)XFS_DADDR_TO_FSB(mp, ioc->bb), ioc->blen,
-		ioc->blen == 1 ? "" : "s");
+	dbprintf(_("\tbuffer block %lld (%s %lld), %d bb%s\n"), ioc->bb,
+			block_unit,
+			(xfs_fsblock_t)XFS_DADDR_TO_FSB(mp, ioc->bb),
+			ioc->blen, ioc->blen == 1 ? "" : "s");
 	if (ioc->bbmap) {
 		dbprintf(_("\tblock map"));
 		for (i = 0; i < ioc->bbmap->nmaps; i++)
diff --git a/db/io.h b/db/io.h
index bd86c31f..f48b67b4 100644
--- a/db/io.h
+++ b/db/io.h
@@ -56,6 +56,9 @@ extern void	set_iocur_type(const struct typ *type);
 extern void	xfs_dummy_verify(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
 
+bool iocur_is_ddev(const struct iocur *ioc);
+bool iocur_is_extlogdev(const struct iocur *ioc);
+
 /*
  * returns -1 for unchecked, 0 for bad and 1 for good
  */

Re: [PATCH 5/5] xfs_db: report the device associated with each io cursor

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCHSET 2/4] xfs_metadump: various bug fixes

[Darrick J. Wong]

Hi all,

This series fixes numerous errors I found in the new metadump v2 format
handling code.

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.

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=metadump-fixes-6.6
---
 man/man8/xfs_mdrestore.8  |    6 ++
 man/man8/xfs_metadump.8   |    7 ++-
 mdrestore/xfs_mdrestore.c |  122 ++++++++++++++++++++++++++-------------------
 3 files changed, 81 insertions(+), 54 deletions(-)

[PATCH 1/6] xfs_metadump.8: update for external log device options

[Darrick J. Wong]

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

Update the documentation to reflect that we can metadump external log
device contents.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 man/man8/xfs_mdrestore.8 |    6 +++++-
 man/man8/xfs_metadump.8  |    7 +++++--
 2 files changed, 10 insertions(+), 3 deletions(-)


diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
index 6e7457c0..f60e7b56 100644
--- a/man/man8/xfs_mdrestore.8
+++ b/man/man8/xfs_mdrestore.8
@@ -14,6 +14,10 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
 .br
 .B xfs_mdrestore
 .B \-i
+[
+.B \-l
+.I logdev
+]
 .I source
 .br
 .B xfs_mdrestore \-V
@@ -52,7 +56,7 @@ Shows metadump information on stdout.  If no
 is specified, exits after displaying information.  Older metadumps man not
 include any descriptive information.
 .TP
-.B \-l " logdev"
+.BI \-l " logdev"
 Metadump in v2 format can contain metadata dumped from an external log.
 In such a scenario, the user has to provide a device to which the log device
 contents from the metadump file are copied.
diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8
index 1732012c..496b5926 100644
--- a/man/man8/xfs_metadump.8
+++ b/man/man8/xfs_metadump.8
@@ -132,8 +132,11 @@ is stdout.
 .TP
 .BI \-l " logdev"
 For filesystems which use an external log, this specifies the device where the
-external log resides. The external log is not copied, only internal logs are
-copied.
+external log resides.
+If the v2 metadump format is selected, the contents of the external log will be
+copied to the metadump.
+The v2 metadump format will be selected automatically if this option is
+specified.
 .TP
 .B \-m
 Set the maximum size of an allowed metadata extent.  Extremely large metadata

[PATCH 2/6] xfs_mdrestore: fix uninitialized variables in mdrestore main

[Darrick J. Wong]

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

Coverity complained about the "is fd a file?" flags being uninitialized.
Clean this up.

Coverity-id: 1554270
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 mdrestore/xfs_mdrestore.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)


diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 2de177c6..5dfc4234 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -472,11 +472,11 @@ main(
 	union mdrestore_headers	headers;
 	FILE			*src_f;
 	char			*logdev = NULL;
-	int			data_dev_fd;
-	int			log_dev_fd;
+	int			data_dev_fd = -1;
+	int			log_dev_fd = -1;
 	int			c;
-	bool			is_data_dev_file;
-	bool			is_log_dev_file;
+	bool			is_data_dev_file = false;
+	bool			is_log_dev_file = false;
 
 	mdrestore.show_progress = false;
 	mdrestore.show_info = false;
@@ -561,7 +561,6 @@ main(
 	/* check and open data device */
 	data_dev_fd = open_device(argv[optind], &is_data_dev_file);
 
-	log_dev_fd = -1;
 	if (mdrestore.external_log)
 		/* check and open log device */
 		log_dev_fd = open_device(logdev, &is_log_dev_file);

[PATCH 3/6] xfs_mdrestore: emit newlines for fatal errors

[Darrick J. Wong]

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

Spit out a newline after a fatal error message.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 mdrestore/xfs_mdrestore.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 5dfc4234..3190e07e 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -275,10 +275,10 @@ read_header_v2(
 		fatal("error reading from metadump file\n");
 
 	if (h->v2.xmh_incompat_flags != 0)
-		fatal("Metadump header has unknown incompat flags set");
+		fatal("Metadump header has unknown incompat flags set\n");
 
 	if (h->v2.xmh_reserved != 0)
-		fatal("Metadump header's reserved field has a non-zero value");
+		fatal("Metadump header's reserved field has a non-zero value\n");
 
 	want_external_log = !!(be32_to_cpu(h->v2.xmh_incompat_flags) &
 			XFS_MD2_COMPAT_EXTERNALLOG);

[PATCH 4/6] xfs_mdrestore: EXTERNALLOG is a compat value, not incompat

[Darrick J. Wong]

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

Fix this check to look at the correct header field.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 mdrestore/xfs_mdrestore.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)


diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 3190e07e..1d65765d 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -268,7 +268,7 @@ read_header_v2(
 	union mdrestore_headers		*h,
 	FILE				*md_fp)
 {
-	bool				want_external_log;
+	unsigned int			compat;
 
 	if (fread((uint8_t *)&(h->v2) + sizeof(h->v2.xmh_magic),
 			sizeof(h->v2) - sizeof(h->v2.xmh_magic), 1, md_fp) != 1)
@@ -280,10 +280,9 @@ read_header_v2(
 	if (h->v2.xmh_reserved != 0)
 		fatal("Metadump header's reserved field has a non-zero value\n");
 
-	want_external_log = !!(be32_to_cpu(h->v2.xmh_incompat_flags) &
-			XFS_MD2_COMPAT_EXTERNALLOG);
+	compat = be32_to_cpu(h->v2.xmh_compat_flags);
 
-	if (want_external_log && !mdrestore.external_log)
+	if (!mdrestore.external_log && (compat & XFS_MD2_COMPAT_EXTERNALLOG))
 		fatal("External Log device is required\n");
 }
 

[PATCH 5/6] xfs_mdrestore: fix missed progress reporting

[Darrick J. Wong]

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

Currently, the progress reporting only triggers when the number of bytes
read is exactly a multiple of a megabyte.  This isn't always guaranteed,
since AG headers can be 512 bytes in size.  Fix the algorithm by
recording the number of megabytes we've reported as being read, and emit
a new report any time the bytes_read count, once converted to megabytes,
doesn't match.

Fix the v2 code to emit one final status message in case the last
extent restored is more than a megabyte.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>
---
 mdrestore/xfs_mdrestore.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)


diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 1d65765d..685ca4c1 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -7,6 +7,7 @@
 #include "libxfs.h"
 #include "xfs_metadump.h"
 #include <libfrog/platform.h>
+#include "libfrog/div64.h"
 
 union mdrestore_headers {
 	__be32				magic;
@@ -160,6 +161,7 @@ restore_v1(
 	int			mb_count;
 	xfs_sb_t		sb;
 	int64_t			bytes_read;
+	int64_t			mb_read = 0;
 
 	block_size = 1 << h->v1.mb_blocklog;
 	max_indices = (block_size - sizeof(xfs_metablock_t)) / sizeof(__be64);
@@ -208,9 +210,14 @@ restore_v1(
 	bytes_read = 0;
 
 	for (;;) {
-		if (mdrestore.show_progress &&
-		    (bytes_read & ((1 << 20) - 1)) == 0)
-			print_progress("%lld MB read", bytes_read >> 20);
+		if (mdrestore.show_progress) {
+			int64_t		mb_now = bytes_read >> 20;
+
+			if (mb_now != mb_read) {
+				print_progress("%lld MB read", mb_now);
+				mb_read = mb_now;
+			}
+		}
 
 		for (cur_index = 0; cur_index < mb_count; cur_index++) {
 			if (pwrite(ddev_fd, &block_buffer[cur_index <<
@@ -240,6 +247,9 @@ restore_v1(
 		bytes_read += block_size + (mb_count << h->v1.mb_blocklog);
 	}
 
+	if (mdrestore.show_progress && bytes_read > (mb_read << 20))
+		print_progress("%lld MB read", howmany_64(bytes_read, 1U << 20));
+
 	if (mdrestore.progress_since_warning)
 		putchar('\n');
 
@@ -343,6 +353,7 @@ restore_v2(
 	struct xfs_sb		sb;
 	struct xfs_meta_extent	xme;
 	char			*block_buffer;
+	int64_t			mb_read = 0;
 	int64_t			bytes_read;
 	uint64_t		offset;
 	int			len;
@@ -419,16 +430,18 @@ restore_v2(
 		bytes_read += len;
 
 		if (mdrestore.show_progress) {
-			static int64_t mb_read;
-			int64_t mb_now = bytes_read >> 20;
+			int64_t	mb_now = bytes_read >> 20;
 
 			if (mb_now != mb_read) {
-				print_progress("%lld MB read", mb_now);
+				print_progress("%lld mb read", mb_now);
 				mb_read = mb_now;
 			}
 		}
 	} while (1);
 
+	if (mdrestore.show_progress && bytes_read > (mb_read << 20))
+		print_progress("%lld mb read", howmany_64(bytes_read, 1U << 20));
+
 	if (mdrestore.progress_since_warning)
 		putchar('\n');
 

[PATCH 6/6] xfs_mdrestore: refactor progress printing and sb fixup code

[Darrick J. Wong]

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

Now that we've fixed the dissimilarities between the two progress
printing callsites, refactor them into helpers.  Do the same for the
duplicate code that clears sb_inprogress from the primary superblock
after the copy succeeds.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 mdrestore/xfs_mdrestore.c |  109 ++++++++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 50 deletions(-)


diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 685ca4c1..8e3998db 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -58,6 +58,58 @@ print_progress(const char *fmt, ...)
 	mdrestore.progress_since_warning = true;
 }
 
+static inline void
+maybe_print_progress(
+	int64_t		*cursor,
+	int64_t		bytes_read)
+{
+	int64_t		mb_now = bytes_read >> 20;
+
+	if (!mdrestore.show_progress)
+		return;
+
+	if (mb_now != *cursor) {
+		print_progress("%lld MB read", mb_now);
+		*cursor = mb_now;
+	}
+}
+
+static inline void
+final_print_progress(
+	int64_t		*cursor,
+	int64_t		bytes_read)
+{
+	if (!mdrestore.show_progress)
+		goto done;
+
+	if (bytes_read <= (*cursor << 20))
+		goto done;
+
+	print_progress("%lld MB read", howmany_64(bytes_read, 1U << 20));
+
+done:
+	if (mdrestore.progress_since_warning)
+		putchar('\n');
+}
+
+static void
+fixup_superblock(
+	int		ddev_fd,
+	char		*block_buffer,
+	struct xfs_sb	*sb)
+{
+	memset(block_buffer, 0, sb->sb_sectsize);
+	sb->sb_inprogress = 0;
+	libxfs_sb_to_disk((struct xfs_dsb *)block_buffer, sb);
+	if (xfs_sb_version_hascrc(sb)) {
+		xfs_update_cksum(block_buffer, sb->sb_sectsize,
+				 offsetof(struct xfs_sb, sb_crc));
+	}
+
+	if (pwrite(ddev_fd, block_buffer, sb->sb_sectsize, 0) < 0)
+		fatal("error writing primary superblock: %s\n", strerror(errno));
+}
+
 static int
 open_device(
 	char		*path,
@@ -210,14 +262,7 @@ restore_v1(
 	bytes_read = 0;
 
 	for (;;) {
-		if (mdrestore.show_progress) {
-			int64_t		mb_now = bytes_read >> 20;
-
-			if (mb_now != mb_read) {
-				print_progress("%lld MB read", mb_now);
-				mb_read = mb_now;
-			}
-		}
+		maybe_print_progress(&mb_read, bytes_read);
 
 		for (cur_index = 0; cur_index < mb_count; cur_index++) {
 			if (pwrite(ddev_fd, &block_buffer[cur_index <<
@@ -247,22 +292,9 @@ restore_v1(
 		bytes_read += block_size + (mb_count << h->v1.mb_blocklog);
 	}
 
-	if (mdrestore.show_progress && bytes_read > (mb_read << 20))
-		print_progress("%lld MB read", howmany_64(bytes_read, 1U << 20));
+	final_print_progress(&mb_read, bytes_read);
 
-	if (mdrestore.progress_since_warning)
-		putchar('\n');
-
-	memset(block_buffer, 0, sb.sb_sectsize);
-	sb.sb_inprogress = 0;
-	libxfs_sb_to_disk((struct xfs_dsb *)block_buffer, &sb);
-	if (xfs_sb_version_hascrc(&sb)) {
-		xfs_update_cksum(block_buffer, sb.sb_sectsize,
-				 offsetof(struct xfs_sb, sb_crc));
-	}
-
-	if (pwrite(ddev_fd, block_buffer, sb.sb_sectsize, 0) < 0)
-		fatal("error writing primary superblock: %s\n", strerror(errno));
+	fixup_superblock(ddev_fd, block_buffer, &sb);
 
 	free(metablock);
 }
@@ -401,6 +433,8 @@ restore_v2(
 		char *device;
 		int fd;
 
+		maybe_print_progress(&mb_read, bytes_read);
+
 		if (fread(&xme, sizeof(xme), 1, md_fp) != 1) {
 			if (feof(md_fp))
 				break;
@@ -428,38 +462,13 @@ restore_v2(
 				len);
 
 		bytes_read += len;
-
-		if (mdrestore.show_progress) {
-			int64_t	mb_now = bytes_read >> 20;
-
-			if (mb_now != mb_read) {
-				print_progress("%lld mb read", mb_now);
-				mb_read = mb_now;
-			}
-		}
 	} while (1);
 
-	if (mdrestore.show_progress && bytes_read > (mb_read << 20))
-		print_progress("%lld mb read", howmany_64(bytes_read, 1U << 20));
+	final_print_progress(&mb_read, bytes_read);
 
-	if (mdrestore.progress_since_warning)
-		putchar('\n');
-
-	memset(block_buffer, 0, sb.sb_sectsize);
-	sb.sb_inprogress = 0;
-	libxfs_sb_to_disk((struct xfs_dsb *)block_buffer, &sb);
-	if (xfs_sb_version_hascrc(&sb)) {
-		xfs_update_cksum(block_buffer, sb.sb_sectsize,
-				offsetof(struct xfs_sb, sb_crc));
-	}
-
-	if (pwrite(ddev_fd, block_buffer, sb.sb_sectsize, 0) < 0)
-		fatal("error writing primary superblock: %s\n",
-			strerror(errno));
+	fixup_superblock(ddev_fd, block_buffer, &sb);
 
 	free(block_buffer);
-
-	return;
 }
 
 static struct mdrestore_ops mdrestore_ops_v2 = {

Re: [PATCH 6/6] xfs_mdrestore: refactor progress printing and sb fixup code

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCHSET v28.3 3/4] xfs_io: clean up scrub subcommand code

[Darrick J. Wong]

Hi all,

This series cleans up the code in io/scrub.c so that argument parsing is
more straightforward and there are fewer indirections.

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.

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=io-scrub-cleanups-6.6
---
 io/scrub.c        |  259 ++++++++++++++++++++++++++++-------------------------
 man/man8/xfs_io.8 |    3 +
 2 files changed, 140 insertions(+), 122 deletions(-)

[PATCH 1/4] xfs_io: set exitcode = 1 on parsing errors in scrub/repair command

[Darrick J. Wong]

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

Set exitcode to 1 if there is an error parsing the CLI arguments to the
scrub or repair commands, like we do most other places in xfs_io.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/scrub.c |   24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)


diff --git a/io/scrub.c b/io/scrub.c
index fc22ba49..8b3bdd77 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -103,11 +103,14 @@ parse_args(
 	while ((c = getopt(argc, argv, "")) != EOF) {
 		switch (c) {
 		default:
+			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
 	}
-	if (optind > argc - 1)
+	if (optind > argc - 1) {
+		exitcode = 1;
 		return command_usage(cmdinfo);
+	}
 
 	for (i = 0, d = xfrog_scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
 		if (strcmp(d->name, argv[optind]) == 0) {
@@ -117,6 +120,7 @@ parse_args(
 	}
 	if (type < 0) {
 		printf(_("Unknown type '%s'.\n"), argv[optind]);
+		exitcode = 1;
 		return command_usage(cmdinfo);
 	}
 	optind++;
@@ -132,19 +136,22 @@ parse_args(
 				fprintf(stderr,
 					_("Bad inode number '%s'.\n"),
 					argv[optind]);
-				return 0;
+				exitcode = 1;
+				return command_usage(cmdinfo);
 			}
 			control2 = strtoul(argv[optind + 1], &p, 0);
 			if (*p != '\0') {
 				fprintf(stderr,
 					_("Bad generation number '%s'.\n"),
 					argv[optind + 1]);
-				return 0;
+				exitcode = 1;
+				return command_usage(cmdinfo);
 			}
 		} else {
 			fprintf(stderr,
 				_("Must specify inode number and generation.\n"));
-			return 0;
+			exitcode = 1;
+			return command_usage(cmdinfo);
 		}
 		break;
 	case XFROG_SCRUB_TYPE_AGHEADER:
@@ -152,13 +159,15 @@ parse_args(
 		if (optind != argc - 1) {
 			fprintf(stderr,
 				_("Must specify one AG number.\n"));
-			return 0;
+			exitcode = 1;
+			return command_usage(cmdinfo);
 		}
 		control = strtoul(argv[optind], &p, 0);
 		if (*p != '\0') {
 			fprintf(stderr,
 				_("Bad AG number '%s'.\n"), argv[optind]);
-			return 0;
+			exitcode = 1;
+			return command_usage(cmdinfo);
 		}
 		break;
 	case XFROG_SCRUB_TYPE_FS:
@@ -166,7 +175,8 @@ parse_args(
 		if (optind != argc) {
 			fprintf(stderr,
 				_("No parameters allowed.\n"));
-			return 0;
+			exitcode = 1;
+			return command_usage(cmdinfo);
 		}
 		break;
 	default:

Re: [PATCH 1/4] xfs_io: set exitcode = 1 on parsing errors in scrub/repair command

[Christoph Hellwig]

On Wed, Dec 20, 2023 at 09:14:33AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Set exitcode to 1 if there is an error parsing the CLI arguments to the
> scrub or repair commands, like we do most other places in xfs_io.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/4] xfs_io: collapse trivial helpers

[Darrick J. Wong]

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

Simply the call chain by having parse_args set the scrub ioctl
parameters in the caller's object.  The parse_args callers can then
invoke the ioctl directly, eliminating one function and one indirect
call.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/scrub.c |  124 +++++++++++++++++++-----------------------------------------
 1 file changed, 40 insertions(+), 84 deletions(-)


diff --git a/io/scrub.c b/io/scrub.c
index 8b3bdd77..238d9240 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -41,57 +41,12 @@ scrub_help(void)
 	printf("\n");
 }
 
-static void
-scrub_ioctl(
-	int				fd,
-	int				type,
-	uint64_t			control,
-	uint32_t			control2)
-{
-	struct xfs_scrub_metadata	meta;
-	const struct xfrog_scrub_descr	*sc;
-	int				error;
-
-	sc = &xfrog_scrubbers[type];
-	memset(&meta, 0, sizeof(meta));
-	meta.sm_type = type;
-	switch (sc->type) {
-	case XFROG_SCRUB_TYPE_AGHEADER:
-	case XFROG_SCRUB_TYPE_PERAG:
-		meta.sm_agno = control;
-		break;
-	case XFROG_SCRUB_TYPE_INODE:
-		meta.sm_ino = control;
-		meta.sm_gen = control2;
-		break;
-	case XFROG_SCRUB_TYPE_NONE:
-	case XFROG_SCRUB_TYPE_FS:
-		/* no control parameters */
-		break;
-	}
-	meta.sm_flags = 0;
-
-	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
-	if (error)
-		perror("scrub");
-	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
-		printf(_("Corruption detected.\n"));
-	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
-		printf(_("Optimization possible.\n"));
-	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
-		printf(_("Cross-referencing failed.\n"));
-	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
-		printf(_("Corruption detected during cross-referencing.\n"));
-	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
-		printf(_("Scan was not complete.\n"));
-}
-
 static int
 parse_args(
 	int				argc,
 	char				**argv,
-	struct cmdinfo			*cmdinfo,
-	void				(*fn)(int, int, uint64_t, uint32_t))
+	const struct cmdinfo		*cmdinfo,
+	struct xfs_scrub_metadata	*meta)
 {
 	char				*p;
 	int				type = -1;
@@ -100,6 +55,7 @@ parse_args(
 	uint32_t			control2 = 0;
 	const struct xfrog_scrub_descr	*d = NULL;
 
+	memset(meta, 0, sizeof(struct xfs_scrub_metadata));
 	while ((c = getopt(argc, argv, "")) != EOF) {
 		switch (c) {
 		default:
@@ -125,6 +81,8 @@ parse_args(
 	}
 	optind++;
 
+	meta->sm_type = type;
+
 	switch (d->type) {
 	case XFROG_SCRUB_TYPE_INODE:
 		if (optind == argc) {
@@ -153,6 +111,8 @@ parse_args(
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
+		meta->sm_ino = control;
+		meta->sm_gen = control2;
 		break;
 	case XFROG_SCRUB_TYPE_AGHEADER:
 	case XFROG_SCRUB_TYPE_PERAG:
@@ -169,6 +129,7 @@ parse_args(
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
+		meta->sm_agno = control;
 		break;
 	case XFROG_SCRUB_TYPE_FS:
 	case XFROG_SCRUB_TYPE_NONE:
@@ -178,13 +139,12 @@ parse_args(
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
+		/* no control parameters */
 		break;
 	default:
 		ASSERT(0);
 		break;
 	}
-	fn(file->fd, type, control, control2);
-
 	return 0;
 }
 
@@ -193,7 +153,27 @@ scrub_f(
 	int				argc,
 	char				**argv)
 {
-	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);
+	struct xfs_scrub_metadata	meta;
+	int				error;
+
+	error = parse_args(argc, argv, &scrub_cmd, &meta);
+	if (error)
+		return error;
+
+	error = ioctl(file->fd, XFS_IOC_SCRUB_METADATA, &meta);
+	if (error)
+		perror("scrub");
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+		printf(_("Corruption detected.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
+		printf(_("Optimization possible.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
+		printf(_("Cross-referencing failed.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
+		printf(_("Corruption detected during cross-referencing.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
+		printf(_("Scan was not complete.\n"));
+	return 0;
 }
 
 void
@@ -236,37 +216,20 @@ repair_help(void)
 	printf("\n");
 }
 
-static void
-repair_ioctl(
-	int				fd,
-	int				type,
-	uint64_t			control,
-	uint32_t			control2)
+static int
+repair_f(
+	int				argc,
+	char				**argv)
 {
 	struct xfs_scrub_metadata	meta;
-	const struct xfrog_scrub_descr	*sc;
 	int				error;
 
-	sc = &xfrog_scrubbers[type];
-	memset(&meta, 0, sizeof(meta));
-	meta.sm_type = type;
-	switch (sc->type) {
-	case XFROG_SCRUB_TYPE_AGHEADER:
-	case XFROG_SCRUB_TYPE_PERAG:
-		meta.sm_agno = control;
-		break;
-	case XFROG_SCRUB_TYPE_INODE:
-		meta.sm_ino = control;
-		meta.sm_gen = control2;
-		break;
-	case XFROG_SCRUB_TYPE_NONE:
-	case XFROG_SCRUB_TYPE_FS:
-		/* no control parameters */
-		break;
-	}
-	meta.sm_flags = XFS_SCRUB_IFLAG_REPAIR;
+	error = parse_args(argc, argv, &repair_cmd, &meta);
+	if (error)
+		return error;
+	meta.sm_flags |= XFS_SCRUB_IFLAG_REPAIR;
 
-	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
+	error = ioctl(file->fd, XFS_IOC_SCRUB_METADATA, &meta);
 	if (error)
 		perror("repair");
 	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
@@ -281,14 +244,7 @@ repair_ioctl(
 		printf(_("Repair was not complete.\n"));
 	if (meta.sm_flags & XFS_SCRUB_OFLAG_NO_REPAIR_NEEDED)
 		printf(_("Metadata did not need repair or optimization.\n"));
-}
-
-static int
-repair_f(
-	int				argc,
-	char				**argv)
-{
-	return parse_args(argc, argv, &repair_cmd, repair_ioctl);
+	return 0;
 }
 
 void

Re: [PATCH 2/4] xfs_io: collapse trivial helpers

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/4] xfs_io: extract contorl number parsing routines

[Darrick J. Wong]

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

Break out the parts of parse_args that extract control numbers from the
CLI arguments, so that the function isn't as long.  This isn't all that
exciting now, but the scrub vectorization speedups will introduce a new
ioctl.  For the new command that comes with that, we'll want the control
number parsing helpers.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/scrub.c |  128 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 85 insertions(+), 43 deletions(-)


diff --git a/io/scrub.c b/io/scrub.c
index 238d9240..cde788fb 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -41,6 +41,87 @@ scrub_help(void)
 	printf("\n");
 }
 
+static bool
+parse_inode(
+	int		argc,
+	char		**argv,
+	int		optind,
+	__u64		*ino,
+	__u32		*gen)
+{
+	char		*p;
+	unsigned long long control;
+	unsigned long	control2;
+
+	if (optind == argc) {
+		*ino = 0;
+		*gen = 0;
+		return true;
+	}
+
+	if (optind != argc - 2) {
+		fprintf(stderr,
+ _("Must specify inode number and generation.\n"));
+		return false;
+	}
+
+	control = strtoull(argv[optind], &p, 0);
+	if (*p != '\0') {
+		fprintf(stderr, _("Bad inode number '%s'.\n"),
+				argv[optind]);
+		return false;
+	}
+	control2 = strtoul(argv[optind + 1], &p, 0);
+	if (*p != '\0') {
+		fprintf(stderr, _("Bad generation number '%s'.\n"),
+				argv[optind + 1]);
+		return false;
+	}
+
+	*ino = control;
+	*gen = control2;
+	return true;
+}
+
+static bool
+parse_agno(
+	int		argc,
+	char		**argv,
+	int		optind,
+	__u32		*agno)
+{
+	char		*p;
+	unsigned long	control;
+
+	if (optind != argc - 1) {
+		fprintf(stderr, _("Must specify one AG number.\n"));
+		return false;
+	}
+
+	control = strtoul(argv[optind], &p, 0);
+	if (*p != '\0') {
+		fprintf(stderr, _("Bad AG number '%s'.\n"), argv[optind]);
+		return false;
+	}
+
+	*agno = control;
+	return true;
+}
+
+static bool
+parse_none(
+	int		argc,
+	int		optind)
+{
+	if (optind != argc) {
+		fprintf(stderr, _("No parameters allowed.\n"));
+		return false;
+	}
+
+	/* no control parameters */
+	return true;
+}
+
 static int
 parse_args(
 	int				argc,
@@ -48,11 +129,8 @@ parse_args(
 	const struct cmdinfo		*cmdinfo,
 	struct xfs_scrub_metadata	*meta)
 {
-	char				*p;
 	int				type = -1;
 	int				i, c;
-	uint64_t			control = 0;
-	uint32_t			control2 = 0;
 	const struct xfrog_scrub_descr	*d = NULL;
 
 	memset(meta, 0, sizeof(struct xfs_scrub_metadata));
@@ -85,61 +163,25 @@ parse_args(
 
 	switch (d->type) {
 	case XFROG_SCRUB_TYPE_INODE:
-		if (optind == argc) {
-			control = 0;
-			control2 = 0;
-		} else if (optind == argc - 2) {
-			control = strtoull(argv[optind], &p, 0);
-			if (*p != '\0') {
-				fprintf(stderr,
-					_("Bad inode number '%s'.\n"),
-					argv[optind]);
-				exitcode = 1;
-				return command_usage(cmdinfo);
-			}
-			control2 = strtoul(argv[optind + 1], &p, 0);
-			if (*p != '\0') {
-				fprintf(stderr,
-					_("Bad generation number '%s'.\n"),
-					argv[optind + 1]);
-				exitcode = 1;
-				return command_usage(cmdinfo);
-			}
-		} else {
-			fprintf(stderr,
-				_("Must specify inode number and generation.\n"));
+		if (!parse_inode(argc, argv, optind, &meta->sm_ino,
+						     &meta->sm_gen)) {
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
-		meta->sm_ino = control;
-		meta->sm_gen = control2;
 		break;
 	case XFROG_SCRUB_TYPE_AGHEADER:
 	case XFROG_SCRUB_TYPE_PERAG:
-		if (optind != argc - 1) {
-			fprintf(stderr,
-				_("Must specify one AG number.\n"));
+		if (!parse_agno(argc, argv, optind, &meta->sm_agno)) {
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
-		control = strtoul(argv[optind], &p, 0);
-		if (*p != '\0') {
-			fprintf(stderr,
-				_("Bad AG number '%s'.\n"), argv[optind]);
-			exitcode = 1;
-			return command_usage(cmdinfo);
-		}
-		meta->sm_agno = control;
 		break;
 	case XFROG_SCRUB_TYPE_FS:
 	case XFROG_SCRUB_TYPE_NONE:
-		if (optind != argc) {
-			fprintf(stderr,
-				_("No parameters allowed.\n"));
+		if (!parse_none(argc, optind)) {
 			exitcode = 1;
 			return command_usage(cmdinfo);
 		}
-		/* no control parameters */
 		break;
 	default:
 		ASSERT(0);

Re: [PATCH 3/4] xfs_io: extract contorl number parsing routines

[Christoph Hellwig]

On Wed, Dec 20, 2023 at 09:15:04AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Break out the parts of parse_args that extract control numbers from the
> CLI arguments, so that the function isn't as long.  This isn't all that
> exciting now, but the scrub vectorization speedups will introduce a new
> ioctl.  For the new command that comes with that, we'll want the control
> number parsing helpers.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/4] xfs_io: extract contorl number parsing routines

[Christoph Hellwig]

On Wed, Dec 20, 2023 at 09:49:28PM -0800, Christoph Hellwig wrote:
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Oh, and s/contorl/control/ in the subject line.

[PATCH 4/4] xfs_io: support passing the FORCE_REBUILD flag to online repair

[Darrick J. Wong]

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>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 io/scrub.c        |   11 +++++++++--
 man/man8/xfs_io.8 |    3 +++
 2 files changed, 12 insertions(+), 2 deletions(-)


diff --git a/io/scrub.c b/io/scrub.c
index cde788fb..403b3a72 100644
--- a/io/scrub.c
+++ b/io/scrub.c
@@ -131,11 +131,15 @@ parse_args(
 {
 	int				type = -1;
 	int				i, c;
+	uint32_t			flags = 0;
 	const struct xfrog_scrub_descr	*d = NULL;
 
 	memset(meta, 0, sizeof(struct xfs_scrub_metadata));
-	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:
 			exitcode = 1;
 			return command_usage(cmdinfo);
@@ -160,6 +164,7 @@ parse_args(
 	optind++;
 
 	meta->sm_type = type;
+	meta->sm_flags = flags;
 
 	switch (d->type) {
 	case XFROG_SCRUB_TYPE_INODE:
@@ -248,11 +253,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");
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index ef7087b3..d46dc369 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

[PATCHSET v28.3 4/4] xfsprogs: force rebuilding of metadata

[Darrick J. Wong]

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.

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.

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=repair-force-rebuild-6.6

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=repair-force-rebuild-6.6
---
 scrub/phase1.c    |   28 ++++++++++++++++++++++++
 scrub/scrub.c     |   61 ++++++++++++++++++++++++++++-------------------------
 scrub/scrub.h     |    1 +
 scrub/vfs.c       |    2 +-
 scrub/xfs_scrub.c |    3 +++
 scrub/xfs_scrub.h |    1 +
 6 files changed, 66 insertions(+), 30 deletions(-)

[PATCH 1/3] xfs_scrub: handle spurious wakeups in scan_fs_tree

[Darrick J. Wong]

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

Coverity reminded me that the pthread_cond_wait can wake up and return
without the predicate variable (sft.nr_dirs > 0) actually changing.
Therefore, one has to retest the condition after each wakeup.

Coverity-id: 1554280
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 scrub/vfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/scrub/vfs.c b/scrub/vfs.c
index 577eb6dc..3c1825a7 100644
--- a/scrub/vfs.c
+++ b/scrub/vfs.c
@@ -263,7 +263,7 @@ scan_fs_tree(
 	 * about to tear everything down.
 	 */
 	pthread_mutex_lock(&sft.lock);
-	if (sft.nr_dirs)
+	while (sft.nr_dirs > 0)
 		pthread_cond_wait(&sft.wakeup, &sft.lock);
 	assert(sft.nr_dirs == 0);
 	pthread_mutex_unlock(&sft.lock);

[PATCH 2/3] xfs_scrub: don't retry unsupported optimizations

[Darrick J. Wong]

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>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 scrub/scrub.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)


diff --git a/scrub/scrub.c b/scrub/scrub.c
index e83d0d9c..1a450687 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? */

[PATCH 3/3] xfs_scrub: try to use XFS_SCRUB_IFLAG_FORCE_REBUILD

[Darrick J. Wong]

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>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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 fd1050c9..2daf5c7b 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 1a450687..1469058b 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 fccd82f2..023069ee 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 7a0411b0..597be59f 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 f6712d36..0d6b9dad 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,