Skip to contentSkip to conversation

FAILED: patch "[PATCH] xfs: fix under-reservation of blocks when repairing sf" failed to apply to 6.18-stable tree

PATCH New [email protected] 2 participants 5 messages Sep 16 – Sep 17

Subsystems (2)
Stable branch XFS filesystem
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.18-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.18.y
git checkout FETCH_HEAD
git cherry-pick -x 4d3c07591534517c633945c8d8e6526f10e3fabc
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026091628-approval-prefix-a4c6@gregkh' --subject-prefix 'PATCH 6.18.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 4d3c07591534517c633945c8d8e6526f10e3fabc Mon Sep 17 00:00:00 2001
From: "Darrick J. Wong" <[email protected]>
Date: Thu, 10 Sep 2026 21:42:28 -0700
Subject: [PATCH] xfs: fix under-reservation of blocks when repairing sf
directories

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 [email protected] # v6.10
Cc [email protected]
Reported-by [email protected]
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 <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Signed-off-by Carlos Maiolino <[email protected]>
fs/xfs/libxfs/xfs_da_btree.c +1 -1
index 8cbdd6574755..3d02a0d7ba44 100644
@@ -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;
fs/xfs/libxfs/xfs_da_btree.h +2 -0
index afcf2d3c7a21..a718b1ceb0aa 100644
@@ -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__ */
fs/xfs/scrub/attr_repair.c +1 -1
index 6e6af142f1fb..28f92e9ba72b 100644
@@ -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),
 		};
fs/xfs/scrub/dir_repair.c +1 -1
index 31a23c5f386a..d9d6b7e2abda 100644
@@ -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),
 		};
fs/xfs/scrub/tempfile.c +22 -7
index 98820003b929..59a9213a3c7d 100644
@@ -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;
 	}
maintainer
#1 lore raw

PATCH 1/4 xfs: add a xfs_rmap_inode_owner helper

From: Christoph Hellwig <[email protected]>

[ Upstream commit f882fc7dd9f04f73d94379deb9e68d8db613c976 ]

Add a small wrapper for initializing the rmap owner to i_ino.

Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Reviewed-by "Darrick J. Wong" <[email protected]>
Signed-off-by Carlos Maiolino <[email protected]>

Stable-dep-of: 4d3c07591534 ("xfs: fix under-reservation of blocks when repairing sf directories")

Signed-off-by Sasha Levin <[email protected]>
---
 fs/xfs/libxfs/xfs_bmap.c | 2 +-
 fs/xfs/libxfs/xfs_rmap.h | 2 ++
 fs/xfs/scrub/bmap.c      | 6 +++---
 fs/xfs/scrub/reap.c      | 3 +--
 4 files changed, 7 insertions(+), 6 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +1 -1
index 53ef4b7e504d6..bdffade9848f8 100644
@@ -820,7 +820,7 @@ xfs_bmap_local_to_extents(
 	args.mp = ip->i_mount;
 	args.total = total;
 	args.minlen = args.maxlen = args.prod = 1;
-	xfs_rmap_ino_owner(&args.oinfo, ip->i_ino, whichfork, 0);
+	xfs_rmap_inode_owner(&args.oinfo, ip, whichfork, 0);
 
 	/*
 	 * Allocate a block.  We know we need only one, since the
fs/xfs/libxfs/xfs_rmap.h +2 -0
index 5f39f6e53cd19..3670a33fc927d 100644
@@ -35,6 +35,8 @@ xfs_rmap_ino_owner(
 	if (whichfork == XFS_ATTR_FORK)
 		oi->oi_flags |= XFS_OWNER_INFO_ATTR_FORK;
 }
+#define xfs_rmap_inode_owner(oi, ip, whichfork, offset) \
+	xfs_rmap_ino_owner(oi, (ip)->i_ino, whichfork, offset)
 
 static inline bool
 xfs_rmap_should_skip_owner_update(
fs/xfs/scrub/bmap.c +3 -3
index 845e1538077e3..910a1f42622a8 100644
@@ -352,7 +352,7 @@ xchk_bmap_rt_iextent_xref(
 	case XFS_DATA_FORK:
 		xchk_bmap_xref_rmap(info, irec, rgbno);
 		if (!xfs_is_reflink_inode(info->sc->ip)) {
-			xfs_rmap_ino_owner(&oinfo, info->sc->ip->i_ino,
+			xfs_rmap_inode_owner(&oinfo, info->sc->ip,
 					info->whichfork, irec->br_startoff);
 			xchk_xref_is_only_rt_owned_by(info->sc, rgbno,
 					irec->br_blockcount, &oinfo);

@@ -407,7 +407,7 @@ xchk_bmap_iextent_xref(
 	case XFS_DATA_FORK:
 		xchk_bmap_xref_rmap(info, irec, agbno);
 		if (!xfs_is_reflink_inode(info->sc->ip)) {
-			xfs_rmap_ino_owner(&oinfo, info->sc->ip->i_ino,
+			xfs_rmap_inode_owner(&oinfo, info->sc->ip,
 					info->whichfork, irec->br_startoff);
 			xchk_xref_is_only_owned_by(info->sc, agbno,
 					irec->br_blockcount, &oinfo);

@@ -419,7 +419,7 @@ xchk_bmap_iextent_xref(
 		break;
 	case XFS_ATTR_FORK:
 		xchk_bmap_xref_rmap(info, irec, agbno);
-		xfs_rmap_ino_owner(&oinfo, info->sc->ip->i_ino,
+		xfs_rmap_inode_owner(&oinfo, info->sc->ip,
 				info->whichfork, irec->br_startoff);
 		xchk_xref_is_only_owned_by(info->sc, agbno, irec->br_blockcount,
 				&oinfo);
fs/xfs/scrub/reap.c +1 -2
index 07f5bb8a64212..ab0c133ffb98b 100644
@@ -1249,8 +1249,7 @@ xreap_bmapi_select(
 	cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, sc->sa.agf_bp,
 			sc->sa.pag);
 
-	xfs_rmap_ino_owner(&oinfo, rs->ip->i_ino, rs->whichfork,
-			imap->br_startoff);
+	xfs_rmap_inode_owner(&oinfo, rs->ip, rs->whichfork, imap->br_startoff);
 	error = xfs_rmap_has_other_keys(cur, agbno, 1, &oinfo, crosslinked);
 	if (error)
 		goto out_cur;
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/4 xfs: convert xchk_inode_xref_set_corrupt to xchk_ip_xref_set_corrupt

From: Christoph Hellwig <[email protected]>

[ Upstream commit bef4cee25fbc26ee8b07028e461190fa57b02788 ]

All xref corruption reports have the xfs_inode structure, so switch the helper to work based on that.

Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Reviewed-by "Darrick J. Wong" <[email protected]>
Signed-off-by Carlos Maiolino <[email protected]>

Stable-dep-of: 4d3c07591534 ("xfs: fix under-reservation of blocks when repairing sf directories")

Signed-off-by Sasha Levin <[email protected]>
---
 fs/xfs/scrub/common.c    | 6 +++---
 fs/xfs/scrub/common.h    | 4 ++--
 fs/xfs/scrub/dirtree.c   | 4 ++--
 fs/xfs/scrub/inode.c     | 6 +++---
 fs/xfs/scrub/nlinks.c    | 2 +-
 fs/xfs/scrub/rtbitmap.c  | 4 ++--
 fs/xfs/scrub/rtsummary.c | 2 +-
 7 files changed, 14 insertions(+), 14 deletions(-)
fs/xfs/scrub/common.c +3 -3
index 58f2ed19ef5ac..ecbf965ad6897 100644
@@ -309,12 +309,12 @@ xchk_ino_set_corrupt(
 
 /* Record a corruption while cross-referencing with an inode. */
 void
-xchk_ino_xref_set_corrupt(
+xchk_ip_xref_set_corrupt(
 	struct xfs_scrub	*sc,
-	xfs_ino_t		ino)
+	struct xfs_inode	*ip)
 {
 	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_XCORRUPT;
-	trace_xchk_ino_error(sc, ino, __return_address);
+	trace_xchk_ino_error(sc, ip->i_ino, __return_address);
 }
 
 /* Record corruption in a block indexed by a file fork. */
fs/xfs/scrub/common.h +2 -2
index ae998501f8aa9..5f854d3c7e8e1 100644
@@ -41,8 +41,8 @@ void xchk_qcheck_set_corrupt(struct xfs_scrub *sc, unsigned int dqtype,
 
 void xchk_block_xref_set_corrupt(struct xfs_scrub *sc,
 		struct xfs_buf *bp);
-void xchk_ino_xref_set_corrupt(struct xfs_scrub *sc,
-		xfs_ino_t ino);
+void xchk_ip_xref_set_corrupt(struct xfs_scrub *sc,
+		struct xfs_inode *ip);
 void xchk_fblock_xref_set_corrupt(struct xfs_scrub *sc,
 		int whichfork, xfs_fileoff_t offset);
fs/xfs/scrub/dirtree.c +2 -2
index e202fbc22b013..543a7aecfc7bb 100644
@@ -954,7 +954,7 @@ xchk_dirtree(
 		 * parent pointers are corrupt; this scan cannot be completed
 		 * without full information.
 		 */
-		xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
+		xchk_ip_xref_set_corrupt(sc, sc->ip);
 		error = 0;
 		goto out_scanlock;
 	}

@@ -984,7 +984,7 @@ xchk_dirtree(
 		if (oc.bad || oc.good + oc.suspect != 1)
 			xchk_ip_set_corrupt(sc, sc->ip);
 		if (oc.suspect)
-			xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
+			xchk_ip_xref_set_corrupt(sc, sc->ip);
 	}
 
 out_scanlock:
fs/xfs/scrub/inode.c +3 -3
index bb3f475b63532..0d30d65f336a3 100644
@@ -710,17 +710,17 @@ xchk_inode_xref_bmap(
 	if (!xchk_should_check_xref(sc, &error, NULL))
 		return;
 	if (nextents < xfs_dfork_data_extents(dip))
-		xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
+		xchk_ip_xref_set_corrupt(sc, sc->ip);
 
 	error = xchk_inode_count_blocks(sc, XFS_ATTR_FORK, &nextents, &acount);
 	if (!xchk_should_check_xref(sc, &error, NULL))
 		return;
 	if (nextents != xfs_dfork_attr_extents(dip))
-		xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
+		xchk_ip_xref_set_corrupt(sc, sc->ip);
 
 	/* Check nblocks against the inode. */
 	if (count + acount != be64_to_cpu(dip->di_nblocks))
-		xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
+		xchk_ip_xref_set_corrupt(sc, sc->ip);
 }
 
 /* Cross-reference with the other btrees. */
fs/xfs/scrub/nlinks.c +1 -1
index 988262fc99533..d783b32a0b281 100644
@@ -741,7 +741,7 @@ xchk_nlinks_compare_inode(
 		 * number of subdirectory entries in the directory.
 		 */
 		if (obs.children != obs.backrefs)
-			xchk_ino_xref_set_corrupt(sc, ip->i_ino);
+			xchk_ip_xref_set_corrupt(sc, ip);
 	} else {
 		/*
 		 * Non-directories and unlinked directories should not have
fs/xfs/scrub/rtbitmap.c +2 -2
index 68b8338707608..639e1328c199d 100644
@@ -284,7 +284,7 @@ xchk_xref_is_used_rt_space(
 	if (xfs_has_zoned(sc->mp)) {
 		if (!xfs_zone_rgbno_is_valid(rtg,
 				xfs_rtb_to_rgbno(sc->mp, rtbno) + len - 1))
-			xchk_ino_xref_set_corrupt(sc, rtg_rmap(rtg)->i_ino);
+			xchk_ip_xref_set_corrupt(sc, rtg_rmap(rtg));
 		return;
 	}
 

@@ -295,5 +295,5 @@ xchk_xref_is_used_rt_space(
 	if (!xchk_should_check_xref(sc, &error, NULL))
 		return;
 	if (is_free)
-		xchk_ino_xref_set_corrupt(sc, rtg_bitmap(rtg)->i_ino);
+		xchk_ip_xref_set_corrupt(sc, rtg_bitmap(rtg));
 }
fs/xfs/scrub/rtsummary.c +1 -1
index dbbbafe8b92d6..a1e2aa580bc1b 100644
@@ -190,7 +190,7 @@ xchk_rtsum_record_free(
 	rtlen = xfs_rtxlen_to_extlen(mp, rec->ar_extcount);
 
 	if (!xfs_verify_rtbext(mp, rtbno, rtlen)) {
-		xchk_ino_xref_set_corrupt(sc, rtg_bitmap(rtg)->i_ino);
+		xchk_ip_xref_set_corrupt(sc, rtg_bitmap(rtg));
 		return -EFSCORRUPTED;
 	}
Signature · 1 line
2.53.0
maintainer
#3 lore raw

PATCH 3/4 xfs: remove the i_ino field in struct xfs_inode

In reply to Sasha Levin

From: Christoph Hellwig <[email protected]>

[ Upstream commit 1113a6d6d5d1336f4415fa1367aac0f853f0892d ]

Now that the VFS inode has a u64 i_ino field, there is no need to store a copy of the inode number in the xfs_inode structure.

Introduce an I_INO() wrapper as a shortcut to the inode number so that we don't have to propagate the VFS inode everywhere.

The only non-obvious part is the clearing of i_ino to 0 for RCU freeing the inode. None of this calls into VFS paths, which makes clearing the VFS inode field here just as safe as clearing the old field in the xfs_inode.

Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Reviewed-by "Darrick J. Wong" <[email protected]>
Signed-off-by Carlos Maiolino <[email protected]>

Stable adaptation for dependency of 4d3c07591534517c633945c8d8e6526f10e3fabc:

The stable VFS inode still has an unsigned long i_ino, so retain the 64-bit xfs_inode::i_ino and its existing initialization/reclaim handling. Define I_INO as a macro over that field instead of adding a function. Keep only the owner conversions in xrep_xattr_swap_prep and xrep_dir_swap_prep, which provide the context needed for the target to apply unchanged. Drop the remaining conversions, including changes to helpers and health monitoring code absent from this stable tree.

Also finish the preceding dependency's helper conversion in the stable xchk_rtsummary error path: call xchk_ip_xref_set_corrupt(sc, rbmip) instead of the removed xchk_ino_xref_set_corrupt. This preserves its cross-reference corruption reporting and fixes the allmodconfig build.

[ sashal: Reduced backport -- upstream 1113a6d6d5d13 touches 91 file(s), this
backport carries 4. Not backported here:
fs/xfs/libxfs/xfs_attr.c
fs/xfs/libxfs/xfs_attr_leaf.c
fs/xfs/libxfs/xfs_bmap_btree.c
fs/xfs/libxfs/xfs_bmap.c
fs/xfs/libxfs/xfs_btree.c
fs/xfs/libxfs/xfs_btree_staging.c
fs/xfs/libxfs/xfs_da_btree.c
fs/xfs/libxfs/xfs_dir2.c
... and 80 more
This note is generated from the file lists only; see the resolution record for the reasoning. ]

Stable-dep-of: 4d3c07591534 ("xfs: fix under-reservation of blocks when repairing sf directories")

Signed-off-by Sasha Levin <[email protected]>
---
 fs/xfs/scrub/attr_repair.c | 2 +-
 fs/xfs/scrub/dir_repair.c  | 2 +-
 fs/xfs/scrub/rtsummary.c   | 2 +-
 fs/xfs/xfs_inode.h         | 3 +++
 4 files changed, 6 insertions(+), 3 deletions(-)
fs/xfs/scrub/attr_repair.c +1 -1
index fa68ecabd983f..8dd88d775376a 100644
@@ -1295,7 +1295,7 @@ xrep_xattr_swap_prep(
 			.whichfork	= XFS_ATTR_FORK,
 			.trans		= sc->tp,
 			.total		= 1,
-			.owner		= sc->ip->i_ino,
+			.owner		= I_INO(sc->ip),
 		};
 
 		error = xfs_attr_shortform_to_leaf(&args);
fs/xfs/scrub/dir_repair.c +1 -1
index e0fa5e6ca1fe8..5375f677bc7a9 100644
@@ -1468,7 +1468,7 @@ xrep_dir_swap_prep(
 			.whichfork	= XFS_DATA_FORK,
 			.trans		= sc->tp,
 			.total		= 1,
-			.owner		= sc->ip->i_ino,
+			.owner		= I_INO(sc->ip),
 		};
 
 		error = xfs_dir2_sf_to_block(&args);
fs/xfs/scrub/rtsummary.c +1 -1
index a1e2aa580bc1b..8fb06ce4a2003 100644
@@ -359,7 +359,7 @@ xchk_rtsummary(
 		 * EFSCORRUPTED means the rtbitmap is corrupt, which is an xref
 		 * error since we're checking the summary file.
 		 */
-		xchk_ino_xref_set_corrupt(sc, rbmip->i_ino);
+		xchk_ip_xref_set_corrupt(sc, rbmip);
 		return 0;
 	}
 	if (error)
fs/xfs/xfs_inode.h +3 -0
index bd6d335571943..11598ee7c1792 100644
@@ -184,6 +184,9 @@ static inline const struct inode *VFS_IC(const struct xfs_inode *ip)
 	return &ip->i_vnode;
 }
 
+/* The VFS inode number is only unsigned long on this stable branch. */
+#define I_INO(ip)	((ip)->i_ino)
+
 /*
  * For regular files we only update the on-disk filesize when actually
  * writing data back to disk.  Until then only the copy in the VFS inode
Signature · 1 line
2.53.0
maintainer
#4 lore raw

PATCH 4/4 xfs: fix under-reservation of blocks when repairing sf directories

In reply to Sasha Levin

From: "Darrick J. Wong" <[email protected]>

[ Upstream commit 4d3c07591534517c633945c8d8e6526f10e3fabc ]

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 [email protected] # v6.10
Cc [email protected]
Reported-by [email protected]
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 <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Signed-off-by Carlos Maiolino <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
 fs/xfs/libxfs/xfs_da_btree.c |  2 +-
 fs/xfs/libxfs/xfs_da_btree.h |  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(-)
fs/xfs/libxfs/xfs_da_btree.c +1 -1
index 90f7fc219fccc..90fbdef23def1 100644
@@ -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;
fs/xfs/libxfs/xfs_da_btree.h +2 -0
index 354d5d65043e4..cd2e213f5e74f 100644
@@ -242,4 +242,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__ */
fs/xfs/scrub/attr_repair.c +1 -1
index 8dd88d775376a..6f668ddea8197 100644
@@ -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),
 		};
fs/xfs/scrub/dir_repair.c +1 -1
index 5375f677bc7a9..f3e1e71b2ddba 100644
@@ -1467,7 +1467,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),
 		};
fs/xfs/scrub/tempfile.c +22 -7
index fabf6ae2e5120..d9741a566fc6c 100644
@@ -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;
 	}
Signature · 1 line
2.53.0