[PATCHSET] xfs: LLM-inspired bug fixes, part 5

[PATCHSET] xfs: LLM-inspired bug fixes, part 5

[Darrick J. Wong]

Hi all,

Here's a fifth 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-5
---
Commits in this patchset:
 * xfs: fix unit conversions in per_binval computation
 * xfs: fix short ifork reaping computation in xreap_bmapi_binval
 * xfs: fix name string recording in slowpath pptr tracepoints
 * xfs: don't leak dqacct if rhashtable insertion fails
 * xfs: adjust datadev sector count to reflect internal rt volumes
 * xfs: fix the rtrmap and rtrefcount _maxlevels_ondisk functions
---
 fs/xfs/scrub/trace.h                 |    2 +-
 fs/xfs/libxfs/xfs_rtrefcount_btree.c |    7 +++++--
 fs/xfs/libxfs/xfs_rtrmap_btree.c     |    4 +++-
 fs/xfs/scrub/quotacheck.c            |    4 +++-
 fs/xfs/scrub/reap.c                  |    6 +++---
 fs/xfs/xfs_buf_item_recover.c        |   12 ++++++++++--
 fs/xfs/xfs_super.c                   |   11 ++++++++++-
 7 files changed, 35 insertions(+), 11 deletions(-)

[PATCH 1/6] xfs: fix unit conversions in per_binval computation

[Darrick J. Wong]

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

LOLLM noticed that we're doing the unit conversion in the per_binval
computation backwards -- xfs_buf_inval_log_space's second parameter is
supposed to be in bytes, but max_binval is in units of fsblocks.  Hence
the conversion should be FSB -> B, not the other way around.

Cc: <stable@vger.kernel.org> # v6.18
Fixes: b2311ec6778fcd ("xfs: compute per-AG extent reap limits dynamically")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/reap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/reap.c b/fs/xfs/scrub/reap.c
index fcd14c1703ea04..496c6eab555e91 100644
--- a/fs/xfs/scrub/reap.c
+++ b/fs/xfs/scrub/reap.c
@@ -601,7 +601,7 @@ xreap_configure_agextent_limits(
 
 	/* Maximum overhead of invalidating one buffer. */
 	const unsigned int	per_binval =
-		xfs_buf_inval_log_space(1, XFS_B_TO_FSBT(mp, max_binval));
+		xfs_buf_inval_log_space(1, XFS_FSB_TO_B(mp, max_binval));
 
 	/*
 	 * For each transaction in a reap chain, we can delete some number of
@@ -680,7 +680,7 @@ xreap_configure_agcow_limits(
 
 	/* Overhead of invalidating one buffer */
 	const unsigned int	per_binval =
-		xfs_buf_inval_log_space(1, XFS_B_TO_FSBT(mp, max_binval));
+		xfs_buf_inval_log_space(1, XFS_FSB_TO_B(mp, max_binval));
 
 	/*
 	 * For each transaction in a reap chain, we can delete some number of

[PATCH 2/6] xfs: fix short ifork reaping computation in xreap_bmapi_binval

[Darrick J. Wong]

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

LOLLM got really confused about the update to imap->br_blockcount in
xreap_bmapi_binval if xreap_inc_binval returns false.  The intent of
this code is that we shorten the imap to whatever length of space we
invalidated so that the next iteration through the loop will start
wherever we left off.  Unfortunately, the calculation sets br_blockcount
to the amount of *unfinished* work, which means that we pointlessly
re-scan blocks that we already reaped.  This is benign, but we should
fix the computation anyway.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 5befb047b9f4de ("xfs: add the ability to reap entire inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/reap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/reap.c b/fs/xfs/scrub/reap.c
index 496c6eab555e91..f698b9be3dd1df 100644
--- a/fs/xfs/scrub/reap.c
+++ b/fs/xfs/scrub/reap.c
@@ -1399,7 +1399,7 @@ xreap_bmapi_binval(
 			 * far we've gotten.
 			 */
 			if (!xreap_inc_binval(rs)) {
-				imap->br_blockcount = agbno_next - bno;
+				imap->br_blockcount = bno - agbno;
 				goto out;
 			}
 		}

[PATCH 3/6] xfs: fix name string recording in slowpath pptr tracepoints

[Darrick J. Wong]

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

LOLLM observes that we memcpy from the xfs_name object, not the name
string pointed to by the xfs_name.  Fix that.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: b961c8bf1fc3d0 ("xfs: deferred scrub of dirents")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/trace.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 14aa0ec1f09e4a..362c6d39e9f580 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -1640,7 +1640,7 @@ DECLARE_EVENT_CLASS(xchk_pptr_class,
 		__entry->dev = ip->i_mount->m_super->s_dev;
 		__entry->ino = I_INO(ip);
 		__entry->namelen = name->len;
-		memcpy(__get_str(name), name, name->len);
+		memcpy(__get_str(name), name->name, name->len);
 		__entry->far_ino = far_ino;
 	),
 	TP_printk("dev %d:%d ino 0x%llx name '%.*s' far_ino 0x%llx",

[PATCH 4/6] xfs: don't leak dqacct if rhashtable insertion fails

[Darrick J. Wong]

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

LOLLM observes that xqcheck_mod_live_ino_dqtrx doesn't free the newly
allocated dqa object if rhashtable insertion fails.  Fix this leak.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: 200491875ce144 ("xfs: track quota updates during live quotacheck")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/quotacheck.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/quotacheck.c b/fs/xfs/scrub/quotacheck.c
index c199d128538e55..c32030a0544065 100644
--- a/fs/xfs/scrub/quotacheck.c
+++ b/fs/xfs/scrub/quotacheck.c
@@ -263,8 +263,10 @@ xqcheck_mod_live_ino_dqtrx(
 		dqa->tx_id = p->tx_id;
 		error = rhashtable_insert_fast(&xqc->shadow_dquot_acct,
 				&dqa->hash, xqcheck_dqacct_hash_params);
-		if (error)
+		if (error) {
+			kfree(dqa);
 			goto out_abort;
+		}
 	}
 
 	/* Find the shadow dqtrx (or an empty slot) here. */

[PATCH 5/6] xfs: adjust datadev sector count to reflect internal rt volumes

[Darrick J. Wong]

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

A media scan of a filesystem containing an internal rt volume produced
an error in xfs_scrub phase 6 complaining about a truncated realtime
device.  The rt device wasn't truncated, but the media scan code thought
we were trying to start a scan past the end of m_rtdev_targp.  That in
turn is an alias for m_ddev_targp, but in xfs_configure_buftarg we set
nr_sectors to the size of the data section.  Oops.

On these filesystems, the internal rt section comes immediately after
the data section.  We need to set the sector count for the data device
buftarg to the size of both sections.  Without this, media scans don't
work and media failure notifications from the kernel will be discarded
silently.

We also need to fix the superblock buffer recovery code to do the same.

Cc: <stable@vger.kernel.org> # v6.15
Fixes: bdc03eb5f98f6f ("xfs: allow internal RT devices for zoned mode")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_buf_item_recover.c |   12 ++++++++++--
 fs/xfs/xfs_super.c            |   11 ++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 02b95b89d1b597..140b1c16ea15c3 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -720,6 +720,7 @@ xlog_recover_do_primary_sb_buffer(
 	xfs_lsn_t			current_lsn)
 {
 	struct xfs_dsb			*dsb = bp->b_addr;
+	xfs_rfsblock_t			dblocks;
 	xfs_agnumber_t			orig_agcount = mp->m_sb.sb_agcount;
 	xfs_rgnumber_t			orig_rgcount = mp->m_sb.sb_rgcount;
 	int				error;
@@ -738,9 +739,16 @@ xlog_recover_do_primary_sb_buffer(
 
 	/*
 	 * Grow can change the device size.  Mirror that into the buftarg.
+	 *
+	 * Internal rt volumes are placed immediately after the data device,
+	 * so set the buftarg sector count to the end of the rt volume so that
+	 * we can do media scans and handle media failure reports.
 	 */
-	mp->m_ddev_targp->bt_nr_sectors =
-		XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
+	if (mp->m_sb.sb_rtstart)
+		dblocks = mp->m_sb.sb_rtstart + mp->m_sb.sb_rblocks;
+	else
+		dblocks = mp->m_sb.sb_dblocks;
+	mp->m_ddev_targp->bt_nr_sectors = XFS_FSB_TO_BB(mp, dblocks);
 	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) {
 		mp->m_rtdev_targp->bt_nr_sectors =
 			XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 8531d526fc4409..9898cd9db0ada6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -555,10 +555,19 @@ STATIC int
 xfs_setup_devices(
 	struct xfs_mount	*mp)
 {
+	xfs_rfsblock_t		dblocks = mp->m_sb.sb_dblocks;
 	int			error;
 
+	/*
+	 * Internal rt volumes are placed immediately after the data device,
+	 * so set the buftarg sector count to the end of the rt volume so that
+	 * we can do media scans and handle media failure reports.
+	 */
+	if (mp->m_sb.sb_rtstart)
+		dblocks = mp->m_sb.sb_rtstart + mp->m_sb.sb_rblocks;
+
 	error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize,
-			mp->m_sb.sb_dblocks);
+			dblocks);
 	if (error)
 		return error;
 

[PATCH 6/6] xfs: fix the rtrmap and rtrefcount _maxlevels_ondisk functions

[Darrick J. Wong]

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

The _maxlevels_ondisk functions are used to compute the size of
in-memory btree cursors for each btree type.  Unfortunately, LOLLM
noticed that the rtrmap and rtrefcount versions of these functions
forget to account for the inode root, which means that we could access
beyond the end of the cursor given a sufficiently large btree.  Fix
this.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 9abe03a0e4f978 ("xfs: introduce realtime refcount btree ondisk definitions")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_rtrefcount_btree.c |    7 +++++--
 fs/xfs/libxfs/xfs_rtrmap_btree.c     |    4 +++-
 2 files changed, 8 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
index f27b80a199ba2e..971cb27400b042 100644
--- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
@@ -489,8 +489,11 @@ xfs_rtrefcountbt_maxlevels_ondisk(void)
 	minrecs[0] = xfs_rtrefcountbt_block_maxrecs(blocklen, true) / 2;
 	minrecs[1] = xfs_rtrefcountbt_block_maxrecs(blocklen, false) / 2;
 
-	/* We need at most one record for every block in an rt group. */
-	return xfs_btree_compute_maxlevels(minrecs, XFS_MAX_RGBLOCKS);
+	/*
+	 * We need at most one record for every block in an rt group, and
+	 * one extra level for the inode root.
+	 */
+	return xfs_btree_compute_maxlevels(minrecs, XFS_MAX_RGBLOCKS) + 1;
 }
 
 int __init
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index c264bc5651c0a6..0cb2113d5b4045 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -716,10 +716,12 @@ xfs_rtrmapbt_maxlevels_ondisk(void)
 	 * happens, which means that we must compute the max height based on
 	 * what the btree will look like if it consumes almost all the blocks
 	 * in the data device due to maximal sharing factor.
+	 *
+	 * Add one extra level for the inode root.
 	 */
 	max_dblocks = -1U; /* max ag count */
 	max_dblocks *= XFS_MAX_CRC_AG_BLOCKS;
-	return xfs_btree_space_to_height(minrecs, max_dblocks);
+	return xfs_btree_space_to_height(minrecs, max_dblocks) + 1;
 }
 
 int __init

Re: [PATCH 1/6] xfs: fix unit conversions in per_binval computation

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 10:26:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we're doing the unit conversion in the per_binval
> computation backwards -- xfs_buf_inval_log_space's second parameter is
> supposed to be in bytes, but max_binval is in units of fsblocks.  Hence
> the conversion should be FSB -> B, not the other way around.

Oops.

Looks good:

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

Re: [PATCH 2/6] xfs: fix short ifork reaping computation in xreap_bmapi_binval

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 10:26:33PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM got really confused about the update to imap->br_blockcount in
> xreap_bmapi_binval if xreap_inc_binval returns false.  The intent of
> this code is that we shorten the imap to whatever length of space we
> invalidated so that the next iteration through the loop will start
> wherever we left off.  Unfortunately, the calculation sets br_blockcount
> to the amount of *unfinished* work, which means that we pointlessly
> re-scan blocks that we already reaped.  This is benign, but we should
> fix the computation anyway.

Looks good:

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

Re: [PATCH 3/6] xfs: fix name string recording in slowpath pptr tracepoints

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 10:26:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM observes that we memcpy from the xfs_name object, not the name
> string pointed to by the xfs_name.  Fix that.

Heh.  I guess no one really uses the tracepoints all that much? :)

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

Re: [PATCH 4/6] xfs: don't leak dqacct if rhashtable insertion fails

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 10:27:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM observes that xqcheck_mod_live_ino_dqtrx doesn't free the newly
> allocated dqa object if rhashtable insertion fails.  Fix this leak.

Looks good:

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

Re: [PATCH 5/6] xfs: adjust datadev sector count to reflect internal rt volumes

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 10:27:20PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> A media scan of a filesystem containing an internal rt volume produced
> an error in xfs_scrub phase 6 complaining about a truncated realtime
> device.  The rt device wasn't truncated, but the media scan code thought
> we were trying to start a scan past the end of m_rtdev_targp.  That in
> turn is an alias for m_ddev_targp, but in xfs_configure_buftarg we set
> nr_sectors to the size of the data section.  Oops.
> 
> On these filesystems, the internal rt section comes immediately after
> the data section.  We need to set the sector count for the data device
> buftarg to the size of both sections.  Without this, media scans don't
> work and media failure notifications from the kernel will be discarded
> silently.
> 
> We also need to fix the superblock buffer recovery code to do the same.

This feels wrong.  Nothing should acess the internal rtdev through
the data device buftarg.

Is this an LLM report bug, or did you actually run it using scrub?  If
so can you help with the reproducer?

Re: [PATCH 6/6] xfs: fix the rtrmap and rtrefcount _maxlevels_ondisk functions

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 5/6] xfs: adjust datadev sector count to reflect internal rt volumes

[Darrick J. Wong]

On Thu, Jul 30, 2026 at 10:22:04AM +0200, Christoph Hellwig wrote:
> On Wed, Jul 29, 2026 at 10:27:20PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > A media scan of a filesystem containing an internal rt volume produced
> > an error in xfs_scrub phase 6 complaining about a truncated realtime
> > device.  The rt device wasn't truncated, but the media scan code thought
> > we were trying to start a scan past the end of m_rtdev_targp.  That in
> > turn is an alias for m_ddev_targp, but in xfs_configure_buftarg we set
> > nr_sectors to the size of the data section.  Oops.
> > 
> > On these filesystems, the internal rt section comes immediately after
> > the data section.  We need to set the sector count for the data device
> > buftarg to the size of both sections.  Without this, media scans don't
> > work and media failure notifications from the kernel will be discarded
> > silently.
> > 
> > We also need to fix the superblock buffer recovery code to do the same.
> 
> This feels wrong.  Nothing should acess the internal rtdev through
> the data device buftarg.

It's the only one available, because xfs_setup_devices aliases
ddev_targp to rtdev_targp for internal rt volumes:

	if (mp->m_sb.sb_rtstart) {
		if (mp->m_rtdev_targp) {
			xfs_warn(mp,
		"can't use internal and external rtdev at the same time");
			return -EINVAL;
		}
		mp->m_rtdev_targp = mp->m_ddev_targp;
	}

So we're doing that anyway.  We could separate them by creating a second
buftarg with a duplicate struct file, but that would involve a bunch of
rototillingi because a fair amount of code changes behavior on
rtdev==ddev now.

The only field that the media verification code uses is bt_nr_sectors,
which it uses to trim verification requests to wherever the filesystem
thinks is the end of the device.  I could change that to call
bdev_nr_sectors() to fix the bug, but then we'd have to deal with media
failures for LBAs outside of the filesystem, which didn't seem ideal.

Also, if someone asynchronously reports a media error in the internal
rtdev to us through fs_holder_ops, the report will mention a range that
is beyond bt_nr_sectors but actually within the filesystem.
xfs_dax_notify_failure currently does the right thing because it doesn't
check bt_nr_sectors, but that also feels wrong. ;)

So I went with making bt_nr_sectors bigger because that felt the least
awkward.  IIRC it's supposed to represent the location of the end of the
filesystem on a particular blockdev, not the highest LBA that the fs
can xfs_buf_read(), right?

> Is this an LLM report bug, or did you actually run it using scrub?  If
> so can you help with the reproducer?

I found it accidentally while triaging another bug that LOLLM found.
I turned on the tracepoints (and btrace) for debugging and noticed that
it wasn't issuing reads for the internal rt device:

# mkfs.xfs /dev/sda -r zoned=1
# mount /dev/sda /mnt/scratch
# trace-cmd <big ugly command line> &
# xfs_io -x -c 'verifymedia -r' /mnt/scratch

--D