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

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

[Darrick J. Wong]

Hi all,

Here's a twelveth 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-12
---
Commits in this patchset:
 * xfs: fix under-reservation of blocks when repairing sf directories
 * xfs: actually check internal-rtdev fields in the superblock
 * xfs: fix rtrmap cross-referencing elision logic
 * xfs: fix termination logic in xchk_bmap
 * xfs: fix replaying dirent removals into the temporary directory
 * xfs: reset parent pointer args before each dir tree unlink repair
 * xfs: advance the findparent inode scan cursor while holding ILOCK
---
 fs/xfs/scrub/agheader.c       |    7 +++++
 fs/xfs/scrub/attr_repair.c    |    2 +-
 fs/xfs/scrub/bmap.c           |    7 +++--
 fs/xfs/scrub/dir_repair.c     |    4 ++-
 fs/xfs/scrub/dirtree_repair.c |    1 +
 fs/xfs/scrub/findparent.c     |   54 +++++++++++++++++++++++++++--------------
 fs/xfs/scrub/tempfile.c       |   28 ++++++++++++++++-----
 7 files changed, 72 insertions(+), 31 deletions(-)

[PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Darrick J. Wong]

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

Whilst running QA on XFS for-next as of 7.3-rc2 with MKFS_OPTIONS="-n
size=8192", I observed the following (trimmed) dmesg splat:

 XFS: Assertion failed: args->total >= dp->i_nblocks - nblks, file: fs/xfs/libxfs/xfs_da_btree.c, line: 2387
 WARNING: fs/xfs/xfs_message.c:104 at assfail+0x46/0x4a [xfs], CPU#0: xfs_scrub/1426511
 CPU: 0 UID: 0 PID: 1426511 Comm: xfs_scrub Tainted: G        W           7.3.0-rc2-djwx #rc2 PREEMPT(lazy)  6e418570b606a39783b0e7e7b30dc407b965f9e8
 Tainted: [W]=WARN
 RIP: 0010:assfail+0x46/0x4a [xfs]
 RSP: 0018:ffffc900010d7890 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000ffffffd1
 RDX: 0000000000000000 RSI: 0000000000000021 RDI: ffffffffa059fd38
 RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
 R10: 000000000000000a R11: 000000007fffffff R12: ffffc900010d7940
 R13: ffff888368d8f980 R14: ffffc900010d7a48 R15: ffffc900010d78d0
 FS:  00007f445c5ce680(0000) GS:ffff8884a97ea000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f443803b9a8 CR3: 0000000107a4b000 CR4: 00000000003506f0
 Call Trace:
  <TASK>
  xfs_da_grow_inode_int+0x2e0/0x300 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_dir2_grow_inode+0x6e/0x150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_dir2_sf_to_block+0x149/0x870 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_swap_prep+0xe2/0x110 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_swap+0xfb/0x2f0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_rebuild_tree+0x99/0x100 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_directory+0x83/0x1c0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_attempt+0x4f/0x1e0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_scrub_metadata+0x393/0x5b0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_ioc_scrubv_metadata+0x306/0x570 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_file_ioctl+0xa4f/0x1150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  __x64_sys_ioctl+0x76/0xc0
  do_syscall_64+0x7a/0x3b0
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

This is a consequence of commit 0fe77e57588b98, which added the
following assertion to xfs_da_grow_inode_int:

 ASSERT(args->total >= dp->i_nblocks - nblks);

Tracing this back to xrep_dir_swap_prep, I noticed that the xfs_da_args
object that's passed to xfs_dir2_sf_to_block sets args->total to 1.
This is incorrect because mkfs set the directory block size to 8k and
the filesystem block size to 4k.  In other words, args->total should be
2 here, not 1.

Dave Chinner tripped over the same problem with the same branch through
a different channel -- his test setup set the fs block size to 1k, in
which case the directory block size is still set to 4k.  Here,
args->total should be 4.

Changing the assignment of args->total to sc->mp->m_dir_geo->fsbcount
makes the assertion go away, but that isn't a complete fix.  In
xrep_tempexch_estimate, we also incorrectly assume that a shortform
conversion requires 1 fsblock when it should be m_dir_geo->fsbcount.
Without that, we can under-reserve space in the transaction and cause a
filesystem shutdown.

For completeness, we fix xrep_xattr_swap_prep to assign args->total from
m_attr_geo->fsbcount instead of the magic value 1.

Cc: <stable@vger.kernel.org> # v6.10
Cc: floss@jetm.me
Reported-by: dgc@kernel.org
Fixes: 629fdaf5f5b1b7 ("xfs: use atomic extent swapping to fix user file fork data")
Tripped-by: 0fe77e57588b98 ("xfs: assert the reservation covers each da fork growth")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/attr_repair.c |    2 +-
 fs/xfs/scrub/dir_repair.c  |    2 +-
 fs/xfs/scrub/tempfile.c    |   28 +++++++++++++++++++++-------
 3 files changed, 23 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index 6e6af142f1fb8e..a1fa8ad5402ea7 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -1294,7 +1294,7 @@ xrep_xattr_swap_prep(
 			.geo		= sc->mp->m_attr_geo,
 			.whichfork	= XFS_ATTR_FORK,
 			.trans		= sc->tp,
-			.total		= 1,
+			.total		= sc->mp->m_attr_geo->fsbcount,
 			.owner		= I_INO(sc->ip),
 		};
 
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 31a23c5f386ae6..0a3d02d8f04153 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1488,7 +1488,7 @@ xrep_dir_swap_prep(
 			.geo		= sc->mp->m_dir_geo,
 			.whichfork	= XFS_DATA_FORK,
 			.trans		= sc->tp,
-			.total		= 1,
+			.total		= sc->mp->m_dir_geo->fsbcount,
 			.owner		= I_INO(sc->ip),
 		};
 
diff --git a/fs/xfs/scrub/tempfile.c b/fs/xfs/scrub/tempfile.c
index 98820003b9298f..f244411fa9e531 100644
--- a/fs/xfs/scrub/tempfile.c
+++ b/fs/xfs/scrub/tempfile.c
@@ -649,6 +649,18 @@ xrep_tempexch_prep_request(
 	return 0;
 }
 
+static inline unsigned int
+xrep_tempexch_estimate_sf_resblks(
+	struct xfs_scrub	*sc,
+	int			whichfork)
+{
+	if (whichfork == XFS_ATTR_FORK)
+		return sc->mp->m_attr_geo->fsbcount;
+	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
+		return sc->mp->m_dir_geo->fsbcount;
+	return 1;
+}
+
 /*
  * Fill out the mapping exchange resource estimation structures in preparation
  * for exchanging the contents of a metadata file that we've rebuilt in the
@@ -663,6 +675,8 @@ xrep_tempexch_estimate(
 	struct xfs_ifork	*ifp;
 	struct xfs_ifork	*tifp;
 	int			whichfork = xfs_exchmaps_reqfork(req);
+	unsigned int		sf_resblks =
+		xrep_tempexch_estimate_sf_resblks(sc, whichfork);
 	int			state = 0;
 
 	/*
@@ -693,9 +707,9 @@ xrep_tempexch_estimate(
 		 * plus the block we converted.
 		 */
 		req->ip1_bcount = sc->tempip->i_nblocks;
-		req->ip2_bcount = 1;
+		req->ip2_bcount = sf_resblks;
 		req->nr_exchanges = 1 + tifp->if_nextents;
-		req->resblks = 1;
+		req->resblks = sf_resblks;
 		break;
 	case 2:
 		/*
@@ -707,10 +721,10 @@ xrep_tempexch_estimate(
 		 * is (worst case) the extent count of the file being repaired
 		 * plus the block we converted.
 		 */
-		req->ip1_bcount = 1;
+		req->ip1_bcount = sf_resblks;
 		req->ip2_bcount = sc->ip->i_nblocks;
 		req->nr_exchanges = 1 + ifp->if_nextents;
-		req->resblks = 1;
+		req->resblks = sf_resblks;
 		break;
 	case 3:
 		/*
@@ -722,10 +736,10 @@ xrep_tempexch_estimate(
 		 * fileoff 0.  Presumably, the caller could not exchange the
 		 * two inode fork areas directly.
 		 */
-		req->ip1_bcount = 1;
-		req->ip2_bcount = 1;
+		req->ip1_bcount = sf_resblks;
+		req->ip2_bcount = sf_resblks;
 		req->nr_exchanges = 1;
-		req->resblks = 2;
+		req->resblks = 2 * sf_resblks;
 		break;
 	}
 

[PATCH 2/7] xfs: actually check internal-rtdev fields in the superblock

[Darrick J. Wong]

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

LOLLM points out that the superblock scrubber doesn't check the new
fields that were added for internal realtime volumes when we added zoned
device support.

Cc: <stable@vger.kernel.org> # v6.15
Fixes: 2167eaabe2fadd ("xfs: define the zoned on-disk format")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/agheader.c |    7 +++++++
 1 file changed, 7 insertions(+)


diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 1fa66aa68e169f..fa5d32ec020aec 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -418,6 +418,13 @@ xchk_superblock(
 			xchk_block_set_corrupt(sc, bp);
 	}
 
+	if (xfs_has_zoned(mp)) {
+		if (sb->sb_rtstart != cpu_to_be64(mp->m_sb.sb_rtstart))
+			xchk_block_set_corrupt(sc, bp);
+		if (sb->sb_rtreserved != cpu_to_be64(mp->m_sb.sb_rtreserved))
+			xchk_block_set_corrupt(sc, bp);
+	}
+
 	/* Everything else must be zero. */
 	sblen = xchk_superblock_ondisk_size(mp);
 	if (memchr_inv((char *)sb + sblen, 0, BBTOB(bp->b_length) - sblen))

[PATCH 3/7] xfs: fix rtrmap cross-referencing elision logic

[Darrick J. Wong]

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

LOLLM points out that xchk_bmap_xref_rmap_cow skips the cross-reference
if the data-section rmapbt cursor is not present.  However, this is
broken for realtime file data fork scanning, because they will have an
rtrmapbt cursor and not an rmapbt cursor.  Fix the behavior by removing
the cursor checks because xchk_bmap_get_rmap already accounts for that.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 037a44d8277adf ("xfs: cross-reference the realtime rmapbt")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/bmap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 401c278725d216..3b0f1dbd9147ac 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -274,7 +274,7 @@ xchk_bmap_xref_rmap_cow(
 	unsigned long long	rmap_end;
 	uint64_t		owner = XFS_RMAP_OWN_COW;
 
-	if (!info->sc->sa.rmap_cur || xchk_skip_xref(info->sc->sm))
+	if (xchk_skip_xref(info->sc->sm))
 		return;
 
 	/* Find the rmap record for this irec. */

[PATCH 4/7] xfs: fix termination logic in xchk_bmap

[Darrick J. Wong]

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

xchk_should_terminate can turn its @error argument into -EINTR if the
user is sitting on ^C.  Unfortunately, this code here turns that into a
0 return, which isn't quite correct.  LOLLM complains about this, though
I think it's a very minor matter because the only way -EINTR happens is
if there's a fatal signal.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/bmap.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 3b0f1dbd9147ac..4f3c7f681bd921 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -1103,8 +1103,9 @@ xchk_bmap(
 	 * the rmap must match the combined mapping exactly.
 	 */
 	while (xchk_bmap_iext_iter(&info, &irec)) {
-		if (xchk_should_terminate(sc, &error) ||
-		    (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT))
+		if (xchk_should_terminate(sc, &error))
+			return error;
+		if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 			return 0;
 
 		if (irec.br_startoff >= endoff) {

[PATCH 5/7] xfs: fix replaying dirent removals into the temporary directory

[Darrick J. Wong]

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

xrep_dir_replay_removename is the function that replays a directory
entry removal from sc->ip into the temporary directory so that when we
swap the contents of sc->tempip and sc->ip, the directory is correct.
LOLLM noticed that we were passing the wrong inode pointer into
xrep_dir_init_args.  It doesn't make sense to set rd->args.dp to
rd->args.dp so let's fix this.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 8559b21a64d983 ("xfs: implement live updates for directory repairs")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 0a3d02d8f04153..82ec5cfad63c59 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -727,7 +727,7 @@ xrep_dir_replay_removename(
 	const struct xfs_name	*name,
 	xfs_extlen_t		total)
 {
-	struct xfs_inode	*dp = rd->args.dp;
+	struct xfs_inode	*dp = rd->sc->tempip;
 
 	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 

[PATCH 6/7] xfs: reset parent pointer args before each dir tree unlink repair

[Darrick J. Wong]

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

LOLLM noticed that xfs_parent_removename only partially initializes the
passed-in parent pointer arguments object.  In the directory tree repair
code, we could decide to remove multiple links to a file, so we don't
want state from one call to bleed into the next one.  Zero the whole
thing explicitly.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 3f31406aef493b ("xfs: fix corruptions in the directory tree")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dirtree_repair.c |    1 +
 1 file changed, 1 insertion(+)


diff --git a/fs/xfs/scrub/dirtree_repair.c b/fs/xfs/scrub/dirtree_repair.c
index 8acd55b8c7696d..1d1eafcf6eb598 100644
--- a/fs/xfs/scrub/dirtree_repair.c
+++ b/fs/xfs/scrub/dirtree_repair.c
@@ -479,6 +479,7 @@ xrep_dirtree_unlink(
 	}
 
 	if (xfs_has_parent(sc->mp)) {
+		memset(&dl->ppargs, 0, sizeof(dl->ppargs));
 		error = xfs_parent_removename(sc->tp, &dl->ppargs, dp,
 				&dl->xname, sc->ip);
 		if (error)

[PATCH 7/7] xfs: advance the findparent inode scan cursor while holding ILOCK

[Darrick J. Wong]

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

LOLLM pointed out a race condition in xrep_findparent_scan -- the
directory live update hook holds the directory ILOCK when it calls the
xchk_iscan_want_live_update predicate to figure out if it needs to
remember the live update, but xrep_findparent_scan drops the directory
ILOCK before advancing the cursor.  Therefore, it's possible for a live
update to check the scan cursor after the scan drops the ILOCK but
before the scan updates its cursor.  If this happens, we'll fail to
record the live update.  Fix this by moving the cursor update logic
inside xrep_findparent_walk_directory.

Note that for non-directories it's ok to advance the cursor without
holding any ILOCK because the findparent scan only cares about directory
parents, not the children.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: a07b45576264e7 ("xfs: scan the filesystem to repair a directory dotdot entry")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/findparent.c |   54 ++++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 18 deletions(-)


diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c
index 04b6b96b0a30ed..eab3ac2704befe 100644
--- a/fs/xfs/scrub/findparent.c
+++ b/fs/xfs/scrub/findparent.c
@@ -139,32 +139,52 @@ xrep_findparent_dirent(
 	return 0;
 }
 
-/*
- * If this is a directory, walk the dirents looking for any that point to the
- * scrub target inode.
- */
-STATIC int
-xrep_findparent_walk_directory(
-	struct xrep_findparent_info	*fpi)
+static inline bool
+xrep_findparent_want_scan_file(
+	const struct xrep_findparent_info	*fpi)
 {
-	struct xfs_scrub		*sc = fpi->sc;
-	struct xfs_inode		*dp = fpi->dp;
-	unsigned int			lock_mode;
-	int				error = 0;
+	const struct xfs_scrub			*sc = fpi->sc;
+	const struct xfs_inode			*dp = fpi->dp;
+
+	/* Only directories can be parents */
+	if (!S_ISDIR(VFS_IC(dp)->i_mode))
+		return false;
 
 	/*
 	 * The inode being scanned cannot be its own parent, nor can any
 	 * temporary directory we created to stage this repair.
 	 */
 	if (dp == sc->ip || dp == sc->tempip)
-		return 0;
+		return false;
 
 	/*
 	 * Similarly, temporary files created to stage a repair cannot be the
 	 * parent of this inode.
 	 */
 	if (xrep_is_tempfile(dp))
+		return false;
+
+	return true;
+}
+
+/*
+ * If this is a directory, walk the dirents looking for any that point to the
+ * scrub target inode.
+ */
+STATIC int
+xrep_findparent_walk_file(
+	struct xrep_findparent_info	*fpi)
+{
+	struct xfs_scrub		*sc = fpi->sc;
+	struct xfs_inode		*dp = fpi->dp;
+	unsigned int			lock_mode;
+	int				error = 0;
+
+	if (!xrep_findparent_want_scan_file(fpi)) {
+		if (fpi->parent_scan)
+			xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
 		return 0;
+	}
 
 	/*
 	 * Scan the directory to see if there it contains an entry pointing to
@@ -201,6 +221,8 @@ xrep_findparent_walk_directory(
 		goto out_unlock;
 
 out_unlock:
+	if (fpi->parent_scan)
+		xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
 	xfs_iunlock(dp, lock_mode);
 	return error;
 }
@@ -308,11 +330,7 @@ xrep_findparent_scan(
 	ASSERT(S_ISDIR(VFS_IC(sc->ip)->i_mode));
 
 	while ((ret = xchk_iscan_iter(&pscan->iscan, &fpi.dp)) == 1) {
-		if (S_ISDIR(VFS_I(fpi.dp)->i_mode))
-			ret = xrep_findparent_walk_directory(&fpi);
-		else
-			ret = 0;
-		xchk_iscan_mark_visited(&pscan->iscan, fpi.dp);
+		ret = xrep_findparent_walk_file(&fpi);
 		xchk_irele(sc, fpi.dp);
 		if (ret)
 			break;
@@ -401,7 +419,7 @@ xrep_findparent_confirm(
 		goto out_rele;
 	}
 
-	error = xrep_findparent_walk_directory(&fpi);
+	error = xrep_findparent_walk_file(&fpi);
 	if (error)
 		goto out_rele;
 

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Christoph Hellwig]

On Tue, Sep 08, 2026 at 11:04:35PM -0700, Darrick J. Wong wrote:
> +static inline unsigned int
> +xrep_tempexch_estimate_sf_resblks(
> +	struct xfs_scrub	*sc,
> +	int			whichfork)
> +{
> +	if (whichfork == XFS_ATTR_FORK)
> +		return sc->mp->m_attr_geo->fsbcount;
> +	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
> +		return sc->mp->m_dir_geo->fsbcount;
> +	return 1;

geo->fsbcount for attrs is a always 1 and a lot of code relies on that,
so this feels a bit overcomplicated and misleading (but still correct).

What about the more usual:

	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
		return sc->mp->m_dir_geo->fsbcount;
	return 1;

?

Otherwise looks good:

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

Re: [PATCH 3/7] xfs: fix rtrmap cross-referencing elision logic

[Christoph Hellwig]

On Tue, Sep 08, 2026 at 11:05:06PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that xchk_bmap_xref_rmap_cow skips the cross-reference
> if the data-section rmapbt cursor is not present.  However, this is
> broken for realtime file data fork scanning, because they will have an
> rtrmapbt cursor and not an rmapbt cursor.  Fix the behavior by removing
> the cursor checks because xchk_bmap_get_rmap already accounts for that.

Looks good:

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

Re: [PATCH 4/7] xfs: fix termination logic in xchk_bmap

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 5/7] xfs: fix replaying dirent removals into the temporary directory

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 6/7] xfs: reset parent pointer args before each dir tree unlink repair

[Christoph Hellwig]

On Tue, Sep 08, 2026 at 11:05:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xfs_parent_removename only partially initializes the
> passed-in parent pointer arguments object.  In the directory tree repair
> code, we could decide to remove multiple links to a file, so we don't
> want state from one call to bleed into the next one.  Zero the whole
> thing explicitly.

Looks good:

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

Re: [PATCH 7/7] xfs: advance the findparent inode scan cursor while holding ILOCK

[Christoph Hellwig]

> +static inline bool
> +xrep_findparent_want_scan_file(
> +	const struct xrep_findparent_info	*fpi)
>  {
> -	struct xfs_scrub		*sc = fpi->sc;
> -	struct xfs_inode		*dp = fpi->dp;
> -	unsigned int			lock_mode;
> -	int				error = 0;
> +	const struct xfs_scrub			*sc = fpi->sc;
> +	const struct xfs_inode			*dp = fpi->dp;
> +
> +	/* Only directories can be parents */
> +	if (!S_ISDIR(VFS_IC(dp)->i_mode))
> +		return false;

Not very inclusive :)

The patch looks good:


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

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Carlos Maiolino]

On Thu, Sep 10, 2026 at 07:00:31AM +0200, Christoph Hellwig wrote:
> On Tue, Sep 08, 2026 at 11:04:35PM -0700, Darrick J. Wong wrote:
> > +static inline unsigned int
> > +xrep_tempexch_estimate_sf_resblks(
> > +	struct xfs_scrub	*sc,
> > +	int			whichfork)
> > +{
> > +	if (whichfork == XFS_ATTR_FORK)
> > +		return sc->mp->m_attr_geo->fsbcount;
> > +	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
> > +		return sc->mp->m_dir_geo->fsbcount;
> > +	return 1;
> 
> geo->fsbcount for attrs is a always 1 and a lot of code relies on that,
> so this feels a bit overcomplicated and misleading (but still correct).
> 
> What about the more usual:
> 
> 	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
> 		return sc->mp->m_dir_geo->fsbcount;
> 	return 1;

Either are fine to me so feel free to add:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> ?
> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 

Re: [PATCH 2/7] xfs: actually check internal-rtdev fields in the superblock

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:04:50PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that the superblock scrubber doesn't check the new
> fields that were added for internal realtime volumes when we added zoned
> device support.
> 
> Cc: <stable@vger.kernel.org> # v6.15
> Fixes: 2167eaabe2fadd ("xfs: define the zoned on-disk format")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs

This looks fine

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/scrub/agheader.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index 1fa66aa68e169f..fa5d32ec020aec 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -418,6 +418,13 @@ xchk_superblock(
>  			xchk_block_set_corrupt(sc, bp);
>  	}
>  
> +	if (xfs_has_zoned(mp)) {
> +		if (sb->sb_rtstart != cpu_to_be64(mp->m_sb.sb_rtstart))
> +			xchk_block_set_corrupt(sc, bp);
> +		if (sb->sb_rtreserved != cpu_to_be64(mp->m_sb.sb_rtreserved))
> +			xchk_block_set_corrupt(sc, bp);
> +	}
> +
>  	/* Everything else must be zero. */
>  	sblen = xchk_superblock_ondisk_size(mp);
>  	if (memchr_inv((char *)sb + sblen, 0, BBTOB(bp->b_length) - sblen))
> 
> 

Re: [PATCH 3/7] xfs: fix rtrmap cross-referencing elision logic

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:05:06PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that xchk_bmap_xref_rmap_cow skips the cross-reference
> if the data-section rmapbt cursor is not present.  However, this is
> broken for realtime file data fork scanning, because they will have an
> rtrmapbt cursor and not an rmapbt cursor.  Fix the behavior by removing
> the cursor checks because xchk_bmap_get_rmap already accounts for that.
> 
> Cc: <stable@vger.kernel.org> # v6.14
> Fixes: 037a44d8277adf ("xfs: cross-reference the realtime rmapbt")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  fs/xfs/scrub/bmap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index 401c278725d216..3b0f1dbd9147ac 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -274,7 +274,7 @@ xchk_bmap_xref_rmap_cow(
>  	unsigned long long	rmap_end;
>  	uint64_t		owner = XFS_RMAP_OWN_COW;
>  
> -	if (!info->sc->sa.rmap_cur || xchk_skip_xref(info->sc->sm))
> +	if (xchk_skip_xref(info->sc->sm))
>  		return;
>  
>  	/* Find the rmap record for this irec. */
> 
> 

Re: [PATCH 4/7] xfs: fix termination logic in xchk_bmap

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:05:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xchk_should_terminate can turn its @error argument into -EINTR if the
> user is sitting on ^C.  Unfortunately, this code here turns that into a
> 0 return, which isn't quite correct.  LOLLM complains about this, though
> I think it's a very minor matter because the only way -EINTR happens is
> if there's a fatal signal.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  fs/xfs/scrub/bmap.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index 3b0f1dbd9147ac..4f3c7f681bd921 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -1103,8 +1103,9 @@ xchk_bmap(
>  	 * the rmap must match the combined mapping exactly.
>  	 */
>  	while (xchk_bmap_iext_iter(&info, &irec)) {
> -		if (xchk_should_terminate(sc, &error) ||
> -		    (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT))
> +		if (xchk_should_terminate(sc, &error))
> +			return error;
> +		if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
>  			return 0;
>  
>  		if (irec.br_startoff >= endoff) {
> 
> 

Re: [PATCH 5/7] xfs: fix replaying dirent removals into the temporary directory

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:05:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xrep_dir_replay_removename is the function that replays a directory
> entry removal from sc->ip into the temporary directory so that when we
> swap the contents of sc->tempip and sc->ip, the directory is correct.
> LOLLM noticed that we were passing the wrong inode pointer into
> xrep_dir_init_args.  It doesn't make sense to set rd->args.dp to
> rd->args.dp so let's fix this.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 8559b21a64d983 ("xfs: implement live updates for directory repairs")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/scrub/dir_repair.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> index 0a3d02d8f04153..82ec5cfad63c59 100644
> --- a/fs/xfs/scrub/dir_repair.c
> +++ b/fs/xfs/scrub/dir_repair.c
> @@ -727,7 +727,7 @@ xrep_dir_replay_removename(
>  	const struct xfs_name	*name,
>  	xfs_extlen_t		total)
>  {
> -	struct xfs_inode	*dp = rd->args.dp;
> +	struct xfs_inode	*dp = rd->sc->tempip;
>  
>  	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
>  
> 
> 

Re: [PATCH 6/7] xfs: reset parent pointer args before each dir tree unlink repair

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:05:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xfs_parent_removename only partially initializes the
> passed-in parent pointer arguments object.  In the directory tree repair
> code, we could decide to remove multiple links to a file, so we don't
> want state from one call to bleed into the next one.  Zero the whole
> thing explicitly.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 3f31406aef493b ("xfs: fix corruptions in the directory tree")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  fs/xfs/scrub/dirtree_repair.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> 
> diff --git a/fs/xfs/scrub/dirtree_repair.c b/fs/xfs/scrub/dirtree_repair.c
> index 8acd55b8c7696d..1d1eafcf6eb598 100644
> --- a/fs/xfs/scrub/dirtree_repair.c
> +++ b/fs/xfs/scrub/dirtree_repair.c
> @@ -479,6 +479,7 @@ xrep_dirtree_unlink(
>  	}
>  
>  	if (xfs_has_parent(sc->mp)) {
> +		memset(&dl->ppargs, 0, sizeof(dl->ppargs));
>  		error = xfs_parent_removename(sc->tp, &dl->ppargs, dp,
>  				&dl->xname, sc->ip);
>  		if (error)
> 
> 

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Darrick J. Wong]

On Thu, Sep 10, 2026 at 07:31:37AM +0200, Carlos Maiolino wrote:
> On Thu, Sep 10, 2026 at 07:00:31AM +0200, Christoph Hellwig wrote:
> > On Tue, Sep 08, 2026 at 11:04:35PM -0700, Darrick J. Wong wrote:
> > > +static inline unsigned int
> > > +xrep_tempexch_estimate_sf_resblks(
> > > +	struct xfs_scrub	*sc,
> > > +	int			whichfork)
> > > +{
> > > +	if (whichfork == XFS_ATTR_FORK)
> > > +		return sc->mp->m_attr_geo->fsbcount;
> > > +	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
> > > +		return sc->mp->m_dir_geo->fsbcount;
> > > +	return 1;
> > 
> > geo->fsbcount for attrs is a always 1 and a lot of code relies on that,
> > so this feels a bit overcomplicated and misleading (but still correct).
> > 
> > What about the more usual:
> > 
> > 	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
> > 		return sc->mp->m_dir_geo->fsbcount;
> > 	return 1;

I prefer not to leave a logic bomb in the scrub code in case we ever
have multi-fsblock attr blocks.

Though I guess I'd be ok with

	ASSERT(mp->m_attr_geo->fsbcount == 1);

	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
		return sc->mp->m_dir_geo->fsbcount;
	return 1;

or something like that?

> Either are fine to me so feel free to add:
> 
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

But only if Carlos really wants me to add that.

--D

> 
> > 
> > ?
> > 
> > Otherwise looks good:
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Carlos Maiolino]

On Wed, Sep 09, 2026 at 11:09:07PM -0700, Darrick J. Wong wrote:
> On Thu, Sep 10, 2026 at 07:31:37AM +0200, Carlos Maiolino wrote:
> > On Thu, Sep 10, 2026 at 07:00:31AM +0200, Christoph Hellwig wrote:
> > > On Tue, Sep 08, 2026 at 11:04:35PM -0700, Darrick J. Wong wrote:
> > > > +static inline unsigned int
> > > > +xrep_tempexch_estimate_sf_resblks(
> > > > +	struct xfs_scrub	*sc,
> > > > +	int			whichfork)
> > > > +{
> > > > +	if (whichfork == XFS_ATTR_FORK)
> > > > +		return sc->mp->m_attr_geo->fsbcount;
> > > > +	if (S_ISDIR(VFS_I(sc->ip)->i_mode))
> > > > +		return sc->mp->m_dir_geo->fsbcount;
> > > > +	return 1;
> > > 
> > > geo->fsbcount for attrs is a always 1 and a lot of code relies on that,
> > > so this feels a bit overcomplicated and misleading (but still correct).
> > > 
> > > What about the more usual:
> > > 
> > > 	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
> > > 		return sc->mp->m_dir_geo->fsbcount;
> > > 	return 1;
> 
> I prefer not to leave a logic bomb in the scrub code in case we ever
> have multi-fsblock attr blocks.
> 
> Though I guess I'd be ok with
> 
> 	ASSERT(mp->m_attr_geo->fsbcount == 1);
> 
> 	if (S_ISDIR(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
> 		return sc->mp->m_dir_geo->fsbcount;
> 	return 1;
> 
> or something like that?

That sounds good to me too.

Between all three I prefer the first explicit, self-documenting one, but
I have no strong feelings for any.

I agree about not leaving any magic numbers unchecked though.

> 
> > Either are fine to me so feel free to add:
> > 
> > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> But only if Carlos really wants me to add that.
> 
> --D
> 
> > 
> > > 
> > > ?
> > > 
> > > Otherwise looks good:
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> 

Re: [PATCH 7/7] xfs: advance the findparent inode scan cursor while holding ILOCK

[Carlos Maiolino]

On Tue, Sep 08, 2026 at 11:06:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM pointed out a race condition in xrep_findparent_scan -- the
> directory live update hook holds the directory ILOCK when it calls the
> xchk_iscan_want_live_update predicate to figure out if it needs to
> remember the live update, but xrep_findparent_scan drops the directory
> ILOCK before advancing the cursor.  Therefore, it's possible for a live
> update to check the scan cursor after the scan drops the ILOCK but
> before the scan updates its cursor.  If this happens, we'll fail to
> record the live update.  Fix this by moving the cursor update logic
> inside xrep_findparent_walk_directory.
> 
> Note that for non-directories it's ok to advance the cursor without
> holding any ILOCK because the findparent scan only cares about directory
> parents, not the children.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: a07b45576264e7 ("xfs: scan the filesystem to repair a directory dotdot entry")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


>  fs/xfs/scrub/findparent.c |   54 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 36 insertions(+), 18 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c
> index 04b6b96b0a30ed..eab3ac2704befe 100644
> --- a/fs/xfs/scrub/findparent.c
> +++ b/fs/xfs/scrub/findparent.c
> @@ -139,32 +139,52 @@ xrep_findparent_dirent(
>  	return 0;
>  }
>  
> -/*
> - * If this is a directory, walk the dirents looking for any that point to the
> - * scrub target inode.
> - */
> -STATIC int
> -xrep_findparent_walk_directory(
> -	struct xrep_findparent_info	*fpi)
> +static inline bool
> +xrep_findparent_want_scan_file(
> +	const struct xrep_findparent_info	*fpi)
>  {
> -	struct xfs_scrub		*sc = fpi->sc;
> -	struct xfs_inode		*dp = fpi->dp;
> -	unsigned int			lock_mode;
> -	int				error = 0;
> +	const struct xfs_scrub			*sc = fpi->sc;
> +	const struct xfs_inode			*dp = fpi->dp;
> +
> +	/* Only directories can be parents */
> +	if (!S_ISDIR(VFS_IC(dp)->i_mode))
> +		return false;
>  
>  	/*
>  	 * The inode being scanned cannot be its own parent, nor can any
>  	 * temporary directory we created to stage this repair.
>  	 */
>  	if (dp == sc->ip || dp == sc->tempip)
> -		return 0;
> +		return false;
>  
>  	/*
>  	 * Similarly, temporary files created to stage a repair cannot be the
>  	 * parent of this inode.
>  	 */
>  	if (xrep_is_tempfile(dp))
> +		return false;
> +
> +	return true;
> +}
> +
> +/*
> + * If this is a directory, walk the dirents looking for any that point to the
> + * scrub target inode.
> + */
> +STATIC int
> +xrep_findparent_walk_file(
> +	struct xrep_findparent_info	*fpi)
> +{
> +	struct xfs_scrub		*sc = fpi->sc;
> +	struct xfs_inode		*dp = fpi->dp;
> +	unsigned int			lock_mode;
> +	int				error = 0;
> +
> +	if (!xrep_findparent_want_scan_file(fpi)) {
> +		if (fpi->parent_scan)
> +			xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
>  		return 0;
> +	}
>  
>  	/*
>  	 * Scan the directory to see if there it contains an entry pointing to
> @@ -201,6 +221,8 @@ xrep_findparent_walk_directory(
>  		goto out_unlock;
>  
>  out_unlock:
> +	if (fpi->parent_scan)
> +		xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
>  	xfs_iunlock(dp, lock_mode);
>  	return error;
>  }
> @@ -308,11 +330,7 @@ xrep_findparent_scan(
>  	ASSERT(S_ISDIR(VFS_IC(sc->ip)->i_mode));
>  
>  	while ((ret = xchk_iscan_iter(&pscan->iscan, &fpi.dp)) == 1) {
> -		if (S_ISDIR(VFS_I(fpi.dp)->i_mode))
> -			ret = xrep_findparent_walk_directory(&fpi);
> -		else
> -			ret = 0;
> -		xchk_iscan_mark_visited(&pscan->iscan, fpi.dp);
> +		ret = xrep_findparent_walk_file(&fpi);
>  		xchk_irele(sc, fpi.dp);
>  		if (ret)
>  			break;
> @@ -401,7 +419,7 @@ xrep_findparent_confirm(
>  		goto out_rele;
>  	}
>  
> -	error = xrep_findparent_walk_directory(&fpi);
> +	error = xrep_findparent_walk_file(&fpi);
>  	if (error)
>  		goto out_rele;
>  
> 
> 

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Christoph Hellwig]

On Wed, Sep 09, 2026 at 11:09:07PM -0700, Darrick J. Wong wrote:
> I prefer not to leave a logic bomb in the scrub code in case we ever
> have multi-fsblock attr blocks.

There's plenty of logic bombs to unwind in that case :)

Best if we want to prepare for that would be to use xfs_dabuf_nfsb
and make sure we use a helper like that everywhere.

Re: [PATCH 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Darrick J. Wong]

On Thu, Sep 10, 2026 at 11:43:44AM +0200, Christoph Hellwig wrote:
> On Wed, Sep 09, 2026 at 11:09:07PM -0700, Darrick J. Wong wrote:
> > I prefer not to leave a logic bomb in the scrub code in case we ever
> > have multi-fsblock attr blocks.
> 
> There's plenty of logic bombs to unwind in that case :)
> 
> Best if we want to prepare for that would be to use xfs_dabuf_nfsb
> and make sure we use a helper like that everywhere.

Actually, I like that a lot better.  Let me rework the patch with that.

--D

[PATCH v1.1 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Darrick J. Wong]

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

Whilst running QA on XFS for-next as of 7.3-rc2 with MKFS_OPTIONS="-n
size=8192", I observed the following (trimmed) dmesg splat:

 XFS: Assertion failed: args->total >= dp->i_nblocks - nblks, file: fs/xfs/libxfs/xfs_da_btree.c, line: 2387
 WARNING: fs/xfs/xfs_message.c:104 at assfail+0x46/0x4a [xfs], CPU#0: xfs_scrub/1426511
 CPU: 0 UID: 0 PID: 1426511 Comm: xfs_scrub Tainted: G        W           7.3.0-rc2-djwx #rc2 PREEMPT(lazy)  6e418570b606a39783b0e7e7b30dc407b965f9e8
 Tainted: [W]=WARN
 RIP: 0010:assfail+0x46/0x4a [xfs]
 RSP: 0018:ffffc900010d7890 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000ffffffd1
 RDX: 0000000000000000 RSI: 0000000000000021 RDI: ffffffffa059fd38
 RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
 R10: 000000000000000a R11: 000000007fffffff R12: ffffc900010d7940
 R13: ffff888368d8f980 R14: ffffc900010d7a48 R15: ffffc900010d78d0
 FS:  00007f445c5ce680(0000) GS:ffff8884a97ea000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f443803b9a8 CR3: 0000000107a4b000 CR4: 00000000003506f0
 Call Trace:
  <TASK>
  xfs_da_grow_inode_int+0x2e0/0x300 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_dir2_grow_inode+0x6e/0x150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_dir2_sf_to_block+0x149/0x870 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_swap_prep+0xe2/0x110 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_swap+0xfb/0x2f0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_dir_rebuild_tree+0x99/0x100 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_directory+0x83/0x1c0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xrep_attempt+0x4f/0x1e0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_scrub_metadata+0x393/0x5b0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_ioc_scrubv_metadata+0x306/0x570 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  xfs_file_ioctl+0xa4f/0x1150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
  __x64_sys_ioctl+0x76/0xc0
  do_syscall_64+0x7a/0x3b0
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

This is a consequence of commit 0fe77e57588b98, which added the
following assertion to xfs_da_grow_inode_int:

 ASSERT(args->total >= dp->i_nblocks - nblks);

Tracing this back to xrep_dir_swap_prep, I noticed that the xfs_da_args
object that's passed to xfs_dir2_sf_to_block sets args->total to 1.
This is incorrect because mkfs set the directory block size to 8k and
the filesystem block size to 4k.  In other words, args->total should be
2 here, not 1.

Dave Chinner tripped over the same problem with the same branch through
a different channel -- his test setup set the fs block size to 1k, in
which case the directory block size is still set to 4k.  Here,
args->total should be 4.

Changing the assignment of args->total to sc->mp->m_dir_geo->fsbcount
makes the assertion go away, but that isn't a complete fix.  In
xrep_tempexch_estimate, we also incorrectly assume that a shortform
conversion requires 1 fsblock when it should be m_dir_geo->fsbcount.
Without that, we can under-reserve space in the transaction and cause a
filesystem shutdown.

Note that the xfs_dabuf_nfsb helper will compute the correct value for
directories and xattr, so we use that instead of open-coding the logic.
Also fix xrep_xattr_swap_prep to assign args->total via xfs_dabuf_nfsb
to avoid one logic bomb if we ever support multi-fsblock attrs.

Cc: <stable@vger.kernel.org> # v6.10
Cc: floss@jetm.me
Reported-by: dgc@kernel.org
Fixes: 629fdaf5f5b1b7 ("xfs: use atomic extent swapping to fix user file fork data")
Tripped-by: 0fe77e57588b98 ("xfs: assert the reservation covers each da fork growth")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
v1.1: use xfs_dabuf_nfsb instead of opencoding the logic
---
 fs/xfs/libxfs/xfs_da_btree.h |    2 ++
 fs/xfs/libxfs/xfs_da_btree.c |    2 +-
 fs/xfs/scrub/attr_repair.c   |    2 +-
 fs/xfs/scrub/dir_repair.c    |    2 +-
 fs/xfs/scrub/tempfile.c      |   29 ++++++++++++++++++++++-------
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index afcf2d3c7a21c0..a718b1ceb0aab7 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -244,4 +244,6 @@ xfs_failaddr_t xfs_da3_node_header_check(struct xfs_buf *bp, xfs_ino_t owner);
 
 extern struct kmem_cache	*xfs_da_state_cache;
 
+int xfs_dabuf_nfsb(struct xfs_mount *mp, int whichfork);
+
 #endif	/* __XFS_DA_BTREE_H__ */
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 7938d2324e8769..9955b084c94b9f 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -130,7 +130,7 @@ xfs_da_state_reset(
 	state->mp = state->args->dp->i_mount;
 }
 
-static inline int xfs_dabuf_nfsb(struct xfs_mount *mp, int whichfork)
+inline int xfs_dabuf_nfsb(struct xfs_mount *mp, int whichfork)
 {
 	if (whichfork == XFS_DATA_FORK)
 		return mp->m_dir_geo->fsbcount;
diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index 6e6af142f1fb8e..28f92e9ba72b24 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -1294,7 +1294,7 @@ xrep_xattr_swap_prep(
 			.geo		= sc->mp->m_attr_geo,
 			.whichfork	= XFS_ATTR_FORK,
 			.trans		= sc->tp,
-			.total		= 1,
+			.total		= xfs_dabuf_nfsb(sc->mp, XFS_ATTR_FORK),
 			.owner		= I_INO(sc->ip),
 		};
 
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 31a23c5f386ae6..d9d6b7e2abda72 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1488,7 +1488,7 @@ xrep_dir_swap_prep(
 			.geo		= sc->mp->m_dir_geo,
 			.whichfork	= XFS_DATA_FORK,
 			.trans		= sc->tp,
-			.total		= 1,
+			.total		= xfs_dabuf_nfsb(sc->mp, XFS_DATA_FORK),
 			.owner		= I_INO(sc->ip),
 		};
 
diff --git a/fs/xfs/scrub/tempfile.c b/fs/xfs/scrub/tempfile.c
index 98820003b9298f..59a9213a3c7ddf 100644
--- a/fs/xfs/scrub/tempfile.c
+++ b/fs/xfs/scrub/tempfile.c
@@ -649,6 +649,19 @@ xrep_tempexch_prep_request(
 	return 0;
 }
 
+static inline unsigned int
+xrep_tempexch_estimate_sf_resblks(
+	struct xfs_scrub	*sc,
+	int			whichfork)
+{
+	/* repairing a symlink target */
+	if (S_ISLNK(VFS_I(sc->ip)->i_mode) && whichfork == XFS_DATA_FORK)
+		return 1;
+
+	/* everything else is a directory or an xattr structure */
+	return xfs_dabuf_nfsb(sc->mp, whichfork);
+}
+
 /*
  * Fill out the mapping exchange resource estimation structures in preparation
  * for exchanging the contents of a metadata file that we've rebuilt in the
@@ -663,6 +676,8 @@ xrep_tempexch_estimate(
 	struct xfs_ifork	*ifp;
 	struct xfs_ifork	*tifp;
 	int			whichfork = xfs_exchmaps_reqfork(req);
+	unsigned int		sf_resblks =
+		xrep_tempexch_estimate_sf_resblks(sc, whichfork);
 	int			state = 0;
 
 	/*
@@ -693,9 +708,9 @@ xrep_tempexch_estimate(
 		 * plus the block we converted.
 		 */
 		req->ip1_bcount = sc->tempip->i_nblocks;
-		req->ip2_bcount = 1;
+		req->ip2_bcount = sf_resblks;
 		req->nr_exchanges = 1 + tifp->if_nextents;
-		req->resblks = 1;
+		req->resblks = sf_resblks;
 		break;
 	case 2:
 		/*
@@ -707,10 +722,10 @@ xrep_tempexch_estimate(
 		 * is (worst case) the extent count of the file being repaired
 		 * plus the block we converted.
 		 */
-		req->ip1_bcount = 1;
+		req->ip1_bcount = sf_resblks;
 		req->ip2_bcount = sc->ip->i_nblocks;
 		req->nr_exchanges = 1 + ifp->if_nextents;
-		req->resblks = 1;
+		req->resblks = sf_resblks;
 		break;
 	case 3:
 		/*
@@ -722,10 +737,10 @@ xrep_tempexch_estimate(
 		 * fileoff 0.  Presumably, the caller could not exchange the
 		 * two inode fork areas directly.
 		 */
-		req->ip1_bcount = 1;
-		req->ip2_bcount = 1;
+		req->ip1_bcount = sf_resblks;
+		req->ip2_bcount = sf_resblks;
 		req->nr_exchanges = 1;
-		req->resblks = 2;
+		req->resblks = 2 * sf_resblks;
 		break;
 	}
 

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

[Carlos Maiolino]

On Tue, 08 Sep 2026 23:02:56 -0700, Darrick J. Wong wrote:
> Here's a twelveth 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.
> 
> [...]

Applied to for-next, thanks!

[1/7] xfs: fix under-reservation of blocks when repairing sf directories
      commit: 4d3c07591534517c633945c8d8e6526f10e3fabc
[1/7] xfs: fix under-reservation of blocks when repairing sf directories
      commit: 4d3c07591534517c633945c8d8e6526f10e3fabc
[3/7] xfs: fix rtrmap cross-referencing elision logic
      commit: 3bdbf472a608aeb7e8e4dc70ee86738ad5256356
[4/7] xfs: fix termination logic in xchk_bmap
      commit: d3a6a35a220615c4f4578aedf3b1626b91d3acae
[5/7] xfs: fix replaying dirent removals into the temporary directory
      commit: e854f9a28b1fa08dfa5bf18ee4184fae90106180
[6/7] xfs: reset parent pointer args before each dir tree unlink repair
      commit: 69e10c2b4a51b4ff3c88a70e90f5180ad58c758f
[7/7] xfs: advance the findparent inode scan cursor while holding ILOCK
      commit: ad4497a92caba4630f75c80d49cb947026213280

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH v1.1 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Carlos Maiolino]

On Thu, 10 Sep 2026 21:42:28 -0700, Darrick J. Wong wrote:
> Whilst running QA on XFS for-next as of 7.3-rc2 with MKFS_OPTIONS="-n
> size=8192", I observed the following (trimmed) dmesg splat:
> 
>  XFS: Assertion failed: args->total >= dp->i_nblocks - nblks, file: fs/xfs/libxfs/xfs_da_btree.c, line: 2387
>  WARNING: fs/xfs/xfs_message.c:104 at assfail+0x46/0x4a [xfs], CPU#0: xfs_scrub/1426511
>  CPU: 0 UID: 0 PID: 1426511 Comm: xfs_scrub Tainted: G        W           7.3.0-rc2-djwx #rc2 PREEMPT(lazy)  6e418570b606a39783b0e7e7b30dc407b965f9e8
>  Tainted: [W]=WARN
>  RIP: 0010:assfail+0x46/0x4a [xfs]
>  RSP: 0018:ffffc900010d7890 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000ffffffd1
>  RDX: 0000000000000000 RSI: 0000000000000021 RDI: ffffffffa059fd38
>  RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
>  R10: 000000000000000a R11: 000000007fffffff R12: ffffc900010d7940
>  R13: ffff888368d8f980 R14: ffffc900010d7a48 R15: ffffc900010d78d0
>  FS:  00007f445c5ce680(0000) GS:ffff8884a97ea000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f443803b9a8 CR3: 0000000107a4b000 CR4: 00000000003506f0
>  Call Trace:
>   <TASK>
>   xfs_da_grow_inode_int+0x2e0/0x300 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xfs_dir2_grow_inode+0x6e/0x150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xfs_dir2_sf_to_block+0x149/0x870 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xrep_dir_swap_prep+0xe2/0x110 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xrep_dir_swap+0xfb/0x2f0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xrep_dir_rebuild_tree+0x99/0x100 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xrep_directory+0x83/0x1c0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xrep_attempt+0x4f/0x1e0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xfs_scrub_metadata+0x393/0x5b0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xfs_ioc_scrubv_metadata+0x306/0x570 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   xfs_file_ioctl+0xa4f/0x1150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c]
>   __x64_sys_ioctl+0x76/0xc0
>   do_syscall_64+0x7a/0x3b0
>   entry_SYSCALL_64_after_hwframe+0x4b/0x53
> 
> [...]

Applied to for-next, thanks!

[1/7] xfs: fix under-reservation of blocks when repairing sf directories
      commit: 4d3c07591534517c633945c8d8e6526f10e3fabc

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH 2/7] xfs: actually check internal-rtdev fields in the superblock

[Carlos Maiolino]

On Tue, 08 Sep 2026 23:04:50 -0700, Darrick J. Wong wrote:
> LOLLM points out that the superblock scrubber doesn't check the new
> fields that were added for internal realtime volumes when we added zoned
> device support.
> 
> 

Applied to for-next, thanks!

[2/7] xfs: actually check internal-rtdev fields in the superblock
      commit: 1ee2ce797c360785a3813fef62c90f427f3aed34

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH v1.1 1/7] xfs: fix under-reservation of blocks when repairing sf directories

[Christoph Hellwig]

Looks good:

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