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

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

[Darrick J. Wong]

Hi all,

Here's a second 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-2
---
Commits in this patchset:
 * xfs: set xfarray killable sort correctly
 * xfs: fix off-by-one error when calling xchk_xref_has_rt_owner
 * xfs: handle non-inode owners for rtrmap record checking
 * xfs: fully check the parent handle when it points to the rootdir
 * xfs: clamp timestamp nanoseconds correctly
 * xfs: don't zap bmbt forks if they are MAXLEVELS tall
---
 fs/xfs/scrub/scrub.h        |    6 +++---
 fs/xfs/scrub/dirtree.c      |   16 ++++++++--------
 fs/xfs/scrub/inode_repair.c |    4 ++--
 fs/xfs/scrub/rtbitmap.c     |    2 +-
 fs/xfs/scrub/rtrmap.c       |    6 ++++++
 fs/xfs/scrub/xfarray.c      |    3 +--
 6 files changed, 21 insertions(+), 16 deletions(-)

[PATCH 1/6] xfs: set xfarray killable sort correctly

[Darrick J. Wong]

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

LOLLM noticed that we *disable* interruptible sorts when the KILLABLE
flag is set.  This is backwards.  Fix the incorrect logic, and rename
the variable to make the connection more obvious.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 271557de7cbfde ("xfs: reduce the rate of cond_resched calls inside scrub")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/scrub.h   |    6 +++---
 fs/xfs/scrub/xfarray.c |    3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index a3f1abc9139035..6d7d3523b71f25 100644
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -11,7 +11,7 @@ struct xfs_scrub;
 struct xchk_relax {
 	unsigned long	next_resched;
 	unsigned int	resched_nr;
-	bool		interruptible;
+	bool		killable;
 };
 
 /* Yield to the scheduler at most 10x per second. */
@@ -21,7 +21,7 @@ struct xchk_relax {
 	(struct xchk_relax){ \
 		.next_resched	= XCHK_RELAX_NEXT, \
 		.resched_nr	= 0, \
-		.interruptible	= true, \
+		.killable	= true, \
 	}
 
 /*
@@ -45,7 +45,7 @@ static inline int xchk_maybe_relax(struct xchk_relax *widget)
 		widget->next_resched = XCHK_RELAX_NEXT;
 	}
 
-	if (widget->interruptible && fatal_signal_pending(current))
+	if (widget->killable && fatal_signal_pending(current))
 		return -EINTR;
 
 	return 0;
diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index c7c4a71b6fa7c6..2ce24bfe4c0fab 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -487,8 +487,7 @@ xfarray_sortinfo_alloc(
 	xfarray_sortinfo_lo(si)[0] = 0;
 	xfarray_sortinfo_hi(si)[0] = array->nr - 1;
 	si->relax = INIT_XCHK_RELAX;
-	if (flags & XFARRAY_SORT_KILLABLE)
-		si->relax.interruptible = false;
+	si->relax.killable = !!(flags & XFARRAY_SORT_KILLABLE);
 
 	trace_xfarray_sort(si, nr_bytes);
 	*infop = si;

Re: [PATCH 1/6] xfs: set xfarray killable sort correctly

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:06:12PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we *disable* interruptible sorts when the KILLABLE
> flag is set.  This is backwards.  Fix the incorrect logic, and rename
> the variable to make the connection more obvious.

Looks good:

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

Can we come up with a test that actually kills a sort somewhat reliably?

Re: [PATCH 1/6] xfs: set xfarray killable sort correctly

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 08:11:34AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 13, 2026 at 11:06:12PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that we *disable* interruptible sorts when the KILLABLE
> > flag is set.  This is backwards.  Fix the incorrect logic, and rename
> > the variable to make the connection more obvious.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Can we come up with a test that actually kills a sort somewhat reliably?

Userspace doesn't have good visibility into when a sort is running, and
I don't think we want to add XFS_TEST_ERROR() chunks where we inject a
SIGKILL into current... however that might be done.

--D

[PATCH 2/6] xfs: fix off-by-one error when calling xchk_xref_has_rt_owner

[Darrick J. Wong]

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

LOLLM noticed an off-by-one error when computing the length of the
rtrmap to cross-check.

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/rtbitmap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/rtbitmap.c b/fs/xfs/scrub/rtbitmap.c
index de3f22f310f7eb..52c24d3d4be6ce 100644
--- a/fs/xfs/scrub/rtbitmap.c
+++ b/fs/xfs/scrub/rtbitmap.c
@@ -258,7 +258,7 @@ xchk_rtbitmap(
 	 * the last free extent we saw and the last possible extent in the rt
 	 * group.
 	 */
-	last_rgbno = rtg->rtg_extents * mp->m_sb.sb_rextsize - 1;
+	last_rgbno = rtg->rtg_extents * mp->m_sb.sb_rextsize;
 	if (rtb->next_free_rgbno < last_rgbno)
 		xchk_xref_has_rt_owner(sc, rtb->next_free_rgbno,
 				last_rgbno - rtb->next_free_rgbno);

Re: [PATCH 2/6] xfs: fix off-by-one error when calling xchk_xref_has_rt_owner

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:06:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed an off-by-one error when computing the length of the
> rtrmap to cross-check.

Looks good:

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

[PATCH 3/6] xfs: handle non-inode owners for rtrmap record checking

[Darrick J. Wong]

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

LOLLM noticed that two helper functions in the rtrmapbt scrub code don't
actually handle non-inode owners correctly -- CoW staging extents and
rgsuperblock extents are not shareable, but they are mergeable.  Fix
these two helpers.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 2d9a3e98053e8c ("xfs: allow overlapping rtrmapbt records for shared data extents")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/rtrmap.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/fs/xfs/scrub/rtrmap.c b/fs/xfs/scrub/rtrmap.c
index b3b2cf17ba2c08..564d19a97a2f8f 100644
--- a/fs/xfs/scrub/rtrmap.c
+++ b/fs/xfs/scrub/rtrmap.c
@@ -87,6 +87,9 @@ xchk_rtrmapbt_is_shareable(
 		return false;
 	if (irec->rm_flags & XFS_RMAP_UNWRITTEN)
 		return false;
+	if (irec->rm_owner == XFS_RMAP_OWN_COW ||
+	    irec->rm_owner == XFS_RMAP_OWN_FS)
+		return false;
 	return true;
 }
 
@@ -146,6 +149,9 @@ xchk_rtrmap_mergeable(
 		return false;
 	if (r1->rm_flags != r2->rm_flags)
 		return false;
+	if (r1->rm_owner == XFS_RMAP_OWN_COW ||
+	    r1->rm_owner == XFS_RMAP_OWN_FS)
+		return true;
 	return r1->rm_offset + r1->rm_blockcount == r2->rm_offset;
 }
 

Re: [PATCH 3/6] xfs: handle non-inode owners for rtrmap record checking

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:06:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that two helper functions in the rtrmapbt scrub code don't
> actually handle non-inode owners correctly -- CoW staging extents and
> rgsuperblock extents are not shareable, but they are mergeable.  Fix
> these two helpers.

The changes looks reasonable:

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

But why do we consider RT superblocks mergable?

Re: [PATCH 3/6] xfs: handle non-inode owners for rtrmap record checking

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 08:14:09AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 13, 2026 at 11:06:43PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that two helper functions in the rtrmapbt scrub code don't
> > actually handle non-inode owners correctly -- CoW staging extents and
> > rgsuperblock extents are not shareable, but they are mergeable.  Fix
> > these two helpers.
> 
> The changes looks reasonable:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But why do we consider RT superblocks mergable?

We don't, really, but most of the cross-referencing code in the two
repair tools don't look at the adjacent records just in case someone
split a record wrongly.  Except for the bmap btree, the kernel never
splits mergeable records which is why it's an error in the rmap btree.

--D

[PATCH 4/6] xfs: fully check the parent handle when it points to the rootdir

[Darrick J. Wong]

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

LOLLM noticed that the directory tree path checking declares the path to
be ok if the inumber in the parent pointer reaches the root directory.
Unfortunately, it neglects to check that the generation is correct.  Fix
that by moving the generation check up.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 928b721a11789a ("xfs: teach online scrub to find directory tree structure problems")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dirtree.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
index c6a210f7508fc2..b2cf6e5439d915 100644
--- a/fs/xfs/scrub/dirtree.c
+++ b/fs/xfs/scrub/dirtree.c
@@ -383,6 +383,14 @@ xchk_dirpath_step_up(
 		goto out_scanlock;
 	}
 
+	/* The handle encoded in the parent pointer must match. */
+	if (VFS_I(dp)->i_generation != be32_to_cpu(dl->pptr_rec.p_gen)) {
+		trace_xchk_dirpath_badgen(dl->sc, dp, path->path_nr,
+				path->nr_steps, &dl->xname, &dl->pptr_rec);
+		error = -EFSCORRUPTED;
+		goto out_scanlock;
+	}
+
 	/* We've reached the root directory; the path is ok. */
 	if (parent_ino == dl->root_ino) {
 		xchk_dirpath_set_outcome(dl, path, XCHK_DIRPATH_OK);
@@ -411,14 +419,6 @@ xchk_dirpath_step_up(
 		goto out_scanlock;
 	}
 
-	/* The handle encoded in the parent pointer must match. */
-	if (VFS_I(dp)->i_generation != be32_to_cpu(dl->pptr_rec.p_gen)) {
-		trace_xchk_dirpath_badgen(dl->sc, dp, path->path_nr,
-				path->nr_steps, &dl->xname, &dl->pptr_rec);
-		error = -EFSCORRUPTED;
-		goto out_scanlock;
-	}
-
 	/* Parent pointer must point up to a directory. */
 	if (!S_ISDIR(VFS_I(dp)->i_mode)) {
 		trace_xchk_dirpath_nondir_parent(dl->sc, dp, path->path_nr,

Re: [PATCH 4/6] xfs: fully check the parent handle when it points to the rootdir

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:06:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that the directory tree path checking declares the path to
> be ok if the inumber in the parent pointer reaches the root directory.
> Unfortunately, it neglects to check that the generation is correct.  Fix
> that by moving the generation check up.

Looks good:

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

[PATCH 5/6] xfs: clamp timestamp nanoseconds correctly

[Darrick J. Wong]

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

LOLLM noticed an off-by-one error in the nsec clamping; fix that so that
we never have tv_nsec == 1e9.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 2d295fe65776d1 ("xfs: repair inode records")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 493dcf5cc6c159..5565e80691a69e 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1757,7 +1757,7 @@ xrep_clamp_timestamp(
 	struct xfs_inode	*ip,
 	struct timespec64	*ts)
 {
-	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC);
+	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC - 1);
 	*ts = timestamp_truncate(*ts, VFS_I(ip));
 }
 

Re: [PATCH 5/6] xfs: clamp timestamp nanoseconds correctly

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:07:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed an off-by-one error in the nsec clamping; fix that so that
> we never have tv_nsec == 1e9.

Hah..  

>  {
> -	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC);
> +	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC - 1);
>  	*ts = timestamp_truncate(*ts, VFS_I(ip));

Looks good:

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

But I'd really expect this would be handled by core timing/timestamp
helpers.
>

Re: [PATCH 5/6] xfs: clamp timestamp nanoseconds correctly

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 08:15:57AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 13, 2026 at 11:07:15PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed an off-by-one error in the nsec clamping; fix that so that
> > we never have tv_nsec == 1e9.
> 
> Hah..  
> 
> >  {
> > -	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC);
> > +	ts->tv_nsec = clamp_t(long, ts->tv_nsec, 0, NSEC_PER_SEC - 1);
> >  	*ts = timestamp_truncate(*ts, VFS_I(ip));
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But I'd really expect this would be handled by core timing/timestamp
> helpers.

I'd have thought so too, but timestamp_truncate doesn't clamp tv_nsec to
[0, 1e9) because it assumes that callers (mostly the vfs) already did
that.

--D

[PATCH 6/6] xfs: don't zap bmbt forks if they are MAXLEVELS tall

[Darrick J. Wong]

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

LOLLM noticed a discrepancy between the bmbt level checks in the libxfs
bmbt code vs. the inode repair code.  We do actually allow a bmbt root
that proclaims to have a height of XFS_BM_MAXLEVELS.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: e744cef2060559 ("xfs: zap broken inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 5565e80691a69e..3ec41c19835116 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -921,7 +921,7 @@ xrep_dinode_bad_bmbt_fork(
 
 	if (nrecs == 0 || xfs_bmdr_space_calc(nrecs) > dfork_size)
 		return true;
-	if (level == 0 || level >= XFS_BM_MAXLEVELS(sc->mp, whichfork))
+	if (level == 0 || level > XFS_BM_MAXLEVELS(sc->mp, whichfork))
 		return true;
 
 	dmxr = xfs_bmdr_maxrecs(dfork_size, 0);

Re: [PATCH 6/6] xfs: don't zap bmbt forks if they are MAXLEVELS tall

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:07:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed a discrepancy between the bmbt level checks in the libxfs
> bmbt code vs. the inode repair code.  We do actually allow a bmbt root
> that proclaims to have a height of XFS_BM_MAXLEVELS.

Looks good:

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

I guess we need a test that actually creates such a deep tree.  But
we'll probably run out space / extents before..

Re: [PATCH 6/6] xfs: don't zap bmbt forks if they are MAXLEVELS tall

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 08:16:52AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 13, 2026 at 11:07:30PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed a discrepancy between the bmbt level checks in the libxfs
> > bmbt code vs. the inode repair code.  We do actually allow a bmbt root
> > that proclaims to have a height of XFS_BM_MAXLEVELS.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> I guess we need a test that actually creates such a deep tree.  But
> we'll probably run out space / extents before..

The bmapinflate command in xfs_db does this by writing a new bmap btree
with as many mappings pointing to the same "reflinked" block as you ask.
The only problem is that to hit maxlevels, you need to create a 1k
fsblock filesystem and then bmapinflate 2^54 records, which will take a
long time and require much memory.  Back in the day when Chandan was
working on nrext64 I tried this and it took 2 days and 400G of RAM to
write the huge bmap btree out to disk and generate the appropriate
rmap/reflink btrees.

--D