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

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

[Darrick J. Wong]

Hi all,

Here's a sixth 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-6
---
Commits in this patchset:
 * xfs: fix xfs_rtrmapbt_mem_cursor for non-rmap filesystems
 * xfs: preserve owner on in-memory btree creation
 * xfs: don't leak new_bp if xfs_btree_bload_drop_buf fails
 * xfs: actually recover intended file sizes in xfs_xmi_item_recover_intent
 * xfs: don't modify file attributes or poke fsnotify for dry runs
 * xfs: fix bnobt repair space reservation disposal failure
---
 fs/xfs/libxfs/xfs_btree_mem.c     |    2 ++
 fs/xfs/libxfs/xfs_btree_staging.c |    4 +++-
 fs/xfs/libxfs/xfs_rtrmap_btree.c  |    2 +-
 fs/xfs/scrub/alloc_repair.c       |   18 ++++++++++++------
 fs/xfs/xfs_exchmaps_item.c        |   19 ++++++++++++++++++-
 fs/xfs/xfs_exchrange.c            |    6 ++++++
 6 files changed, 42 insertions(+), 9 deletions(-)

[PATCH 1/6] xfs: fix xfs_rtrmapbt_mem_cursor for non-rmap filesystems

[Darrick J. Wong]

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

It's possible to construct an in-memory rtrmap btree for filesystems
that don't have the rmap feature enabled.  The kernel doesn't do this,
but xfs_repair will, if asked to reindex a filesystem that has rtreflink
enabled but not rtrmap.  Therefore, we must create the cursor with
enough levels to handle a maximally sized btree possible.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 4a61f12eb11958 ("xfs: create a shadow rmap btree during realtime rmap repair")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_rtrmap_btree.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index 0cb2113d5b4045..a15e460a1ec7f4 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -618,7 +618,7 @@ xfs_rtrmapbt_mem_cursor(
 	struct xfs_btree_cur	*cur;
 
 	cur = xfs_btree_alloc_cursor(mp, tp, &xfs_rtrmapbt_mem_ops,
-			mp->m_rtrmap_maxlevels, xfs_rtrmapbt_cur_cache);
+			xfs_rtrmapbt_maxlevels_ondisk(), xfs_rtrmapbt_cur_cache);
 	cur->bc_mem.xfbtree = xfbt;
 	cur->bc_nlevels = xfbt->nlevels;
 	cur->bc_group = xfs_group_hold(rtg_group(rtg));

[PATCH 2/6] xfs: preserve owner on in-memory btree creation

[Darrick J. Wong]

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

LOLLM points out a minor bug where a higher level function creating an
in-memory btree is required to pass in an owner number, but the creation
function erases that.  In-memory btrees are ephemeral so this really
doesn't matter except for debugging.  But let's fix this papercut.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: a095686a238352 ("xfs: support in-memory btrees")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_btree_mem.c |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_btree_mem.c b/fs/xfs/libxfs/xfs_btree_mem.c
index 37136a70e56d66..1d83a4251ceeb6 100644
--- a/fs/xfs/libxfs/xfs_btree_mem.c
+++ b/fs/xfs/libxfs/xfs_btree_mem.c
@@ -117,6 +117,7 @@ xfbtree_init(
 	struct xfs_buftarg		*btp,
 	const struct xfs_btree_ops	*ops)
 {
+	unsigned long long		owner = xfbt->owner;
 	unsigned int			blocklen = xfbtree_rec_bytes(mp, ops);
 	unsigned int			keyptr_len;
 	int				error;
@@ -133,6 +134,7 @@ xfbtree_init(
 
 	memset(xfbt, 0, sizeof(*xfbt));
 	xfbt->target = btp;
+	xfbt->owner = owner;
 
 	/* Set up min/maxrecs for this btree. */
 	keyptr_len = ops->key_len + sizeof(__be64);

[PATCH 3/6] xfs: don't leak new_bp if xfs_btree_bload_drop_buf fails

[Darrick J. Wong]

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

LOLLM observes that in xfs_btree_bload_prep_block,
xfs_btree_bload_drop_buf can hit an IO error if writing the delwri
buffer list to disk fails.  In this case, we fail to release new_bp,
which means we lose a locked buffer.  Fix that.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: e069d549705e49 ("xfs: constrain dirty buffers while formatting a staged btree")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_btree_staging.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_btree_staging.c b/fs/xfs/libxfs/xfs_btree_staging.c
index c3c7ea54895a60..c5b7613e5216ab 100644
--- a/fs/xfs/libxfs/xfs_btree_staging.c
+++ b/fs/xfs/libxfs/xfs_btree_staging.c
@@ -337,8 +337,10 @@ xfs_btree_bload_prep_block(
 		xfs_btree_set_sibling(cur, *blockp, &new_ptr, XFS_BB_RIGHTSIB);
 
 	ret = xfs_btree_bload_drop_buf(bbl, buffers_list, bpp);
-	if (ret)
+	if (ret) {
+		xfs_buf_relse(new_bp);
 		return ret;
+	}
 
 	/* Initialize the new btree block. */
 	xfs_btree_init_block_cur(cur, new_bp, level, nr_this_block);

[PATCH 4/6] xfs: actually recover intended file sizes in xfs_xmi_item_recover_intent

[Darrick J. Wong]

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

LOLLM points out that xfs_xmi_item_recover_intent doesn't actually
restore the isize1 and isize2 fields that were recovered from an
unfinished exchmaps log intent item.  Instead, xfs_exchmaps_init_intent
sets the wrong isize values from the recovered inodes, with the result
that the file sizes are not set correctly when item recovery finishes.
Fix this by restoring isize[12] from the log item.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_exchmaps_item.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
index c3745d33e54ea2..dd5d92ca1010fe 100644
--- a/fs/xfs/xfs_exchmaps_item.c
+++ b/fs/xfs/xfs_exchmaps_item.c
@@ -344,7 +344,17 @@ xfs_xmi_validate(
 	if (!xfs_verify_fileext(mp, xlf->xmi_startoff1, xlf->xmi_blockcount))
 		return false;
 
-	return xfs_verify_fileext(mp, xlf->xmi_startoff2, xlf->xmi_blockcount);
+	if (!xfs_verify_fileext(mp, xlf->xmi_startoff2, xlf->xmi_blockcount))
+		return false;
+
+	if (xlf->xmi_flags & XFS_EXCHMAPS_SET_SIZES) {
+		if ((int64_t)xlf->xmi_isize1 < 0)
+			return false;
+		if ((int64_t)xlf->xmi_isize2 < 0)
+			return false;
+	}
+
+	return true;
 }
 
 /*
@@ -403,6 +413,13 @@ xfs_xmi_item_recover_intent(
 	*ipp1 = ip1;
 	*ipp2 = ip2;
 	xmi = xfs_exchmaps_init_intent(req);
+
+	/* Restore intended file sizes from recovered logged item */
+	if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
+		xmi->xmi_isize1 = xlf->xmi_isize1;
+		xmi->xmi_isize2 = xlf->xmi_isize2;
+	}
+
 	xfs_defer_add_item(dfp, &xmi->xmi_list);
 	return xmi;
 

[PATCH 5/6] xfs: don't modify file attributes or poke fsnotify for dry runs

[Darrick J. Wong]

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

LOLLM points out that a dry run shouldn't poke fsnotify because we don't
actually change the files.  I noticed that we also shouldn't be removing
file privileges when doing a dry run.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 42672471f938cd ("xfs: bind together the front and back ends of the file range exchange code")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_exchrange.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
index 94965a6c2187ff..56f9a6b47c47b0 100644
--- a/fs/xfs/xfs_exchrange.c
+++ b/fs/xfs/xfs_exchrange.c
@@ -701,6 +701,9 @@ xfs_exchrange_contents(
 	if (error)
 		goto out_unlock;
 
+	if (fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)
+		goto out_unlock;
+
 	/*
 	 * Finish the exchange by removing special file privileges like any
 	 * other file write would do.  This may involve turning on support for
@@ -783,6 +786,9 @@ xfs_exchange_range(
 	if (ret)
 		return ret;
 
+	if (fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)
+		return 0;
+
 	fsnotify_modify(fxr->file1);
 	if (fxr->file2 != fxr->file1)
 		fsnotify_modify(fxr->file2);

[PATCH 6/6] xfs: fix bnobt repair space reservation disposal failure

[Darrick J. Wong]

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

LOLLM complains that we don't bubble failures from xrep_abt_dispose_one
upwards in the callstack.  A failure to clean up the space used (or
reserved but not used) by the new bnobt/cntbt should be reported.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 4bdfd7d15747b1 ("xfs: repair free space btrees")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/alloc_repair.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index dce6ab0429dcfe..95e318e4f3a6c7 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -571,7 +571,7 @@ xrep_abt_dispose_one(
  * allocation, and blocks that didn't get used can be freed via the usual
  * (deferred) means.
  */
-STATIC void
+STATIC int
 xrep_abt_dispose_reservations(
 	struct xrep_abt		*ra,
 	int			error)
@@ -582,9 +582,13 @@ xrep_abt_dispose_reservations(
 		goto junkit;
 
 	list_for_each_entry_safe(resv, n, &ra->new_bnobt.resv_list, list) {
-		error = xrep_abt_dispose_one(ra, resv);
-		if (error)
+		int		error2 = xrep_abt_dispose_one(ra, resv);
+
+		if (error2) {
+			if (!error)
+				error = error2;
 			goto junkit;
+		}
 	}
 
 junkit:
@@ -596,6 +600,7 @@ xrep_abt_dispose_reservations(
 
 	xrep_newbt_cancel(&ra->new_bnobt);
 	xrep_newbt_cancel(&ra->new_cntbt);
+	return error;
 }
 
 /* Retrieve free space data for bulk load. */
@@ -801,7 +806,9 @@ xrep_abt_build_new_trees(
 		goto err_newbt;
 
 	/* Dispose of any unused blocks and the accounting information. */
-	xrep_abt_dispose_reservations(ra, error);
+	error = xrep_abt_dispose_reservations(ra, error);
+	if (error)
+		return error;
 
 	return xrep_roll_ag_trans(sc);
 
@@ -812,8 +819,7 @@ xrep_abt_build_new_trees(
 	xfs_btree_del_cursor(cnt_cur, error);
 	xfs_btree_del_cursor(bno_cur, error);
 err_newbt:
-	xrep_abt_dispose_reservations(ra, error);
-	return error;
+	return xrep_abt_dispose_reservations(ra, error);
 }
 
 /*

Re: [PATCH 1/6] xfs: fix xfs_rtrmapbt_mem_cursor for non-rmap filesystems

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:25:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> It's possible to construct an in-memory rtrmap btree for filesystems
> that don't have the rmap feature enabled.  The kernel doesn't do this,
> but xfs_repair will, if asked to reindex a filesystem that has rtreflink
> enabled but not rtrmap.  Therefore, we must create the cursor with
> enough levels to handle a maximally sized btree possible.

Should repair initialize m_rtrmap_maxlevels instead of overallocating
in the kernel?

Re: [PATCH 2/6] xfs: preserve owner on in-memory btree creation

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:25:56PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out a minor bug where a higher level function creating an
> in-memory btree is required to pass in an owner number, but the creation
> function erases that.  In-memory btrees are ephemeral so this really
> doesn't matter except for debugging.  But let's fix this papercut.
> 
> Cc: <stable@vger.kernel.org> # v6.9

Not really a stable candidate, is it?

Otherwise looks good:

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

Re: [PATCH 3/6] xfs: don't leak new_bp if xfs_btree_bload_drop_buf fails

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:26:12PM -0700, Darrick J. Wong wrote:
> xfs_btree_bload_drop_buf can hit an IO error if writing the delwri
> buffer list to disk fails.  In this case, we fail to release new_bp,
> which means we lose a locked buffer.  Fix that.

Looks good:

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

Re: [PATCH 4/6] xfs: actually recover intended file sizes in xfs_xmi_item_recover_intent

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:26:27PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that xfs_xmi_item_recover_intent doesn't actually
> restore the isize1 and isize2 fields that were recovered from an
> unfinished exchmaps log intent item.  Instead, xfs_exchmaps_init_intent
> sets the wrong isize values from the recovered inodes, with the result
> that the file sizes are not set correctly when item recovery finishes.
> Fix this by restoring isize[12] from the log item.

Eww.  Can we come up with a test cases for this that uses error
injection to shut down the file system at just the right time?

The fix looks good:

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

Re: [PATCH 5/6] xfs: don't modify file attributes or poke fsnotify for dry runs

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:26:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that a dry run shouldn't poke fsnotify because we don't
> actually change the files.  I noticed that we also shouldn't be removing
> file privileges when doing a dry run.

> +++ b/fs/xfs/xfs_exchrange.c
> @@ -701,6 +701,9 @@ xfs_exchrange_contents(
>  	if (error)
>  		goto out_unlock;
>  
> +	if (fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)
> +		goto out_unlock;
> +
>  	/*
>  	 * Finish the exchange by removing special file privileges like any
>  	 * other file write would do.  This may involve turning on support for

This just skip removing the privileges.  I guess the commit mentions
this on the side, but this is probably the important part, so my
emphasis it?  It would also seem easier to move the check into
xfs_exchange_range_finish next to the code that it guards.

> @@ -783,6 +786,9 @@ xfs_exchange_range(
>  	if (ret)
>  		return ret;
>  
> +	if (fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)
> +		return 0;
> +
>  	fsnotify_modify(fxr->file1);
>  	if (fxr->file2 != fxr->file1)
>  		fsnotify_modify(fxr->file2);

Move the fsnotify calls into an if statement instead of the
early return?

Re: [PATCH 6/6] xfs: fix bnobt repair space reservation disposal failure

[Christoph Hellwig]

On Wed, Aug 12, 2026 at 10:26:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that we don't bubble failures from xrep_abt_dispose_one
> upwards in the callstack.  A failure to clean up the space used (or
> reserved but not used) by the new bnobt/cntbt should be reported.

Looks good:

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