[PATCHSET 2/2] xfs: LLM-inspired bug fixes, part 23

[PATCHSET 2/2] xfs: LLM-inspired bug fixes, part 23

[Darrick J. Wong]

Hi all,

Here's a twenty-third batch of xfs fixes resulting from a LLaMma.  Mwa
mwa mwa...

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
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=llm-fixes-23
---
Commits in this patchset:
 * xfs: don't set a metadata directory's parent to sb_rootino
 * xfs: don't rescan ifree and icount when frozen
 * xfs: rescan the AGs if incore delalloc counter is negative
 * xfs: don't try to repair rtsummary if the summary file isn't computed
 * xfs: update quota flags on secondary supers when metadir enabled
 * xfs: refactor directory data block walking loops in scrub
 * xfs: improve dirent bounds checking in scrub and repair
 * xfs: drop unnecessary sa parameter from the xrep_ag init functions
 * xfs: drop unnecessary sr parameter from xrep_rtgroup init functions
 * xfs: drop unnecessary sa parameter from xchk_ag init functions
 * xfs: drop unnecessary sr parameter from xchk_rtgroup init functions
 * xfs: refactor inode forkoff to byte conversions
---
 fs/xfs/libxfs/xfs_bmap.h         |    6 ++
 fs/xfs/libxfs/xfs_format.h       |    6 ++
 fs/xfs/scrub/common.h            |   46 +++++++---------
 fs/xfs/scrub/repair.h            |   11 ++--
 fs/xfs/scrub/rtsummary.h         |    1 
 fs/xfs/xfs_inode.h               |    2 -
 fs/xfs/libxfs/xfs_attr_leaf.c    |   12 ++--
 fs/xfs/libxfs/xfs_bmap.c         |    4 +
 fs/xfs/libxfs/xfs_inode_buf.c    |    4 +
 fs/xfs/libxfs/xfs_inode_util.c   |    2 -
 fs/xfs/scrub/agheader.c          |   16 +++---
 fs/xfs/scrub/alloc_repair.c      |    4 +
 fs/xfs/scrub/bmap.c              |   13 ++---
 fs/xfs/scrub/bmap_repair.c       |   15 ++---
 fs/xfs/scrub/btree.c             |    4 +
 fs/xfs/scrub/common.c            |   50 ++++++++++-------
 fs/xfs/scrub/cow_repair.c        |   12 ++--
 fs/xfs/scrub/dir.c               |  109 +++++++++++++++++++++++++++++++++++---
 fs/xfs/scrub/dir_repair.c        |  102 +++++++++++++++++++++++++++++-------
 fs/xfs/scrub/fscounters.c        |   21 ++++++-
 fs/xfs/scrub/ialloc_repair.c     |    4 +
 fs/xfs/scrub/inode.c             |    6 +-
 fs/xfs/scrub/inode_repair.c      |   10 ++-
 fs/xfs/scrub/refcount_repair.c   |    4 +
 fs/xfs/scrub/repair.c            |   23 ++++----
 fs/xfs/scrub/rgsuper.c           |    4 +
 fs/xfs/scrub/rmap_repair.c       |    8 +--
 fs/xfs/scrub/rtbitmap.c          |    4 +
 fs/xfs/scrub/rtbitmap_repair.c   |    4 +
 fs/xfs/scrub/rtrefcount.c        |    8 +--
 fs/xfs/scrub/rtrefcount_repair.c |    8 +--
 fs/xfs/scrub/rtrmap.c            |    4 +
 fs/xfs/scrub/rtrmap_repair.c     |   12 ++--
 fs/xfs/scrub/rtsummary.c         |   13 +++--
 fs/xfs/scrub/rtsummary_repair.c  |    2 -
 fs/xfs/scrub/scrub.c             |    6 +-
 fs/xfs/xfs_mount.c               |   17 ++++++
 fs/xfs/xfs_qm.c                  |   25 ++++++++-
 38 files changed, 412 insertions(+), 190 deletions(-)

[PATCH 01/12] xfs: don't set a metadata directory's parent to sb_rootino

[Darrick J. Wong]

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

LOLLM complains that it's weird to set a metadata directory's parent
inumber to the regular directory tree root.  Fix this even if nobody's
ever going to notice this because metadata path lookups don't access
dotdot.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: 4f3d4dd1b04b2b ("xfs: define the on-disk format for the metadir feature")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir_repair.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 4a0b912b657696..bb3f2f462bde56 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1674,7 +1674,10 @@ xrep_dir_swap(
 	ASSERT(sc->ilock_flags & XFS_ILOCK_EXCL);
 	if (rd->pscan.parent_ino == NULLFSINO) {
 		rd->needs_adoption = true;
-		rd->pscan.parent_ino = rd->sc->mp->m_sb.sb_rootino;
+		if (xfs_is_metadir_inode(rd->sc->ip))
+			rd->pscan.parent_ino = rd->sc->mp->m_sb.sb_metadirino;
+		else
+			rd->pscan.parent_ino = rd->sc->mp->m_sb.sb_rootino;
 	}
 
 	/*

[PATCH 02/12] xfs: don't rescan ifree and icount when frozen

[Darrick J. Wong]

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

LOLLM points out that it's pointless to rescan the AGs if we've already
frozen the filesystem because there won't be any more perturbations.
Fix this to mark the scan incomplete and exit.

Cc: <stable@vger.kernel.org> # v5.2
Fixes: 75efa57d0bf5fc ("xfs: add online scrub for superblock counters")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/fscounters.c |    5 +++++
 1 file changed, 5 insertions(+)


diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index eb1a095a0fa9b9..5984fe13a973b8 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -379,6 +379,11 @@ xchk_fscount_aggregate_agcounts(
 	 * to maintain ifree <= icount before giving up.
 	 */
 	if (fsc->ifree > fsc->icount) {
+		if (fsc->frozen) {
+			xchk_set_incomplete(sc);
+			return -EFSCORRUPTED;
+		}
+
 		if (tries--)
 			goto retry;
 		return -EDEADLOCK;

[PATCH 03/12] xfs: rescan the AGs if incore delalloc counter is negative

[Darrick J. Wong]

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

LOLLM complains that percpu_counter_sum can return a negative number but
we instead misinterpret that into a huge unsigned value and do bad
things to fdblocks.  It shouldn't do that, so let's add a retry loop and
an attempt to freeze the filesystem to (hopefully) zero out the incore
delalloc counter.  It probably won't work, but at least failing the
repair beats doing the wrong thing.

Cc: <stable@vger.kernel.org> # v5.2
Fixes: 75efa57d0bf5fc ("xfs: add online scrub for superblock counters")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/fscounters.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index 5984fe13a973b8..c055b73d268058 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -297,7 +297,7 @@ xchk_fscount_aggregate_agcounts(
 {
 	struct xfs_mount	*mp = sc->mp;
 	struct xfs_perag	*pag = NULL;
-	uint64_t		delayed;
+	int64_t			delayed;
 	int			tries = 8;
 	int			error = 0;
 
@@ -359,6 +359,16 @@ xchk_fscount_aggregate_agcounts(
 	 * of the computation.
 	 */
 	delayed = percpu_counter_sum(&mp->m_delalloc_blks);
+	if (delayed < 0) {
+		if (fsc->frozen) {
+			xchk_set_incomplete(sc);
+			return -EFSCORRUPTED;
+		}
+
+		if (tries--)
+			goto retry;
+		return -EDEADLOCK;
+	}
 	fsc->fdblocks -= delayed;
 
 	trace_xchk_fscounters_calc(mp, fsc->icount, fsc->ifree, fsc->fdblocks,

[PATCH 04/12] xfs: don't try to repair rtsummary if the summary file isn't computed

[Darrick J. Wong]

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

LOLLM observed that we can erroneously try to repair the rtsummary file
if we've not yet computed its contents.  That results in a corrupt
summary file, so let's add a flag to track whether or not the incore
data is complete.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: abf039e2e4afde ("xfs: online repair of realtime summaries")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/rtsummary.h        |    1 +
 fs/xfs/scrub/rtsummary.c        |    9 ++++++++-
 fs/xfs/scrub/rtsummary_repair.c |    2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/rtsummary.h b/fs/xfs/scrub/rtsummary.h
index e44b04cb6e2d5e..af1ab64c3f2125 100644
--- a/fs/xfs/scrub/rtsummary.h
+++ b/fs/xfs/scrub/rtsummary.h
@@ -17,6 +17,7 @@ struct xchk_rtsummary {
 	xfs_filblks_t		rsumblocks;
 	unsigned int		rsumlevels;
 	unsigned int		resblks;
+	bool			summary_computed;
 
 	/* suminfo position of xfile as we write buffers to disk. */
 	xfs_rtsumoff_t		prep_wordoff;
diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
index 546b335ade1354..fa358746beaf5c 100644
--- a/fs/xfs/scrub/rtsummary.c
+++ b/fs/xfs/scrub/rtsummary.c
@@ -212,13 +212,20 @@ xchk_rtsum_compute(
 {
 	struct xfs_mount	*mp = sc->mp;
 	struct xfs_rtgroup	*rtg = sc->sr.rtg;
+	struct xchk_rtsummary	*rts = sc->buf;
+	int			error;
 
 	/* If the bitmap size doesn't match the computed size, bail. */
 	if (XFS_FSB_TO_B(mp, xfs_rtbitmap_blockcount(mp)) !=
 	    rtg_bitmap(rtg)->i_disk_size)
 		return -EFSCORRUPTED;
 
-	return xfs_rtalloc_query_all(rtg, sc->tp, xchk_rtsum_record_free, sc);
+	error = xfs_rtalloc_query_all(rtg, sc->tp, xchk_rtsum_record_free, sc);
+	if (error)
+		return error;
+
+	rts->summary_computed = true;
+	return 0;
 }
 
 /* Compare the rtsummary file against the one we computed. */
diff --git a/fs/xfs/scrub/rtsummary_repair.c b/fs/xfs/scrub/rtsummary_repair.c
index ed763290aec1e5..45bdefaeab013e 100644
--- a/fs/xfs/scrub/rtsummary_repair.c
+++ b/fs/xfs/scrub/rtsummary_repair.c
@@ -122,7 +122,7 @@ xrep_rtsummary(
 		return -EOPNOTSUPP;
 
 	/* Walk away if we disagree on the size of the rt bitmap. */
-	if (rts->rbmblocks != mp->m_sb.sb_rbmblocks)
+	if (rts->rbmblocks != mp->m_sb.sb_rbmblocks || !rts->summary_computed)
 		return 0;
 
 	/* Make sure any problems with the fork are fixed. */

[PATCH 05/12] xfs: update quota flags on secondary supers when metadir enabled

[Darrick J. Wong]

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

xfs/085 exposes an interesting bug when metadir is enabled, quotas are
enabled via mount option, and the primary superblock crc fails
validation:

 Phase 1 - find and verify superblock...
 bad primary superblock - bad CRC in superblock !!!

 attempting to find secondary superblock...
 .found candidate secondary superblock...
 verified secondary superblock...
 quota accounting and enforcement flags lost

<snip>

 Phase 3 - for each AG...
         - scan and clear agi unlinked lists...
         - process known inodes and perform inode discovery...
         - agno = 0
         - agno = 1
         - agno = 2
 unexpected regular file inode 16908418 with metadata flag will zap
 cleared inode 16908418
 unexpected regular file inode 16908419 with metadata flag will zap
 cleared inode 16908419
 unexpected regular file inode 16908420 with metadata flag will zap
 cleared inode 16908420
         - agno = 3
         - process newly discovered inodes...

Quota files have been created, but xfs_repair thinks that quotas aren't
enabled and trashes the quota files.  Looking at the filesystem reveals
that the quota accounting flags were indeed persisted to the primary
superblock but not the secondary:

 # xfs_db -c 'sb 0' -c 'print qflags' -c 'sb 1' -c 'print qflags' /dev/sdf
 Metadata corruption detected at 0x561c68436080, inode 0x81 dinode
 xfs_db: Failed to load metadir root directory, error 117
 Metadata corruption detected at 0x561c683f7c80, xfs_agf block 0x1/0x200
 xfs_db: cannot init perag data (117). Continuing anyway.
 Metadata CRC error detected at 0x561c6844d1b5, xfs_sb block 0x0/0x200
 qflags = 0x7cf
 qflags = 0

On a metadir filesystem, quota state is supposed to persist across
mount cycles if the sysadmin doesn't provide any quota mount options.
Therefore, any time we update the quota flags on a metadir filesystem,
we should write the same change to the secondary supers so that
xfs_repair will find the quota data if it ever has to restore from a
secondary.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: d5d9dd5b3026a8 ("xfs: persist quota flags with metadir")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_mount.c |   17 ++++++++++++++++-
 fs/xfs/xfs_qm.c    |   25 ++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index be90c7b0399484..4a2470466ad065 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -446,6 +446,8 @@ int
 xfs_mount_reset_sbqflags(
 	struct xfs_mount	*mp)
 {
+	int			error;
+
 	mp->m_qflags = 0;
 
 	/* It is OK to look at sb_qflags in the mount path without m_sb_lock. */
@@ -458,7 +460,20 @@ xfs_mount_reset_sbqflags(
 	if (!xfs_fs_writable(mp, SB_FREEZE_WRITE))
 		return 0;
 
-	return xfs_sync_sb(mp, false);
+	error = xfs_sync_sb(mp, false);
+	if (error || !xfs_has_metadir(mp))
+		return error;
+
+	/*
+	 * For metadir filesystems, quota flag state persists across mounts, so
+	 * update secondary superblocks in case xfs_repair ever has to recover
+	 * the primary from a secondary.  growfs also updates backup supers so
+	 * lock against that.
+	 */
+	mutex_lock(&mp->m_growlock);
+	error = xfs_update_secondary_sbs(mp);
+	mutex_unlock(&mp->m_growlock);
+	return error;
 }
 
 static const char *const xfs_free_pool_name[] = {
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 008fed8624be2c..2e13c2cde8e2c4 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -718,7 +718,20 @@ xfs_qm_prep_metadir_sb(
 	spin_unlock(&mp->m_sb_lock);
 	xfs_log_sb(tp);
 
-	return xfs_trans_commit(tp);
+	error = xfs_trans_commit(tp);
+	if (error)
+		return error;
+
+	/*
+	 * For metadir filesystems, quota flag state persists across mounts, so
+	 * update secondary superblocks in case xfs_repair ever has to recover
+	 * the primary from a secondary.  growfs also updates backup supers so
+	 * lock against that.
+	 */
+	mutex_lock(&mp->m_growlock);
+	error = xfs_update_secondary_sbs(mp);
+	mutex_unlock(&mp->m_growlock);
+	return error;
 }
 
 /*
@@ -1684,6 +1697,16 @@ xfs_qm_mount_quotas(
 			xfs_alert(mp, "%s: Superblock update failed!",
 				__func__);
 		}
+
+		/*
+		 * For metadir filesystems, quota flag state persists across
+		 * mounts, so update secondary superblocks in case xfs_repair
+		 * ever has to recover the primary from a secondary.  growfs
+		 * also updates backup supers so lock against that.
+		 */
+		mutex_lock(&mp->m_growlock);
+		xfs_update_secondary_sbs(mp);
+		mutex_unlock(&mp->m_growlock);
 	}
 
 	if (error) {

[PATCH 06/12] xfs: refactor directory data block walking loops in scrub

[Darrick J. Wong]

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

Hoist the inner loop code of the directory data block walking code in
the directory scrub and repair code to separate functions in preparation
for strengthening the validation in a subsequent patch.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/dir.c        |   56 ++++++++++++++++++++++++++++++-----
 fs/xfs/scrub/dir_repair.c |   73 +++++++++++++++++++++++++++++++++------------
 2 files changed, 102 insertions(+), 27 deletions(-)


diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index a63eebf39fd496..8de593c5382d11 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -315,6 +315,45 @@ xchk_dir_actor(
 	return error;
 }
 
+/*
+ * Deal with an unused directory entry.  Returns a positive number of bytes to
+ * advance in the directory block, zero to stop processing, or -1 if corruption
+ * was found.
+ */
+static int
+xchk_dir_check_unused(
+	struct xchk_da_btree		*ds,
+	struct xfs_buf			*bp,
+	unsigned int			iter_off,
+	unsigned int			end)
+{
+	struct xfs_dir2_data_unused	*dup = bp->b_addr + iter_off;
+
+	return be16_to_cpu(dup->length);
+}
+
+/*
+ * Check a directory entry.  Returns a positive number of bytes to advance
+ * in the directory block, zero to stop processing, or -1 if corruption was
+ * found.
+ */
+static int
+xchk_dir_check_entry(
+	struct xchk_da_btree		*ds,
+	struct xfs_buf			*bp,
+	unsigned int			iter_off,
+	unsigned int			end,
+	struct xfs_dir2_data_entry	*dent)
+{
+	struct xfs_dir2_data_entry	*dep = bp->b_addr + iter_off;
+	struct xfs_mount		*mp = ds->state->mp;
+
+	if (dep == dent)
+		return 0;
+
+	return xfs_dir2_data_entsize(mp, dep->namelen);
+}
+
 /* Scrub a directory btree record. */
 STATIC int
 xchk_dir_rec(
@@ -391,21 +430,24 @@ xchk_dir_rec(
 		goto out_relse;
 	}
 	for (;;) {
-		struct xfs_dir2_data_entry	*dep = bp->b_addr + iter_off;
 		struct xfs_dir2_data_unused	*dup = bp->b_addr + iter_off;
+		int				advance;
 
 		if (iter_off >= end) {
 			xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
 			goto out_relse;
 		}
 
-		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
-			iter_off += be16_to_cpu(dup->length);
-			continue;
-		}
-		if (dep == dent)
+		if (dup->freetag == cpu_to_be16(XFS_DIR2_DATA_FREE_TAG))
+			advance = xchk_dir_check_unused(ds, bp, iter_off, end);
+		else
+			advance = xchk_dir_check_entry(ds, bp, iter_off, end,
+					dent);
+		if (advance < 0)
+			goto out_relse;
+		if (!advance)
 			break;
-		iter_off += xfs_dir2_data_entsize(mp, dep->namelen);
+		iter_off += advance;
 	}
 
 	/* Retrieve the entry, sanity check it, and compare hashes. */
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index bb3f2f462bde56..1169a2ad26876f 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -462,6 +462,52 @@ xrep_dir_salvage_data_entry(
 	return xrep_dir_salvage_entry(rd, dep->name, dep->namelen, ino);
 }
 
+/*
+ * Deal with an unused directory entry.  Returns a positive number of bytes to
+ * advance in the directory block, or zero to stop processing.
+ */
+static int
+xrep_dir_recover_unused(
+	struct xfs_buf			*bp,
+	unsigned int			offset,
+	unsigned int			end)
+{
+	struct xfs_dir2_data_unused	*dup = bp->b_addr + offset;
+
+	return be16_to_cpu(dup->length);
+}
+
+/*
+ * Salvage a directory entry.  Returns a positive number of bytes to advance
+ * in the directory block, zero to stop processing, or a negative errno.
+ */
+static int
+xrep_dir_recover_entry(
+	struct xrep_dir			*rd,
+	struct xfs_buf			*bp,
+	unsigned int			offset,
+	unsigned int			end)
+{
+	struct xfs_dir2_data_entry	*dep = bp->b_addr + offset;
+	unsigned int			advance;
+	int				error;
+
+	/* Don't walk off the end of the block. */
+	advance = xfs_dir2_data_entsize(rd->sc->mp, dep->namelen);
+	if (!advance)
+		return 0;
+	if (offset + advance > end)
+		return 0;
+
+	/* Ok, let's save this entry. */
+	error = xrep_dir_salvage_data_entry(rd, dep);
+	if (error < 0)
+		return error;
+
+	return advance;
+}
+
+
 /* Try to recover block/data format directory entries. */
 STATIC int
 xrep_dir_recover_data(
@@ -483,33 +529,20 @@ xrep_dir_recover_data(
 
 	while (offset < end) {
 		struct xfs_dir2_data_unused	*dup = bp->b_addr + offset;
-		struct xfs_dir2_data_entry	*dep = bp->b_addr + offset;
-		unsigned int			advance;
+		int				advance;
 
 		if (xchk_should_terminate(rd->sc, &error))
 			return error;
 
-		/* Skip unused entries. */
-		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
-			if (!dup->length)
-				break;
-			offset += be16_to_cpu(dup->length);
-			continue;
-		}
-
-		/* Don't walk off the end of the block. */
-		advance = xfs_dir2_data_entsize(rd->sc->mp, dep->namelen);
+		if (dup->freetag == cpu_to_be16(XFS_DIR2_DATA_FREE_TAG))
+			advance = xrep_dir_recover_unused(bp, offset, end);
+		else
+			advance = xrep_dir_recover_entry(rd, bp, offset, end);
+		if (advance < 0)
+			return advance;
 		if (!advance)
 			break;
 		offset += advance;
-		if (offset > end)
-			break;
-
-		/* Ok, let's save this entry. */
-		error = xrep_dir_salvage_data_entry(rd, dep);
-		if (error)
-			return error;
-
 	}
 
 	return 0;

[PATCH 07/12] xfs: improve dirent bounds checking in scrub and repair

[Darrick J. Wong]

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

LOLLM complains that we don't do enough bounds checking of the directory
entries in directory blocks when we're looking for errors or trying to
salvage entries.  Let's improve that with more detailed checks.

Cc: <stable@vger.kernel.org> # v4.16
Fixes: ce92d29ddf9908 ("xfs: directory scrubber must walk through data block to offset")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir.c        |   67 ++++++++++++++++++++++++++++++++++++++-------
 fs/xfs/scrub/dir_repair.c |   28 ++++++++++++++++++-
 2 files changed, 84 insertions(+), 11 deletions(-)


diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 8de593c5382d11..5fb19e98fca4e5 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -325,11 +325,24 @@ xchk_dir_check_unused(
 	struct xchk_da_btree		*ds,
 	struct xfs_buf			*bp,
 	unsigned int			iter_off,
-	unsigned int			end)
+	unsigned int			end,
+	xfs_dablk_t			rec_bno)
 {
 	struct xfs_dir2_data_unused	*dup = bp->b_addr + iter_off;
+	int				advance;
 
-	return be16_to_cpu(dup->length);
+	if (iter_off + sizeof(*dup) > end) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		return -1;
+	}
+
+	advance = xfs_dir2_data_unusedsize(be16_to_cpu(dup->length));
+	if (!advance) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		return -1;
+	}
+
+	return advance;
 }
 
 /*
@@ -343,15 +356,30 @@ xchk_dir_check_entry(
 	struct xfs_buf			*bp,
 	unsigned int			iter_off,
 	unsigned int			end,
-	struct xfs_dir2_data_entry	*dent)
+	struct xfs_dir2_data_entry	*dent,
+	xfs_dablk_t			rec_bno)
 {
 	struct xfs_dir2_data_entry	*dep = bp->b_addr + iter_off;
 	struct xfs_mount		*mp = ds->state->mp;
+	int				advance;
+
+	/* must have namelen */
+	advance = offsetof(typeof(*dep), name);
+	if (iter_off + advance >= end) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		return -1;
+	}
+
+	advance = xfs_dir2_data_entsize(mp, dep->namelen);
+	if (!advance) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		return -1;
+	}
 
 	if (dep == dent)
 		return 0;
 
-	return xfs_dir2_data_entsize(mp, dep->namelen);
+	return advance;
 }
 
 /* Scrub a directory btree record. */
@@ -379,6 +407,7 @@ xchk_dir_rec(
 	xfs_dahash_t			hash;
 	struct xfs_dir3_icleaf_hdr	hdr;
 	unsigned int			tag;
+	bool				foundit = false;
 	int				error;
 
 	ASSERT(blk->magic == XFS_DIR2_LEAF1_MAGIC ||
@@ -423,33 +452,44 @@ xchk_dir_rec(
 	dent = bp->b_addr + off;
 
 	/* Make sure we got a real directory entry. */
-	iter_off = geo->data_entry_offset;
 	end = xfs_dir3_data_end_offset(geo, bp->b_addr);
 	if (!end) {
 		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
 		goto out_relse;
 	}
-	for (;;) {
+	iter_off = geo->data_entry_offset;
+	while (iter_off < end) {
 		struct xfs_dir2_data_unused	*dup = bp->b_addr + iter_off;
 		int				advance;
 
-		if (iter_off >= end) {
+		/* must have freetag */
+		advance = offsetof(typeof(*dup), length);
+		if (iter_off + advance >= end) {
 			xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
 			goto out_relse;
 		}
 
 		if (dup->freetag == cpu_to_be16(XFS_DIR2_DATA_FREE_TAG))
-			advance = xchk_dir_check_unused(ds, bp, iter_off, end);
+			advance = xchk_dir_check_unused(ds, bp, iter_off, end,
+					rec_bno);
 		else
 			advance = xchk_dir_check_entry(ds, bp, iter_off, end,
-					dent);
+					dent, rec_bno);
 		if (advance < 0)
 			goto out_relse;
-		if (!advance)
+		if (!advance) {
+			foundit = true;
 			break;
+		}
 		iter_off += advance;
 	}
 
+	/* Hash tree must point to the exact dirent */
+	if (!foundit) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		goto out_relse;
+	}
+
 	/* Retrieve the entry, sanity check it, and compare hashes. */
 	ino = be64_to_cpu(dent->inumber);
 	hash = be32_to_cpu(ent->hashval);
@@ -461,6 +501,13 @@ xchk_dir_rec(
 		goto out_relse;
 	}
 
+	/* Name must not overflow end of block */
+	if ((char *)dent->name + dent->namelen >
+	    (char *)bp->b_addr + BBTOB(bp->b_length)) {
+		xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
+		goto out_relse;
+	}
+
 	/* Does the directory hash match? */
 	dname.name = dent->name;
 	dname.len = dent->namelen;
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 1169a2ad26876f..9d853de63aea60 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -474,7 +474,10 @@ xrep_dir_recover_unused(
 {
 	struct xfs_dir2_data_unused	*dup = bp->b_addr + offset;
 
-	return be16_to_cpu(dup->length);
+	if (offset + sizeof(*dup) > end)
+		return 0;
+
+	return xfs_dir2_data_unusedsize(be16_to_cpu(dup->length));
 }
 
 /*
@@ -492,6 +495,11 @@ xrep_dir_recover_entry(
 	unsigned int			advance;
 	int				error;
 
+	/* must have namelen */
+	advance = offsetof(typeof(*dep), name);
+	if (offset + advance >= end)
+		return 0;
+
 	/* Don't walk off the end of the block. */
 	advance = xfs_dir2_data_entsize(rd->sc->mp, dep->namelen);
 	if (!advance)
@@ -534,6 +542,11 @@ xrep_dir_recover_data(
 		if (xchk_should_terminate(rd->sc, &error))
 			return error;
 
+		/* must have freetag */
+		advance = offsetof(typeof(*dup), length);
+		if (offset + advance >= end)
+			break;
+
 		if (dup->freetag == cpu_to_be16(XFS_DIR2_DATA_FREE_TAG))
 			advance = xrep_dir_recover_unused(bp, offset, end);
 		else
@@ -565,6 +578,19 @@ xrep_dir_recover_sf(
 	hdr = ifp->if_data;
 	end = (unsigned char *)ifp->if_data + ifp->if_bytes;
 
+	/* sf header must be big enough for count/i8count */
+	if (!hdr || ifp->if_bytes < 2)
+		return 0;
+
+	/* and large enough for the parent inumber */
+	if (hdr->i8count) {
+		if (ifp->if_bytes < offsetof(struct xfs_dir2_sf_hdr, parent[8]))
+			return 0;
+	} else {
+		if (ifp->if_bytes < offsetof(struct xfs_dir2_sf_hdr, parent[4]))
+			return 0;
+	}
+
 	ino = xfs_dir2_sf_get_parent_ino(hdr);
 	trace_xrep_dir_salvaged_parent(rd->sc->ip, ino);
 

[PATCH 08/12] xfs: drop unnecessary sa parameter from the xrep_ag init functions

[Darrick J. Wong]

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

LOLLM went down a crazy path complaining about the horrible things that
happen in these functions if sa != &sc->sa.  However, I observe that the
sa argument is never not &sc->sa, so let's just drop the parameter.
There's no bug here, but let's get rid of some stupid.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/repair.h            |    5 ++---
 fs/xfs/scrub/alloc_repair.c      |    2 +-
 fs/xfs/scrub/bmap_repair.c       |    2 +-
 fs/xfs/scrub/cow_repair.c        |    2 +-
 fs/xfs/scrub/ialloc_repair.c     |    2 +-
 fs/xfs/scrub/refcount_repair.c   |    2 +-
 fs/xfs/scrub/repair.c            |   10 +++++-----
 fs/xfs/scrub/rmap_repair.c       |    4 ++--
 fs/xfs/scrub/rtrefcount_repair.c |    2 +-
 fs/xfs/scrub/rtrmap_repair.c     |    2 +-
 10 files changed, 16 insertions(+), 17 deletions(-)


diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index 1af568651e20a1..a32fd9fb59bdae 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -100,9 +100,8 @@ int xrep_setup_ag_allocbt(struct xfs_scrub *sc);
 struct xfs_imap;
 int xrep_setup_inode(struct xfs_scrub *sc, const struct xfs_imap *imap);
 
-void xrep_ag_btcur_init(struct xfs_scrub *sc, struct xchk_ag *sa);
-int xrep_ag_init(struct xfs_scrub *sc, struct xfs_perag *pag,
-		struct xchk_ag *sa);
+void xrep_ag_btcur_init(struct xfs_scrub *sc);
+int xrep_ag_init(struct xfs_scrub *sc, struct xfs_perag *pag);
 #ifdef CONFIG_XFS_RT
 int xrep_rtgroup_init(struct xfs_scrub *sc, struct xfs_rtgroup *rtg,
 		struct xchk_rt *sr, unsigned int rtglock_flags);
diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 05f27c927adfb9..854cfbbd9ecf4f 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -370,7 +370,7 @@ xrep_abt_find_freespace(
 
 	xagb_bitmap_init(&ra->not_allocbt_blocks);
 
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	/*
 	 * Iterate all the reverse mappings to find gaps in the physical
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index eabffba47776e9..7288940b74e6c1 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -359,7 +359,7 @@ xrep_bmap_scan_ag(
 	struct xfs_scrub	*sc = rb->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index e49d9396edc527..3d4e94757de040 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -280,7 +280,7 @@ xrep_cow_find_bad(
 	if (!pag)
 		return -EFSCORRUPTED;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		goto out_pag;
 
diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
index 43846b3bbf4ed4..f0f2232a9686d8 100644
--- a/fs/xfs/scrub/ialloc_repair.c
+++ b/fs/xfs/scrub/ialloc_repair.c
@@ -473,7 +473,7 @@ xrep_ibt_find_inodes(
 	ri->rie.ir_startino = NULLAGINO;
 
 	/* Collect all reverse mappings for inode blocks. */
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_ibt_walk_rmap, ri);
 	xchk_ag_btcur_free(&sc->sa);
 	if (error)
diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
index ca9c382005ff1d..aa29d7a27abaa8 100644
--- a/fs/xfs/scrub/refcount_repair.c
+++ b/fs/xfs/scrub/refcount_repair.c
@@ -411,7 +411,7 @@ xrep_refc_find_refcounts(
 	bool			have;
 	int			error;
 
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	/*
 	 * Set up a bag to store all the rmap records that we're tracking to
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 956aa75218aa24..19c582223efe42 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -888,10 +888,10 @@ xrep_ino_ensure_extent_count(
  */
 void
 xrep_ag_btcur_init(
-	struct xfs_scrub	*sc,
-	struct xchk_ag		*sa)
+	struct xfs_scrub	*sc)
 {
 	struct xfs_mount	*mp = sc->mp;
+	struct xchk_ag		*sa = &sc->sa;
 
 	/* Set up a bnobt cursor for cross-referencing. */
 	if (sc->sm->sm_type != XFS_SCRUB_TYPE_BNOBT &&
@@ -990,9 +990,9 @@ xrep_reinit_pagi(
 int
 xrep_ag_init(
 	struct xfs_scrub	*sc,
-	struct xfs_perag	*pag,
-	struct xchk_ag		*sa)
+	struct xfs_perag	*pag)
 {
+	struct xchk_ag		*sa = &sc->sa;
 	int			error;
 
 	ASSERT(!sa->pag);
@@ -1007,7 +1007,7 @@ xrep_ag_init(
 
 	/* Grab our own passive reference from the caller's ref. */
 	sa->pag = xfs_perag_hold(pag);
-	xrep_ag_btcur_init(sc, sa);
+	xrep_ag_btcur_init(sc);
 	return 0;
 }
 
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index 725035bf490329..aac9e534df798f 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -910,7 +910,7 @@ xrep_rmap_find_rmaps(
 	int			error;
 
 	/* Find all the per-AG metadata. */
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	error = xrep_rmap_find_inode_rmaps(rr);
 	if (error)
@@ -1167,7 +1167,7 @@ xrep_rmap_reserve_space(
 	} while (!done);
 
 	/* Emit rmaps for everything in the free space bitmap. */
-	xrep_ag_btcur_init(rr->sc, &rr->sc->sa);
+	xrep_ag_btcur_init(rr->sc);
 	error = xrep_rmap_stash_bitmap(rr, &freesp_blocks, &XFS_RMAP_OINFO_AG);
 	xchk_ag_btcur_free(&rr->sc->sa);
 
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index 2b939960c7ddcd..b307ea15197090 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -420,7 +420,7 @@ xrep_rtrefc_scan_ag(
 	struct xfs_scrub	*sc = rr->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index a2b72e61edf5fe..6b435acc2d69ff 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -413,7 +413,7 @@ xrep_rtrmap_scan_ag(
 	struct xfs_scrub	*sc = rr->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;
 

[PATCH 09/12] xfs: drop unnecessary sr parameter from xrep_rtgroup init functions

[Darrick J. Wong]

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

Similar to the last patch, drop this unnecessary sr parameter instead of
dealing with pointer aliasing.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/repair.h            |    6 +++---
 fs/xfs/scrub/bmap_repair.c       |    7 +++----
 fs/xfs/scrub/cow_repair.c        |    4 ++--
 fs/xfs/scrub/inode_repair.c      |    2 +-
 fs/xfs/scrub/repair.c            |    9 +++++----
 fs/xfs/scrub/rtbitmap_repair.c   |    2 +-
 fs/xfs/scrub/rtrefcount_repair.c |    2 +-
 fs/xfs/scrub/rtrmap_repair.c     |    2 +-
 8 files changed, 17 insertions(+), 17 deletions(-)


diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index a32fd9fb59bdae..421b48de7667d5 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -104,13 +104,13 @@ void xrep_ag_btcur_init(struct xfs_scrub *sc);
 int xrep_ag_init(struct xfs_scrub *sc, struct xfs_perag *pag);
 #ifdef CONFIG_XFS_RT
 int xrep_rtgroup_init(struct xfs_scrub *sc, struct xfs_rtgroup *rtg,
-		struct xchk_rt *sr, unsigned int rtglock_flags);
-void xrep_rtgroup_btcur_init(struct xfs_scrub *sc, struct xchk_rt *sr);
+		unsigned int rtglock_flags);
+void xrep_rtgroup_btcur_init(struct xfs_scrub *sc);
 int xrep_require_rtext_inuse(struct xfs_scrub *sc, xfs_rgblock_t rgbno,
 		xfs_filblks_t len);
 xfs_extlen_t xrep_calc_rtgroup_resblks(struct xfs_scrub *sc);
 #else
-# define xrep_rtgroup_init(sc, rtg, sr, lockflags)	(-ENOSYS)
+# define xrep_rtgroup_init(sc, rtg, lockflags)		(-ENOSYS)
 # define xrep_calc_rtgroup_resblks(sc)			(0)
 #endif /* CONFIG_XFS_RT */
 
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index 7288940b74e6c1..5b422f5e13c21c 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -456,10 +456,9 @@ xrep_bmap_scan_rtgroup(
 	if (!xfs_has_rtrmapbt(sc->mp))
 		return 0;
 
-	error = xrep_rtgroup_init(sc, rtg, &sc->sr,
-			XFS_RTGLOCK_RMAP |
-			XFS_RTGLOCK_REFCOUNT |
-			XFS_RTGLOCK_BITMAP_SHARED);
+	error = xrep_rtgroup_init(sc, rtg, XFS_RTGLOCK_RMAP |
+					   XFS_RTGLOCK_REFCOUNT |
+					   XFS_RTGLOCK_BITMAP_SHARED);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index 3d4e94757de040..f2e650c6f7afdf 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -371,8 +371,8 @@ xrep_cow_find_bad_rt(
 	if (!rtg)
 		return -EFSCORRUPTED;
 
-	error = xrep_rtgroup_init(sc, rtg, &sc->sr,
-			XFS_RTGLOCK_RMAP | XFS_RTGLOCK_REFCOUNT);
+	error = xrep_rtgroup_init(sc, rtg, XFS_RTGLOCK_RMAP |
+					   XFS_RTGLOCK_REFCOUNT);
 	if (error)
 		goto out_rtg;
 
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 25e71e7d472f36..599f0f527e1a96 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -816,7 +816,7 @@ xrep_dinode_count_rtgroup_rmaps(
 	struct xfs_scrub	*sc = ri->sc;
 	int			error;
 
-	error = xrep_rtgroup_init(sc, rtg, &sc->sr, XFS_RTGLOCK_RMAP);
+	error = xrep_rtgroup_init(sc, rtg, XFS_RTGLOCK_RMAP);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 19c582223efe42..88494e79340ef8 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1015,10 +1015,10 @@ xrep_ag_init(
 /* Initialize all the btree cursors for a RT repair. */
 void
 xrep_rtgroup_btcur_init(
-	struct xfs_scrub	*sc,
-	struct xchk_rt		*sr)
+	struct xfs_scrub	*sc)
 {
 	struct xfs_mount	*mp = sc->mp;
+	struct xchk_rt		*sr = &sc->sr;
 
 	ASSERT(sr->rtg != NULL);
 
@@ -1041,9 +1041,10 @@ int
 xrep_rtgroup_init(
 	struct xfs_scrub	*sc,
 	struct xfs_rtgroup	*rtg,
-	struct xchk_rt		*sr,
 	unsigned int		rtglock_flags)
 {
+	struct xchk_rt		*sr = &sc->sr;
+
 	ASSERT(sr->rtg == NULL);
 
 	xfs_rtgroup_lock(rtg, rtglock_flags);
@@ -1051,7 +1052,7 @@ xrep_rtgroup_init(
 
 	/* Grab our own passive reference from the caller's ref. */
 	sr->rtg = xfs_rtgroup_hold(rtg);
-	xrep_rtgroup_btcur_init(sc, sr);
+	xrep_rtgroup_btcur_init(sc);
 	return 0;
 }
 
diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index f63aaaff783d50..3f918da62d4ced 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -339,7 +339,7 @@ xrep_rtbitmap_find_freespace(
 	/* Prepare a buffer of ones so that we can accelerate bulk setting. */
 	memset(rtb->words, 0xFF, mp->m_sb.sb_blocksize);
 
-	xrep_rtgroup_btcur_init(sc, &sc->sr);
+	xrep_rtgroup_btcur_init(sc);
 	error = xfs_rmap_query_all(sc->sr.rmap_cur, xrep_rtbitmap_walk_rtrmap,
 			rtb);
 	if (error)
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index b307ea15197090..e7f6d71695fd68 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -453,7 +453,7 @@ xrep_rtrefc_find_refcounts(
 		}
 	}
 
-	xrep_rtgroup_btcur_init(sc, &sc->sr);
+	xrep_rtgroup_btcur_init(sc);
 
 	/*
 	 * Set up a bag to store all the rmap records that we're tracking to
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 6b435acc2d69ff..755675460cb8a4 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -554,7 +554,7 @@ xrep_rtrmap_find_rmaps(
 	}
 
 	/* Find CoW staging extents. */
-	xrep_rtgroup_btcur_init(sc, &sc->sr);
+	xrep_rtgroup_btcur_init(sc);
 	error = xrep_rtrmap_find_refcount_rmaps(rr);
 	xchk_rtgroup_btcur_free(&sc->sr);
 	if (error)

[PATCH 10/12] xfs: drop unnecessary sa parameter from xchk_ag init functions

[Darrick J. Wong]

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

Drop the sa parameter because it's always the same value.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/common.h            |   17 +++++++----------
 fs/xfs/scrub/agheader.c          |   16 ++++++++--------
 fs/xfs/scrub/alloc_repair.c      |    2 +-
 fs/xfs/scrub/bmap.c              |    4 ++--
 fs/xfs/scrub/bmap_repair.c       |    2 +-
 fs/xfs/scrub/btree.c             |    4 ++--
 fs/xfs/scrub/common.c            |   28 +++++++++++++++-------------
 fs/xfs/scrub/cow_repair.c        |    2 +-
 fs/xfs/scrub/fscounters.c        |    4 ++--
 fs/xfs/scrub/ialloc_repair.c     |    2 +-
 fs/xfs/scrub/inode.c             |    4 ++--
 fs/xfs/scrub/refcount_repair.c   |    2 +-
 fs/xfs/scrub/repair.c            |    2 +-
 fs/xfs/scrub/rmap_repair.c       |    4 ++--
 fs/xfs/scrub/rtrefcount.c        |    4 ++--
 fs/xfs/scrub/rtrefcount_repair.c |    2 +-
 fs/xfs/scrub/rtrmap_repair.c     |    2 +-
 fs/xfs/scrub/scrub.c             |    2 +-
 18 files changed, 51 insertions(+), 52 deletions(-)


diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index f0f073a9341337..8478f3a9bed33d 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -109,9 +109,8 @@ xchk_ino_dqattach(struct xfs_scrub *sc)
 int xchk_setup_fscounters(struct xfs_scrub *sc);
 int xchk_setup_nlinks(struct xfs_scrub *sc);
 
-void xchk_ag_free(struct xfs_scrub *sc, struct xchk_ag *sa);
-int xchk_ag_init(struct xfs_scrub *sc, xfs_agnumber_t agno,
-		struct xchk_ag *sa);
+void xchk_ag_free(struct xfs_scrub *sc);
+int xchk_ag_init(struct xfs_scrub *sc, xfs_agnumber_t agno);
 int xchk_perag_drain_and_lock(struct xfs_scrub *sc);
 
 /*
@@ -122,10 +121,9 @@ int xchk_perag_drain_and_lock(struct xfs_scrub *sc);
 static inline int
 xchk_ag_init_existing(
 	struct xfs_scrub	*sc,
-	xfs_agnumber_t		agno,
-	struct xchk_ag		*sa)
+	xfs_agnumber_t		agno)
 {
-	int			error = xchk_ag_init(sc, agno, sa);
+	int			error = xchk_ag_init(sc, agno);
 
 	return error == -ENOENT ? -EFSCORRUPTED : error;
 }
@@ -165,10 +163,9 @@ void xchk_rtgroup_free(struct xfs_scrub *sc, struct xchk_rt *sr);
 # define xchk_rtgroup_free(sc, sr)			do { } while (0)
 #endif /* CONFIG_XFS_RT */
 
-int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno,
-		struct xchk_ag *sa);
-void xchk_ag_btcur_free(struct xchk_ag *sa);
-void xchk_ag_btcur_init(struct xfs_scrub *sc, struct xchk_ag *sa);
+int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno);
+void xchk_ag_btcur_free(struct xfs_scrub *sc);
+void xchk_ag_btcur_init(struct xfs_scrub *sc);
 int xchk_count_rmap_ownedby_ag(struct xfs_scrub *sc, struct xfs_btree_cur *cur,
 		const struct xfs_owner_info *oinfo, xfs_filblks_t *blocks);
 
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index a765d3e5e0686b..be59b0b495472f 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -47,7 +47,7 @@ xchk_superblock_xref(
 
 	agbno = XFS_SB_BLOCK(mp);
 
-	error = xchk_ag_init_existing(sc, agno, &sc->sa);
+	error = xchk_ag_init_existing(sc, agno);
 	if (!xchk_xref_process_error(sc, agno, agbno, &error))
 		return;
 
@@ -56,7 +56,7 @@ xchk_superblock_xref(
 	xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 	xchk_xref_is_not_cow_staging(sc, agbno, 1);
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 }
 
 /*
@@ -615,7 +615,7 @@ xchk_agf_xref(
 
 	agbno = XFS_AGF_BLOCK(mp);
 
-	xchk_ag_btcur_init(sc, &sc->sa);
+	xchk_ag_btcur_init(sc);
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_agf_xref_freeblks(sc);
@@ -648,7 +648,7 @@ xchk_agf(
 	int			level;
 	int			error = 0;
 
-	error = xchk_ag_read_headers(sc, agno, &sc->sa);
+	error = xchk_ag_read_headers(sc, agno);
 	if (!xchk_process_error(sc, agno, XFS_AGF_BLOCK(sc->mp), &error))
 		goto out;
 	xchk_buffer_recheck(sc, sc->sa.agf_bp);
@@ -803,7 +803,7 @@ xchk_agfl_xref(
 
 	agbno = XFS_AGFL_BLOCK(mp);
 
-	xchk_ag_btcur_init(sc, &sc->sa);
+	xchk_ag_btcur_init(sc);
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
@@ -831,7 +831,7 @@ xchk_agfl(
 	int			error;
 
 	/* Lock the AGF and AGI so that nobody can touch this AG. */
-	error = xchk_ag_read_headers(sc, agno, &sc->sa);
+	error = xchk_ag_read_headers(sc, agno);
 	if (!xchk_process_error(sc, agno, XFS_AGFL_BLOCK(sc->mp), &error))
 		return error;
 	if (!sc->sa.agf_bp)
@@ -958,7 +958,7 @@ xchk_agi_xref(
 
 	agbno = XFS_AGI_BLOCK(mp);
 
-	xchk_ag_btcur_init(sc, &sc->sa);
+	xchk_ag_btcur_init(sc);
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
@@ -1072,7 +1072,7 @@ xchk_agi(
 	int			level;
 	int			error = 0;
 
-	error = xchk_ag_read_headers(sc, agno, &sc->sa);
+	error = xchk_ag_read_headers(sc, agno);
 	if (!xchk_process_error(sc, agno, XFS_AGI_BLOCK(sc->mp), &error))
 		goto out;
 	xchk_buffer_recheck(sc, sc->sa.agi_bp);
diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 854cfbbd9ecf4f..2f60e4b517596e 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -406,7 +406,7 @@ xrep_abt_find_freespace(
 err_agfl:
 	xfs_trans_brelse(sc->tp, agfl_bp);
 err:
-	xchk_ag_btcur_free(&sc->sa);
+	xchk_ag_btcur_free(sc);
 	xagb_bitmap_destroy(&ra->not_allocbt_blocks);
 	return error;
 }
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 428be9ecdcd4d4..8447f711ca2c51 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -396,7 +396,7 @@ xchk_bmap_iextent_xref(
 	agbno = XFS_FSB_TO_AGBNO(mp, irec->br_startblock);
 	len = irec->br_blockcount;
 
-	error = xchk_ag_init_existing(info->sc, agno, &info->sc->sa);
+	error = xchk_ag_init_existing(info->sc, agno);
 	if (!xchk_fblock_process_error(info->sc, info->whichfork,
 			irec->br_startoff, &error))
 		goto out_free;
@@ -440,7 +440,7 @@ xchk_bmap_iextent_xref(
 	}
 
 out_free:
-	xchk_ag_free(info->sc, &info->sc->sa);
+	xchk_ag_free(info->sc);
 }
 
 /*
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index 5b422f5e13c21c..4d9ac54d5dfc11 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -364,7 +364,7 @@ xrep_bmap_scan_ag(
 		return error;
 
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_bmap_walk_rmap, rb);
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index ce8c9c1aaa3102..42769514118eda 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -397,7 +397,7 @@ xchk_btree_check_block_owner(
 	 */
 	init_sa = bs->cur->bc_ops->type != XFS_BTREE_TYPE_AG;
 	if (init_sa) {
-		error = xchk_ag_init_existing(bs->sc, agno, &bs->sc->sa);
+		error = xchk_ag_init_existing(bs->sc, agno);
 		if (!xchk_btree_xref_process_error(bs->sc, bs->cur,
 				level, &error))
 			goto out_free;
@@ -418,7 +418,7 @@ xchk_btree_check_block_owner(
 
 out_free:
 	if (init_sa)
-		xchk_ag_free(bs->sc, &bs->sc->sa);
+		xchk_ag_free(bs->sc);
 
 	return error;
 }
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index a61895872072d1..82cda70e4169ae 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -578,10 +578,10 @@ xchk_perag_drain_and_lock(
 int
 xchk_ag_read_headers(
 	struct xfs_scrub	*sc,
-	xfs_agnumber_t		agno,
-	struct xchk_ag		*sa)
+	xfs_agnumber_t		agno)
 {
 	struct xfs_mount	*mp = sc->mp;
+	struct xchk_ag		*sa = &sc->sa;
 
 	ASSERT(!sa->pag);
 	sa->pag = xfs_perag_get(mp, agno);
@@ -594,8 +594,10 @@ xchk_ag_read_headers(
 /* Release all the AG btree cursors. */
 void
 xchk_ag_btcur_free(
-	struct xchk_ag		*sa)
+	struct xfs_scrub	*sc)
 {
+	struct xchk_ag		*sa = &sc->sa;
+
 	if (sa->refc_cur)
 		xfs_btree_del_cursor(sa->refc_cur, XFS_BTREE_ERROR);
 	if (sa->rmap_cur)
@@ -620,10 +622,10 @@ xchk_ag_btcur_free(
 /* Initialize all the btree cursors for an AG. */
 void
 xchk_ag_btcur_init(
-	struct xfs_scrub	*sc,
-	struct xchk_ag		*sa)
+	struct xfs_scrub	*sc)
 {
 	struct xfs_mount	*mp = sc->mp;
+	struct xchk_ag		*sa = &sc->sa;
 
 	if (sa->agf_bp) {
 		/* Set up a bnobt cursor for cross-referencing. */
@@ -675,10 +677,11 @@ xchk_ag_btcur_init(
 /* Release the AG header context and btree cursors. */
 void
 xchk_ag_free(
-	struct xfs_scrub	*sc,
-	struct xchk_ag		*sa)
+	struct xfs_scrub	*sc)
 {
-	xchk_ag_btcur_free(sa);
+	struct xchk_ag		*sa = &sc->sa;
+
+	xchk_ag_btcur_free(sc);
 	xrep_reset_perag_resv(sc);
 	if (sa->agf_bp) {
 		xfs_trans_brelse(sc->tp, sa->agf_bp);
@@ -704,16 +707,15 @@ xchk_ag_free(
 int
 xchk_ag_init(
 	struct xfs_scrub	*sc,
-	xfs_agnumber_t		agno,
-	struct xchk_ag		*sa)
+	xfs_agnumber_t		agno)
 {
 	int			error;
 
-	error = xchk_ag_read_headers(sc, agno, sa);
+	error = xchk_ag_read_headers(sc, agno);
 	if (error)
 		return error;
 
-	xchk_ag_btcur_init(sc, sa);
+	xchk_ag_btcur_init(sc);
 	return 0;
 }
 
@@ -958,7 +960,7 @@ xchk_setup_ag_btree(
 	if (error)
 		return error;
 
-	return xchk_ag_init(sc, sc->sm->sm_agno, &sc->sa);
+	return xchk_ag_init(sc, sc->sm->sm_agno);
 }
 
 /* Push everything out of the log onto disk. */
diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index f2e650c6f7afdf..5a209194ba97eb 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -340,7 +340,7 @@ xrep_cow_find_bad(
 				xc->irec.br_blockcount);
 
 out_sa:
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 out_pag:
 	xfs_perag_put(pag);
 	return error;
diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index c055b73d268058..b9d0c3559d28f6 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -265,7 +265,7 @@ xchk_fscount_btreeblks(
 	xfs_filblks_t		blocks;
 	int			error;
 
-	error = xchk_ag_init_existing(sc, agno, &sc->sa);
+	error = xchk_ag_init_existing(sc, agno);
 	if (error)
 		goto out_free;
 
@@ -280,7 +280,7 @@ xchk_fscount_btreeblks(
 	fsc->fdblocks += blocks - 1;
 
 out_free:
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
index f0f2232a9686d8..146b3fec6d347c 100644
--- a/fs/xfs/scrub/ialloc_repair.c
+++ b/fs/xfs/scrub/ialloc_repair.c
@@ -475,7 +475,7 @@ xrep_ibt_find_inodes(
 	/* Collect all reverse mappings for inode blocks. */
 	xrep_ag_btcur_init(sc);
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_ibt_walk_rmap, ri);
-	xchk_ag_btcur_free(&sc->sa);
+	xchk_ag_btcur_free(sc);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index ac42867acbc4c9..6cded1eb6c2b6f 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -740,7 +740,7 @@ xchk_inode_xref(
 	agno = XFS_INO_TO_AGNO(sc->mp, ino);
 	agbno = XFS_INO_TO_AGBNO(sc->mp, ino);
 
-	error = xchk_ag_init_existing(sc, agno, &sc->sa);
+	error = xchk_ag_init_existing(sc, agno);
 	if (!xchk_xref_process_error(sc, agno, agbno, &error))
 		goto out_free;
 
@@ -752,7 +752,7 @@ xchk_inode_xref(
 	xchk_inode_xref_bmap(sc, dip);
 
 out_free:
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 }
 
 /*
diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
index aa29d7a27abaa8..36b5e2a92cfdae 100644
--- a/fs/xfs/scrub/refcount_repair.c
+++ b/fs/xfs/scrub/refcount_repair.c
@@ -503,7 +503,7 @@ xrep_refc_find_refcounts(
 out_bag:
 	rcbag_free(&rcstack);
 out_cur:
-	xchk_ag_btcur_free(&sc->sa);
+	xchk_ag_btcur_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 88494e79340ef8..32bd8259cedb65 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -68,7 +68,7 @@ xrep_attempt(
 
 	trace_xrep_attempt(XFS_I(file_inode(sc->file)), sc->sm, error);
 
-	xchk_ag_btcur_free(&sc->sa);
+	xchk_ag_btcur_free(sc);
 	xchk_rtgroup_btcur_free(&sc->sr);
 
 	/* Repair whatever's broken. */
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index aac9e534df798f..f88d7b62098b2b 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -926,7 +926,7 @@ xrep_rmap_find_rmaps(
 
 	error = xrep_rmap_find_log_rmaps(rr);
 end_agscan:
-	xchk_ag_btcur_free(&sc->sa);
+	xchk_ag_btcur_free(sc);
 	if (error)
 		return error;
 
@@ -1169,7 +1169,7 @@ xrep_rmap_reserve_space(
 	/* Emit rmaps for everything in the free space bitmap. */
 	xrep_ag_btcur_init(rr->sc);
 	error = xrep_rmap_stash_bitmap(rr, &freesp_blocks, &XFS_RMAP_OINFO_AG);
-	xchk_ag_btcur_free(&rr->sc->sa);
+	xchk_ag_btcur_free(rr->sc);
 
 out_bitmap:
 	xagb_bitmap_destroy(&freesp_blocks);
diff --git a/fs/xfs/scrub/rtrefcount.c b/fs/xfs/scrub/rtrefcount.c
index c312096e410561..824783e4f0e193 100644
--- a/fs/xfs/scrub/rtrefcount.c
+++ b/fs/xfs/scrub/rtrefcount.c
@@ -531,7 +531,7 @@ xchk_rtrefcount_count_agblocks(
 	xfs_filblks_t		agblocks = 0;
 	int			error;
 
-	error = xchk_ag_init_existing(sc, agno, &sc->sa);
+	error = xchk_ag_init_existing(sc, agno);
 	if (error)
 		goto out_free;
 
@@ -552,7 +552,7 @@ xchk_rtrefcount_count_agblocks(
 
 	*blocks += agblocks;
 out_free:
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index e7f6d71695fd68..2e773febffd78e 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -425,7 +425,7 @@ xrep_rtrefc_scan_ag(
 		return error;
 
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_rtrefc_walk_rmap, rr);
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 755675460cb8a4..726ef89ab2f10b 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -418,7 +418,7 @@ xrep_rtrmap_scan_ag(
 		return error;
 
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_rtrmap_walk_rmap, rr);
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 12c228b7f4770d..9dbd16546e8c8b 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -229,7 +229,7 @@ xchk_teardown(
 	struct xfs_scrub	*sc,
 	int			error)
 {
-	xchk_ag_free(sc, &sc->sa);
+	xchk_ag_free(sc);
 	xchk_rtgroup_btcur_free(&sc->sr);
 
 	if (sc->tp) {

[PATCH 11/12] xfs: drop unnecessary sr parameter from xchk_rtgroup init functions

[Darrick J. Wong]

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

Drop the sr parameter from the scrub rtgroup functions because the value
is always the same.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/common.h            |   29 +++++++++++++----------------
 fs/xfs/scrub/bmap.c              |    9 ++++-----
 fs/xfs/scrub/bmap_repair.c       |    4 ++--
 fs/xfs/scrub/common.c            |   22 ++++++++++++++--------
 fs/xfs/scrub/cow_repair.c        |    4 ++--
 fs/xfs/scrub/inode_repair.c      |    4 ++--
 fs/xfs/scrub/repair.c            |    2 +-
 fs/xfs/scrub/rgsuper.c           |    4 ++--
 fs/xfs/scrub/rtbitmap.c          |    4 ++--
 fs/xfs/scrub/rtbitmap_repair.c   |    2 +-
 fs/xfs/scrub/rtrefcount.c        |    4 ++--
 fs/xfs/scrub/rtrefcount_repair.c |    2 +-
 fs/xfs/scrub/rtrmap.c            |    4 ++--
 fs/xfs/scrub/rtrmap_repair.c     |    6 +++---
 fs/xfs/scrub/rtsummary.c         |    4 ++--
 fs/xfs/scrub/scrub.c             |    4 ++--
 16 files changed, 55 insertions(+), 53 deletions(-)


diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index 8478f3a9bed33d..181028f0c65526 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -135,32 +135,29 @@ xchk_ag_init_existing(
 				 XFS_RTGLOCK_RMAP | \
 				 XFS_RTGLOCK_REFCOUNT)
 
-int xchk_rtgroup_init(struct xfs_scrub *sc, xfs_rgnumber_t rgno,
-		struct xchk_rt *sr);
+int xchk_rtgroup_init(struct xfs_scrub *sc, xfs_rgnumber_t rgno);
 
 static inline int
 xchk_rtgroup_init_existing(
 	struct xfs_scrub	*sc,
-	xfs_rgnumber_t		rgno,
-	struct xchk_rt		*sr)
+	xfs_rgnumber_t		rgno)
 {
-	int			error = xchk_rtgroup_init(sc, rgno, sr);
+	int			error = xchk_rtgroup_init(sc, rgno);
 
 	return error == -ENOENT ? -EFSCORRUPTED : error;
 }
 
-int xchk_rtgroup_lock(struct xfs_scrub *sc, struct xchk_rt *sr,
-		unsigned int rtglock_flags);
-void xchk_rtgroup_unlock(struct xchk_rt *sr);
-void xchk_rtgroup_btcur_free(struct xchk_rt *sr);
-void xchk_rtgroup_free(struct xfs_scrub *sc, struct xchk_rt *sr);
+int xchk_rtgroup_lock(struct xfs_scrub *sc, unsigned int rtglock_flags);
+void xchk_rtgroup_unlock(struct xfs_scrub *sc);
+void xchk_rtgroup_btcur_free(struct xfs_scrub *sc);
+void xchk_rtgroup_free(struct xfs_scrub *sc);
 #else
-# define xchk_rtgroup_init(sc, rgno, sr)		(-EFSCORRUPTED)
-# define xchk_rtgroup_init_existing(sc, rgno, sr)	(-EFSCORRUPTED)
-# define xchk_rtgroup_lock(sc, sr, lockflags)		(-EFSCORRUPTED)
-# define xchk_rtgroup_unlock(sr)			do { } while (0)
-# define xchk_rtgroup_btcur_free(sr)			do { } while (0)
-# define xchk_rtgroup_free(sc, sr)			do { } while (0)
+# define xchk_rtgroup_init(sc, rgno)			(-EFSCORRUPTED)
+# define xchk_rtgroup_init_existing(sc, rgno)		(-EFSCORRUPTED)
+# define xchk_rtgroup_lock(sc, lockflags)		(-EFSCORRUPTED)
+# define xchk_rtgroup_unlock(sc)			do { } while (0)
+# define xchk_rtgroup_btcur_free(sc)			do { } while (0)
+# define xchk_rtgroup_free(sc)				do { } while (0)
 #endif /* CONFIG_XFS_RT */
 
 int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno);
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 8447f711ca2c51..3c543f77cf6de5 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -329,13 +329,12 @@ xchk_bmap_rt_iextent_xref(
 	int			error;
 
 	error = xchk_rtgroup_init_existing(info->sc,
-			xfs_rtb_to_rgno(ip->i_mount, irec->br_startblock),
-			&info->sc->sr);
+			xfs_rtb_to_rgno(ip->i_mount, irec->br_startblock));
 	if (!xchk_fblock_process_error(info->sc, info->whichfork,
 			irec->br_startoff, &error))
 		return;
 
-	error = xchk_rtgroup_lock(info->sc, &info->sc->sr, XCHK_RTGLOCK_ALL);
+	error = xchk_rtgroup_lock(info->sc, XCHK_RTGLOCK_ALL);
 	if (!xchk_fblock_process_error(info->sc, info->whichfork,
 			irec->br_startoff, &error))
 		goto out_free;
@@ -373,9 +372,9 @@ xchk_bmap_rt_iextent_xref(
 		break;
 	}
 out_cur:
-	xchk_rtgroup_btcur_free(&info->sc->sr);
+	xchk_rtgroup_btcur_free(info->sc);
 out_free:
-	xchk_rtgroup_free(info->sc, &info->sc->sr);
+	xchk_rtgroup_free(info->sc);
 }
 
 /* Cross-reference a single datadev extent record. */
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index 4d9ac54d5dfc11..85e677d8a77674 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -463,8 +463,8 @@ xrep_bmap_scan_rtgroup(
 		return error;
 
 	error = xfs_rmap_query_all(sc->sr.rmap_cur, xrep_bmap_walk_rtrmap, rb);
-	xchk_rtgroup_btcur_free(&sc->sr);
-	xchk_rtgroup_free(sc, &sc->sr);
+	xchk_rtgroup_btcur_free(sc);
+	xchk_rtgroup_free(sc);
 	return error;
 }
 #else
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 82cda70e4169ae..cbfe5e6f779dfc 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -730,9 +730,10 @@ xchk_ag_init(
 int
 xchk_rtgroup_init(
 	struct xfs_scrub	*sc,
-	xfs_rgnumber_t		rgno,
-	struct xchk_rt		*sr)
+	xfs_rgnumber_t		rgno)
 {
+	struct xchk_rt		*sr = &sc->sr;
+
 	ASSERT(sr->rtg == NULL);
 	ASSERT(sr->rtlock_flags == 0);
 
@@ -746,9 +747,9 @@ xchk_rtgroup_init(
 int
 xchk_rtgroup_lock(
 	struct xfs_scrub	*sc,
-	struct xchk_rt		*sr,
 	unsigned int		rtglock_flags)
 {
+	struct xchk_rt		*sr = &sc->sr;
 	int			error = 0;
 
 	ASSERT(sr->rtg != NULL);
@@ -831,8 +832,10 @@ xchk_rtgroup_lock(
  */
 void
 xchk_rtgroup_btcur_free(
-	struct xchk_rt		*sr)
+	struct xfs_scrub	*sc)
 {
+	struct xchk_rt		*sr = &sc->sr;
+
 	if (sr->rmap_cur)
 		xfs_btree_del_cursor(sr->rmap_cur, XFS_BTREE_ERROR);
 	if (sr->refc_cur)
@@ -848,8 +851,10 @@ xchk_rtgroup_btcur_free(
  */
 void
 xchk_rtgroup_unlock(
-	struct xchk_rt		*sr)
+	struct xfs_scrub	*sc)
 {
+	struct xchk_rt		*sr = &sc->sr;
+
 	ASSERT(sr->rtg != NULL);
 
 	if (sr->rtlock_flags) {
@@ -864,12 +869,13 @@ xchk_rtgroup_unlock(
  */
 void
 xchk_rtgroup_free(
-	struct xfs_scrub	*sc,
-	struct xchk_rt		*sr)
+	struct xfs_scrub	*sc)
 {
+	struct xchk_rt		*sr = &sc->sr;
+
 	ASSERT(sr->rtg != NULL);
 
-	xchk_rtgroup_unlock(sr);
+	xchk_rtgroup_unlock(sc);
 
 	xfs_rtgroup_put(sr->rtg);
 	sr->rtg = NULL;
diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index 5a209194ba97eb..2b1d1701cb539f 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -433,8 +433,8 @@ xrep_cow_find_bad_rt(
 				xc->irec.br_blockcount);
 
 out_sr:
-	xchk_rtgroup_btcur_free(&sc->sr);
-	xchk_rtgroup_free(sc, &sc->sr);
+	xchk_rtgroup_btcur_free(sc);
+	xchk_rtgroup_free(sc);
 out_rtg:
 	xfs_rtgroup_put(rtg);
 	return error;
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 599f0f527e1a96..a42ce1d014dfb3 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -822,8 +822,8 @@ xrep_dinode_count_rtgroup_rmaps(
 
 	error = xfs_rmap_query_all(sc->sr.rmap_cur, xrep_dinode_walk_rtrmap,
 			ri);
-	xchk_rtgroup_btcur_free(&sc->sr);
-	xchk_rtgroup_free(sc, &sc->sr);
+	xchk_rtgroup_btcur_free(sc);
+	xchk_rtgroup_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 32bd8259cedb65..5827717f0d460f 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -69,7 +69,7 @@ xrep_attempt(
 	trace_xrep_attempt(XFS_I(file_inode(sc->file)), sc->sm, error);
 
 	xchk_ag_btcur_free(sc);
-	xchk_rtgroup_btcur_free(&sc->sr);
+	xchk_rtgroup_btcur_free(sc);
 
 	/* Repair whatever's broken. */
 	ASSERT(sc->ops->repair);
diff --git a/fs/xfs/scrub/rgsuper.c b/fs/xfs/scrub/rgsuper.c
index 102bd92e9003db..591c5767208e6e 100644
--- a/fs/xfs/scrub/rgsuper.c
+++ b/fs/xfs/scrub/rgsuper.c
@@ -65,7 +65,7 @@ xchk_rgsuperblock(
 	 * signal that the group no longer exists.  Take the rtbitmap in shared
 	 * mode so that the group can't change while we're doing things.
 	 */
-	error = xchk_rtgroup_init_existing(sc, rgno, &sc->sr);
+	error = xchk_rtgroup_init_existing(sc, rgno);
 	if (!xchk_xref_process_error(sc, 0, 0, &error))
 		return error;
 
@@ -74,7 +74,7 @@ xchk_rgsuperblock(
 	else
 		flags = XFS_RTGLOCK_BITMAP_SHARED;
 
-	error = xchk_rtgroup_lock(sc, &sc->sr, flags);
+	error = xchk_rtgroup_lock(sc, flags);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rtbitmap.c b/fs/xfs/scrub/rtbitmap.c
index 52c24d3d4be6ce..88a53c6ed6c531 100644
--- a/fs/xfs/scrub/rtbitmap.c
+++ b/fs/xfs/scrub/rtbitmap.c
@@ -48,7 +48,7 @@ xchk_setup_rtbitmap(
 	sc->buf = rtb;
 	rtb->sc = sc;
 
-	error = xchk_rtgroup_init(sc, sc->sm->sm_agno, &sc->sr);
+	error = xchk_rtgroup_init(sc, sc->sm->sm_agno);
 	if (error)
 		return error;
 
@@ -70,7 +70,7 @@ xchk_setup_rtbitmap(
 	if (error)
 		return error;
 
-	error = xchk_rtgroup_lock(sc, &sc->sr, XCHK_RTGLOCK_ALL);
+	error = xchk_rtgroup_lock(sc, XCHK_RTGLOCK_ALL);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index 3f918da62d4ced..f8a86596d4bab8 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -357,7 +357,7 @@ xrep_rtbitmap_find_freespace(
 	}
 
 out:
-	xchk_rtgroup_btcur_free(&sc->sr);
+	xchk_rtgroup_btcur_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/rtrefcount.c b/fs/xfs/scrub/rtrefcount.c
index 824783e4f0e193..b8f5a5f9fd01d2 100644
--- a/fs/xfs/scrub/rtrefcount.c
+++ b/fs/xfs/scrub/rtrefcount.c
@@ -42,7 +42,7 @@ xchk_setup_rtrefcountbt(
 			return error;
 	}
 
-	error = xchk_rtgroup_init(sc, sc->sm->sm_agno, &sc->sr);
+	error = xchk_rtgroup_init(sc, sc->sm->sm_agno);
 	if (error)
 		return error;
 
@@ -54,7 +54,7 @@ xchk_setup_rtrefcountbt(
 	if (error)
 		return error;
 
-	return xchk_rtgroup_lock(sc, &sc->sr, XCHK_RTGLOCK_ALL);
+	return xchk_rtgroup_lock(sc, XCHK_RTGLOCK_ALL);
 }
 
 /* Realtime Reference count btree scrubber. */
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index 2e773febffd78e..817c2b8f49ab6c 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -545,7 +545,7 @@ xrep_rtrefc_find_refcounts(
 out_bag:
 	rcbag_free(&rcstack);
 out_cur:
-	xchk_rtgroup_btcur_free(&sc->sr);
+	xchk_rtgroup_btcur_free(sc);
 	return error;
 }
 
diff --git a/fs/xfs/scrub/rtrmap.c b/fs/xfs/scrub/rtrmap.c
index 564d19a97a2f8f..85cc0513d688c9 100644
--- a/fs/xfs/scrub/rtrmap.c
+++ b/fs/xfs/scrub/rtrmap.c
@@ -46,7 +46,7 @@ xchk_setup_rtrmapbt(
 			return error;
 	}
 
-	error = xchk_rtgroup_init(sc, sc->sm->sm_agno, &sc->sr);
+	error = xchk_rtgroup_init(sc, sc->sm->sm_agno);
 	if (error)
 		return error;
 
@@ -58,7 +58,7 @@ xchk_setup_rtrmapbt(
 	if (error)
 		return error;
 
-	return xchk_rtgroup_lock(sc, &sc->sr, XCHK_RTGLOCK_ALL);
+	return xchk_rtgroup_lock(sc, XCHK_RTGLOCK_ALL);
 }
 
 /* Realtime reverse mapping. */
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 726ef89ab2f10b..6a10463de20021 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -556,7 +556,7 @@ xrep_rtrmap_find_rmaps(
 	/* Find CoW staging extents. */
 	xrep_rtgroup_btcur_init(sc);
 	error = xrep_rtrmap_find_refcount_rmaps(rr);
-	xchk_rtgroup_btcur_free(&sc->sr);
+	xchk_rtgroup_btcur_free(sc);
 	if (error)
 		return error;
 
@@ -576,7 +576,7 @@ xrep_rtrmap_find_rmaps(
 	 * do not take sb_internal.
 	 */
 	xchk_trans_cancel(sc);
-	xchk_rtgroup_unlock(&sc->sr);
+	xchk_rtgroup_unlock(sc);
 	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&rr->iscan, &ip)) == 1) {
@@ -600,7 +600,7 @@ xrep_rtrmap_find_rmaps(
 	error = xchk_setup_rt(sc);
 	if (error)
 		return error;
-	error = xchk_rtgroup_lock(sc, &sc->sr, XCHK_RTGLOCK_ALL);
+	error = xchk_rtgroup_lock(sc, XCHK_RTGLOCK_ALL);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
index fa358746beaf5c..b72c27ef45acbb 100644
--- a/fs/xfs/scrub/rtsummary.c
+++ b/fs/xfs/scrub/rtsummary.c
@@ -54,7 +54,7 @@ xchk_setup_rtsummary(
 		return -ENOMEM;
 	sc->buf = rts;
 
-	error = xchk_rtgroup_init(sc, sc->sm->sm_agno, &sc->sr);
+	error = xchk_rtgroup_init(sc, sc->sm->sm_agno);
 	if (error)
 		return error;
 
@@ -85,7 +85,7 @@ xchk_setup_rtsummary(
 	if (error)
 		return error;
 
-	error = xchk_rtgroup_lock(sc, &sc->sr, XFS_RTGLOCK_BITMAP);
+	error = xchk_rtgroup_lock(sc, XFS_RTGLOCK_BITMAP);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 9dbd16546e8c8b..4a2d9bdc087b5f 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -230,7 +230,7 @@ xchk_teardown(
 	int			error)
 {
 	xchk_ag_free(sc);
-	xchk_rtgroup_btcur_free(&sc->sr);
+	xchk_rtgroup_btcur_free(sc);
 
 	if (sc->tp) {
 		if (error == 0 && (sc->sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR))
@@ -240,7 +240,7 @@ xchk_teardown(
 		sc->tp = NULL;
 	}
 	if (sc->sr.rtg)
-		xchk_rtgroup_free(sc, &sc->sr);
+		xchk_rtgroup_free(sc);
 	if (sc->ip) {
 		if (sc->ilock_flags)
 			xchk_iunlock(sc, sc->ilock_flags);

[PATCH 12/12] xfs: refactor inode forkoff to byte conversions

[Darrick J. Wong]

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

A reviewer suggested that I formalize the conversion between the inode
forkoff field and actual byte count, so let's add a few conversion
macros and a helper function to compute the default forkoff; and then
adjust the codebase to use them.

No functional changes intended.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.h       |    6 ++++++
 fs/xfs/libxfs/xfs_format.h     |    6 +++++-
 fs/xfs/xfs_inode.h             |    2 +-
 fs/xfs/libxfs/xfs_attr_leaf.c  |   12 ++++++------
 fs/xfs/libxfs/xfs_bmap.c       |    4 ++--
 fs/xfs/libxfs/xfs_inode_buf.c  |    4 ++--
 fs/xfs/libxfs/xfs_inode_util.c |    2 +-
 fs/xfs/scrub/inode.c           |    2 +-
 fs/xfs/scrub/inode_repair.c    |    4 ++--
 9 files changed, 26 insertions(+), 16 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index d5f2729305fada..d3c7641e36f5e2 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -213,6 +213,12 @@ void	xfs_bmap_del_extent_cow(struct xfs_inode *ip,
 		struct xfs_iext_cursor *cur, struct xfs_bmbt_irec *got,
 		struct xfs_bmbt_irec *del);
 uint	xfs_default_attroffset(struct xfs_inode *ip);
+
+static inline uint xfs_inode_default_forkoff(struct xfs_inode *ip)
+{
+	return XFS_B_TO_FORKOFFT(xfs_default_attroffset(ip));
+}
+
 int	xfs_bmap_collapse_extents(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
 		bool *done);
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 1a7a7e60a170c4..469e879149ac21 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1096,7 +1096,11 @@ enum xfs_dinode_fmt {
 /*
  * Inode data & attribute fork sizes, per inode.
  */
-#define XFS_DFORK_BOFF(dip)		((int)((dip)->di_forkoff << 3))
+#define XFS_B_TO_FORKOFFT(b)		((b) >> 3)
+#define XFS_B_TO_FORKOFF(b)		XFS_B_TO_FORKOFFT((b) + 7)
+#define XFS_FORKOFF_TO_B(f)		((int)(f) << 3)
+
+#define XFS_DFORK_BOFF(dip)		XFS_FORKOFF_TO_B((dip)->di_forkoff)
 
 #define XFS_DFORK_DSIZE(dip,mp) \
 	((dip)->di_forkoff ? XFS_DFORK_BOFF(dip) : XFS_LITINO(mp))
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 1602027cd0aa5d..53a1997b98660b 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -132,7 +132,7 @@ xfs_ifork_ptr(
 
 static inline unsigned int xfs_inode_fork_boff(struct xfs_inode *ip)
 {
-	return ip->i_forkoff << 3;
+	return XFS_FORKOFF_TO_B(ip->i_forkoff);
 }
 
 static inline unsigned int xfs_inode_data_fork_size(struct xfs_inode *ip)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 2c80f4fd0b78de..1cefd41a2f9e01 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -720,10 +720,10 @@ xfs_attr_shortform_bytesfit(
 		return 0;
 
 	/* rounded down */
-	offset = (XFS_LITINO(mp) - bytes) >> 3;
+	offset = XFS_B_TO_FORKOFFT(XFS_LITINO(mp) - bytes);
 
 	if (dp->i_df.if_format == XFS_DINODE_FMT_DEV) {
-		minforkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
+		minforkoff = XFS_B_TO_FORKOFF(sizeof(xfs_dev_t));
 		return (offset >= minforkoff) ? minforkoff : 0;
 	}
 
@@ -779,12 +779,12 @@ xfs_attr_shortform_bytesfit(
 	 * A data fork btree root must have space for at least
 	 * MINDBTPTRS key/ptr pairs if the data fork is small or empty.
 	 */
-	minforkoff = max_t(int64_t, dsize, xfs_bmdr_space_calc(MINDBTPTRS));
-	minforkoff = roundup(minforkoff, 8) >> 3;
+	minforkoff = XFS_B_TO_FORKOFF(
+			max_t(int64_t, dsize, xfs_bmdr_space_calc(MINDBTPTRS)));
 
 	/* attr fork btree root can have at least this many key/ptr pairs */
-	maxforkoff = XFS_LITINO(mp) - xfs_bmdr_space_calc(MINABTPTRS);
-	maxforkoff = maxforkoff >> 3;	/* rounded down */
+	maxforkoff = XFS_B_TO_FORKOFFT(
+			XFS_LITINO(mp) - xfs_bmdr_space_calc(MINABTPTRS));
 
 	if (offset >= maxforkoff)
 		return maxforkoff;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index d64defeda645ed..7e463314af4dcc 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -221,7 +221,7 @@ xfs_bmap_forkoff_reset(
 	if (whichfork == XFS_ATTR_FORK &&
 	    ip->i_df.if_format != XFS_DINODE_FMT_DEV &&
 	    ip->i_df.if_format != XFS_DINODE_FMT_BTREE) {
-		uint	dfl_forkoff = xfs_default_attroffset(ip) >> 3;
+		uint	dfl_forkoff = xfs_inode_default_forkoff(ip);
 
 		if (dfl_forkoff > ip->i_forkoff)
 			ip->i_forkoff = dfl_forkoff;
@@ -997,7 +997,7 @@ xfs_bmap_set_attrforkoff(
 	struct xfs_inode	*ip,
 	int			size)
 {
-	int			default_size = xfs_default_attroffset(ip) >> 3;
+	int			default_size = xfs_inode_default_forkoff(ip);
 
 	switch (ip->i_df.if_format) {
 	case XFS_DINODE_FMT_DEV:
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 0340e2189921db..df77dd5c97611c 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -490,7 +490,7 @@ xfs_dinode_verify_forkoff(
 
 	switch (dip->di_format)  {
 	case XFS_DINODE_FMT_DEV:
-		if (dip->di_forkoff != (roundup(sizeof(xfs_dev_t), 8) >> 3))
+		if (dip->di_forkoff != XFS_B_TO_FORKOFF(sizeof(xfs_dev_t)))
 			return __this_address;
 		break;
 	case XFS_DINODE_FMT_META_BTREE:
@@ -500,7 +500,7 @@ xfs_dinode_verify_forkoff(
 	case XFS_DINODE_FMT_LOCAL:	/* fall through ... */
 	case XFS_DINODE_FMT_EXTENTS:    /* fall through ... */
 	case XFS_DINODE_FMT_BTREE:
-		if (dip->di_forkoff >= (XFS_LITINO(mp) >> 3))
+		if (dip->di_forkoff >= XFS_B_TO_FORKOFFT(XFS_LITINO(mp)))
 			return __this_address;
 		break;
 	default:
diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index 258ac3d0d486e6..e63bb1fad9c014 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -346,7 +346,7 @@ xfs_inode_init(
 	}
 
 	if (xfs_icreate_want_attrfork(mp, args)) {
-		ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
+		ip->i_forkoff = xfs_inode_default_forkoff(ip);
 		xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0);
 
 		if (!xfs_has_attr(mp)) {
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 6cded1eb6c2b6f..bb8d2bc008e019 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -607,7 +607,7 @@ xchk_dinode(
 	}
 
 	/* di_forkoff */
-	if (dip->di_forkoff >= (XFS_LITINO(mp) >> 3))
+	if (dip->di_forkoff >= XFS_B_TO_FORKOFFT(XFS_LITINO(mp)))
 		xchk_ino_set_corrupt(sc, ino);
 	if (naextents != 0 && dip->di_forkoff == 0)
 		xchk_ino_set_corrupt(sc, ino);
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index a42ce1d014dfb3..c6f221254f0bb0 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1490,7 +1490,7 @@ xrep_dinode_ensure_forkoff(
 
 			/* Otherwise, just slide the attr fork up. */
 			before = XFS_DFORK_APTR(dip);
-			dip->di_forkoff = bmdr_minsz >> 3;
+			dip->di_forkoff = XFS_B_TO_FORKOFFT(bmdr_minsz);
 			after = XFS_DFORK_APTR(dip);
 			memmove(after, before, XFS_DFORK_ASIZE(dip, sc->mp));
 		}
@@ -1522,7 +1522,7 @@ xrep_dinode_ensure_forkoff(
 			 * fork is empty, so we don't have any old contents to
 			 * move here.
 			 */
-			dip->di_forkoff = (lit_sz - bmdr_minsz) >> 3;
+			dip->di_forkoff = XFS_B_TO_FORKOFFT(lit_sz - bmdr_minsz);
 		}
 	}
 }

Re: [PATCH 01/12] xfs: don't set a metadata directory's parent to sb_rootino

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:59:07PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that it's weird to set a metadata directory's parent
> inumber to the regular directory tree root.  Fix this even if nobody's
> ever going to notice this because metadata path lookups don't access
> dotdot.

Looks good:

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

Re: [PATCH 02/12] xfs: don't rescan ifree and icount when frozen

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:59:23PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that it's pointless to rescan the AGs if we've already
> frozen the filesystem because there won't be any more perturbations.
> Fix this to mark the scan incomplete and exit.

Looks good:

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

Re: [PATCH 03/12] xfs: rescan the AGs if incore delalloc counter is negative

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:59:38PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that percpu_counter_sum can return a negative number but
> we instead misinterpret that into a huge unsigned value and do bad
> things to fdblocks.  It shouldn't do that, so let's add a retry loop and
> an attempt to freeze the filesystem to (hopefully) zero out the incore
> delalloc counter.  It probably won't work, but at least failing the
> repair beats doing the wrong thing.

I wonder how we could even end up with a negative value here, given
that we should not see live changes.  I.e. might it make more sense
to just treat the theoretical negative values as 0 instead of a never
exercises retry path?

Re: [PATCH 04/12] xfs: don't try to repair rtsummary if the summary file isn't computed

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 05/12] xfs: update quota flags on secondary supers when metadir enabled

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 11:00:09PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfs/085 exposes an interesting bug when metadir is enabled, quotas are
> enabled via mount option, and the primary superblock crc fails
> validation:

The code looks ok, but I think the Sashiko complaints about non-metadir
fses at least are valid.  Handling the errors seems fine, but maybe the
comments shouldn't mislead it that the update is purely opportunistic?

Re: [PATCH 06/12] xfs: refactor directory data block walking loops in scrub

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 07/12] xfs: improve dirent bounds checking in scrub and repair

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 08/12] xfs: drop unnecessary sa parameter from the xrep_ag init functions

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 11:00:56PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM went down a crazy path complaining about the horrible things that
> happen in these functions if sa != &sc->sa.  However, I observe that the
> sa argument is never not &sc->sa, so let's just drop the parameter.
> There's no bug here, but let's get rid of some stupid.

Looks good:

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

Re: [PATCH 09/12] xfs: drop unnecessary sr parameter from xrep_rtgroup init functions

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 10/12] xfs: drop unnecessary sa parameter from xchk_ag init functions

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 11/12] xfs: drop unnecessary sr parameter from xchk_rtgroup init functions

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 11:01:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Drop the sr parameter from the scrub rtgroup functions because the value
> is always the same.

Looks good:

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

Re: [PATCH 12/12] xfs: refactor inode forkoff to byte conversions

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 03/12] xfs: rescan the AGs if incore delalloc counter is negative

[Darrick J. Wong]

On Mon, Oct 12, 2026 at 06:16:37AM -0700, Christoph Hellwig wrote:
> On Sun, Oct 11, 2026 at 10:59:38PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM complains that percpu_counter_sum can return a negative number but
> > we instead misinterpret that into a huge unsigned value and do bad
> > things to fdblocks.  It shouldn't do that, so let's add a retry loop and
> > an attempt to freeze the filesystem to (hopefully) zero out the incore
> > delalloc counter.  It probably won't work, but at least failing the
> > repair beats doing the wrong thing.
> 
> I wonder how we could even end up with a negative value here, given
> that we should not see live changes.  I.e. might it make more sense
> to just treat the theoretical negative values as 0 instead of a never
> exercises retry path?

I'm not sure how in practice you'd end up with a negative value in
the incore delalloc count.  I changed it to try to freeze the filesystem
on the off chance that the summary counter corruption is mirrored in an
xfs_inode somewhere, and that purging them out will fix it.  Or take
down the fs :P

But I could just bail entirely if the incore count is negative, since
in theory this situation Should Never Happen(tm):

	delayed = percpu_counter_sum(&mp->m_delalloc_blks);
	if (delayed < 0) {
		ASSERT(delayed >= 0);
		xchk_set_incomplete(sc);
		return -EFSCORRUPTED;
	}

Perhaps a better long term fix would be to sum the i_delayed_blks
counters across the entire xfs_inode cache and correct the incore
counter, but that sounds expensive for something that we shouldn't have
to worry about unless the CPU forgets arithmetic.

Oh hey, I should do the same for m_delalloc_rtextents.  Let me go fix
that too.

--D

Re: [PATCH 05/12] xfs: update quota flags on secondary supers when metadir enabled

[Darrick J. Wong]

On Mon, Oct 12, 2026 at 06:17:46AM -0700, Christoph Hellwig wrote:
> On Sun, Oct 11, 2026 at 11:00:09PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs/085 exposes an interesting bug when metadir is enabled, quotas are
> > enabled via mount option, and the primary superblock crc fails
> > validation:
> 
> The code looks ok, but I think the Sashiko complaints about non-metadir
> fses at least are valid.

Yep, I'll fix that.

> Handling the errors seems fine, but maybe the
> comments shouldn't mislead it that the update is purely opportunistic?

Yeah.  As written this patch makes secondary sb update failures block
mount, but on second thought I decided that since the secondaries only
come into play for xfs_repair, and only if the primary gets clobbered,
this all can get backed down to a warning:


/*
 * For metadir filesystems, quota flag state persists across mounts, so
 * update secondary superblocks in case xfs_repair ever has to recover
 * the primary from a secondary.  growfs also updates backup supers so
 * lock against that.  Note that the secondary backups are
 * opportunistic, so we log but do not return write errors.
 */
void
__xfs_qm_update_secondary_metadir_sb(
	struct xfs_mount	*mp,
	const char		*func)
{
	int			error;

	if (!xfs_has_metadir(mp))
		return;

	mutex_lock(&mp->m_growlock);
	error = xfs_update_secondary_sbs(mp);
	mutex_unlock(&mp->m_growlock);

	if (error)
		xfs_alert(mp, "%s: Secondary super update failed, err=%d!",
				func, error);
}

And this sole helper can get called from the three places that need it.

--D

[PATCH 08/12] xfs: drop unnecessary sa parameter from the xrep_ag init functions

[Darrick J. Wong]

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

LOLLM went down a crazy path complaining about the horrible things that
happen in these functions if sa != &sc->sa.  However, I observe that the
sa argument is never not &sc->sa, so let's just drop the parameter.
There's no bug here, but let's get rid of some stupid.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/repair.h            |    5 ++---
 fs/xfs/scrub/alloc_repair.c      |    2 +-
 fs/xfs/scrub/bmap_repair.c       |    2 +-
 fs/xfs/scrub/cow_repair.c        |    2 +-
 fs/xfs/scrub/ialloc_repair.c     |    2 +-
 fs/xfs/scrub/refcount_repair.c   |    2 +-
 fs/xfs/scrub/repair.c            |   10 +++++-----
 fs/xfs/scrub/rmap_repair.c       |    4 ++--
 fs/xfs/scrub/rtrefcount_repair.c |    2 +-
 fs/xfs/scrub/rtrmap_repair.c     |    2 +-
 10 files changed, 16 insertions(+), 17 deletions(-)


diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index 1af568651e20a1..a32fd9fb59bdae 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -100,9 +100,8 @@ int xrep_setup_ag_allocbt(struct xfs_scrub *sc);
 struct xfs_imap;
 int xrep_setup_inode(struct xfs_scrub *sc, const struct xfs_imap *imap);
 
-void xrep_ag_btcur_init(struct xfs_scrub *sc, struct xchk_ag *sa);
-int xrep_ag_init(struct xfs_scrub *sc, struct xfs_perag *pag,
-		struct xchk_ag *sa);
+void xrep_ag_btcur_init(struct xfs_scrub *sc);
+int xrep_ag_init(struct xfs_scrub *sc, struct xfs_perag *pag);
 #ifdef CONFIG_XFS_RT
 int xrep_rtgroup_init(struct xfs_scrub *sc, struct xfs_rtgroup *rtg,
 		struct xchk_rt *sr, unsigned int rtglock_flags);
diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 05f27c927adfb9..854cfbbd9ecf4f 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -370,7 +370,7 @@ xrep_abt_find_freespace(
 
 	xagb_bitmap_init(&ra->not_allocbt_blocks);
 
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	/*
 	 * Iterate all the reverse mappings to find gaps in the physical
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index eabffba47776e9..7288940b74e6c1 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -359,7 +359,7 @@ xrep_bmap_scan_ag(
 	struct xfs_scrub	*sc = rb->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index e49d9396edc527..3d4e94757de040 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -280,7 +280,7 @@ xrep_cow_find_bad(
 	if (!pag)
 		return -EFSCORRUPTED;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		goto out_pag;
 
diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
index 43846b3bbf4ed4..f0f2232a9686d8 100644
--- a/fs/xfs/scrub/ialloc_repair.c
+++ b/fs/xfs/scrub/ialloc_repair.c
@@ -473,7 +473,7 @@ xrep_ibt_find_inodes(
 	ri->rie.ir_startino = NULLAGINO;
 
 	/* Collect all reverse mappings for inode blocks. */
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 	error = xfs_rmap_query_all(sc->sa.rmap_cur, xrep_ibt_walk_rmap, ri);
 	xchk_ag_btcur_free(&sc->sa);
 	if (error)
diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
index ca9c382005ff1d..aa29d7a27abaa8 100644
--- a/fs/xfs/scrub/refcount_repair.c
+++ b/fs/xfs/scrub/refcount_repair.c
@@ -411,7 +411,7 @@ xrep_refc_find_refcounts(
 	bool			have;
 	int			error;
 
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	/*
 	 * Set up a bag to store all the rmap records that we're tracking to
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 956aa75218aa24..19c582223efe42 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -888,10 +888,10 @@ xrep_ino_ensure_extent_count(
  */
 void
 xrep_ag_btcur_init(
-	struct xfs_scrub	*sc,
-	struct xchk_ag		*sa)
+	struct xfs_scrub	*sc)
 {
 	struct xfs_mount	*mp = sc->mp;
+	struct xchk_ag		*sa = &sc->sa;
 
 	/* Set up a bnobt cursor for cross-referencing. */
 	if (sc->sm->sm_type != XFS_SCRUB_TYPE_BNOBT &&
@@ -990,9 +990,9 @@ xrep_reinit_pagi(
 int
 xrep_ag_init(
 	struct xfs_scrub	*sc,
-	struct xfs_perag	*pag,
-	struct xchk_ag		*sa)
+	struct xfs_perag	*pag)
 {
+	struct xchk_ag		*sa = &sc->sa;
 	int			error;
 
 	ASSERT(!sa->pag);
@@ -1007,7 +1007,7 @@ xrep_ag_init(
 
 	/* Grab our own passive reference from the caller's ref. */
 	sa->pag = xfs_perag_hold(pag);
-	xrep_ag_btcur_init(sc, sa);
+	xrep_ag_btcur_init(sc);
 	return 0;
 }
 
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index 725035bf490329..aac9e534df798f 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -910,7 +910,7 @@ xrep_rmap_find_rmaps(
 	int			error;
 
 	/* Find all the per-AG metadata. */
-	xrep_ag_btcur_init(sc, &sc->sa);
+	xrep_ag_btcur_init(sc);
 
 	error = xrep_rmap_find_inode_rmaps(rr);
 	if (error)
@@ -1167,7 +1167,7 @@ xrep_rmap_reserve_space(
 	} while (!done);
 
 	/* Emit rmaps for everything in the free space bitmap. */
-	xrep_ag_btcur_init(rr->sc, &rr->sc->sa);
+	xrep_ag_btcur_init(rr->sc);
 	error = xrep_rmap_stash_bitmap(rr, &freesp_blocks, &XFS_RMAP_OINFO_AG);
 	xchk_ag_btcur_free(&rr->sc->sa);
 
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index 2b939960c7ddcd..b307ea15197090 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -420,7 +420,7 @@ xrep_rtrefc_scan_ag(
 	struct xfs_scrub	*sc = rr->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index a2b72e61edf5fe..6b435acc2d69ff 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -413,7 +413,7 @@ xrep_rtrmap_scan_ag(
 	struct xfs_scrub	*sc = rr->sc;
 	int			error;
 
-	error = xrep_ag_init(sc, pag, &sc->sa);
+	error = xrep_ag_init(sc, pag);
 	if (error)
 		return error;