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

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

[Darrick J. Wong]

Hi all,

Here's a thirteenth 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-13
---
Commits in this patchset:
 * xfs: guard against igrab failure in xrep_findparent_from_dcache
 * xfs: don't assert when XFS_SCRUB_TYPE_HEALTHY scans return corruption
 * xfs: fix attr fork block count checks in xrep_inode_blockcounts
 * xfs: release orphanage dir inode if chown fails
 * xfs: release AGFL after walking it during rmapbt repair
 * xfs: use correct jiffies comparison function in xchk_maybe_relax
---
 fs/xfs/scrub/scrub.h        |    2 +-
 fs/xfs/scrub/findparent.c   |    3 +++
 fs/xfs/scrub/health.c       |    6 +++---
 fs/xfs/scrub/inode_repair.c |    2 +-
 fs/xfs/scrub/orphanage.c    |    6 +++++-
 fs/xfs/scrub/rmap_repair.c  |    1 +
 6 files changed, 14 insertions(+), 6 deletions(-)

[PATCH 1/6] xfs: guard against igrab failure in xrep_findparent_from_dcache

[Darrick J. Wong]

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

LOLLM suggests that we need to handle igrab returning NULL here.  I
don't think it's possible for the inode to enter I_FREEING or
I_WILL_FREE while we have an active reference to the corresponding
dentry, but we can code defensively anyway.

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


diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c
index eab3ac2704befe..d921fe5a9b0c76 100644
--- a/fs/xfs/scrub/findparent.c
+++ b/fs/xfs/scrub/findparent.c
@@ -473,6 +473,9 @@ xrep_findparent_from_dcache(
 	pip = igrab(d_inode(parent));
 	dput(parent);
 
+	if (!pip)
+		goto out_dput;
+
 	if (S_ISDIR(pip->i_mode)) {
 		ret = pip->i_ino;
 		trace_xrep_findparent_from_dcache(sc->ip, ret);

[PATCH 2/6] xfs: don't assert when XFS_SCRUB_TYPE_HEALTHY scans return corruption

[Darrick J. Wong]

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

XFS_SCRUB_TYPE_HEALTHY is a synthentic scrub type so that xfs_scrub can
tell the kernel "Hey, I finished a scan and saw no problems" and have
the kernel forget that it saw indirect evidence of corruption.

Unfortunately, as LOLLM points out, it's possible for the health system
to record a new corruption just before xfs_scrub gets to
XFS_SCRUB_TYPE_HEALTHY.  In this case, the existing logic doesn't return
early and instead wanders into unknown regions of type_to_health_flag
and trips the assert because HEALTHY doesn't have a group assignment.

Fix the logic so that we always return early for a HEALTHY scrub type,
even if we decide not to call xchk_mark_all_healthy.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: a1f3e0cca41036 ("xfs: update health status if we get a clean bill of health")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/health.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/scrub/health.c b/fs/xfs/scrub/health.c
index 2171bcf0f6c1f2..487ecc5f9f3c29 100644
--- a/fs/xfs/scrub/health.c
+++ b/fs/xfs/scrub/health.c
@@ -202,9 +202,9 @@ xchk_update_health(
 	 * there's no sick flag defined for it, so we branch here ahead of the
 	 * mask check.
 	 */
-	if (sc->sm->sm_type == XFS_SCRUB_TYPE_HEALTHY &&
-	    !(sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)) {
-		xchk_mark_all_healthy(sc->mp);
+	if (sc->sm->sm_type == XFS_SCRUB_TYPE_HEALTHY) {
+		if (!(sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT))
+			xchk_mark_all_healthy(sc->mp);
 		return;
 	}
 

[PATCH 3/6] xfs: fix attr fork block count checks in xrep_inode_blockcounts

[Darrick J. Wong]

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

LOLLM points out that a file has an attr fork, it will call
xchk_inode_count_blocks to set @ablocks to the number of fsblocks mapped
by the attr fork; but then it'll compare @blocks (aka the count of
fsblocks mapped by the data fork).  We already checked that and we never
do anything with @acount, so I think this is clearly a bug.  Fix the
comparison.

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 8bc508336aa5f4..b87c2214623383 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1702,7 +1702,7 @@ xrep_inode_blockcounts(
 				&acount);
 		if (error)
 			return error;
-		if (count >= sc->mp->m_sb.sb_dblocks)
+		if (acount >= sc->mp->m_sb.sb_dblocks)
 			return -EFSCORRUPTED;
 		error = xrep_ino_ensure_extent_count(sc, XFS_ATTR_FORK,
 				nextents);

[PATCH 4/6] xfs: release orphanage dir inode if chown fails

[Darrick J. Wong]

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

LOLLM points out that we leak the igrab'd reference to the orphanage
directory inode if chowning it fails.  Fix that.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 1e58a8ccf2597c ("xfs: move orphan files to the orphanage")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/orphanage.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c
index 3aca66869b8000..21e31eeaa042f9 100644
--- a/fs/xfs/scrub/orphanage.c
+++ b/fs/xfs/scrub/orphanage.c
@@ -192,12 +192,16 @@ xrep_orphanage_create(
 	/* Make sure the orphanage is owned by root. */
 	error = xrep_chown_orphanage(sc, XFS_I(orphanage_inode));
 	if (error)
-		goto out_dput_orphanage;
+		goto out_rele_orphanage;
 
 	/* Stash the reference for later and bail out. */
 	sc->orphanage = XFS_I(orphanage_inode);
 	sc->orphanage_ilock_flags = 0;
+	orphanage_inode = NULL;
 
+out_rele_orphanage:
+	if (orphanage_inode)
+		xchk_irele(sc, XFS_I(orphanage_inode));
 out_dput_orphanage:
 	end_creating(orphanage_dentry);
 out_dput_root:

[PATCH 5/6] xfs: release AGFL after walking it during rmapbt repair

[Darrick J. Wong]

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

LOLLM suggests that we not hold the AGFL locked to the scrub transaction
for any longer than we have to when we're rebuilding the rmapbt.  Since
we only took it to generate an rmap record for the AGFL blocks, I think
we can safely release it.

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


diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index 590f9f41856ef7..725035bf490329 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -1109,6 +1109,7 @@ xrep_rmap_try_reserve(
 		return error;
 
 	error = xfs_agfl_walk(sc->mp, agf, agfl_bp, xrep_rmap_walk_agfl, &ra);
+	xfs_trans_brelse(sc->tp, agfl_bp);
 	if (error)
 		return error;
 

[PATCH 6/6] xfs: use correct jiffies comparison function in xchk_maybe_relax

[Darrick J. Wong]

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

LOLLM points out that we're supposed to use time_after_eq, not a raw >=
operation here, or else jiffies wraps can go unnoticed.  Fix this.

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


diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index 6d7d3523b71f25..7ade1d005efef6 100644
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -40,7 +40,7 @@ static inline int xchk_maybe_relax(struct xchk_relax *widget)
 		return 0;
 	widget->resched_nr = 0;
 
-	if (unlikely(widget->next_resched <= jiffies)) {
+	if (unlikely(time_after_eq(jiffies, widget->next_resched))) {
 		cond_resched();
 		widget->next_resched = XCHK_RELAX_NEXT;
 	}

Re: [PATCH 1/6] xfs: guard against igrab failure in xrep_findparent_from_dcache

[Christoph Hellwig]

On Wed, Sep 09, 2026 at 10:59:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM suggests that we need to handle igrab returning NULL here.  I
> don't think it's possible for the inode to enter I_FREEING or
> I_WILL_FREE while we have an active reference to the corresponding
> dentry, but we can code defensively anyway.

Yeah, I can't see how that would happen either, but handling the
NULL return looks sane as well, so:

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

Re: [PATCH 2/6] xfs: don't assert when XFS_SCRUB_TYPE_HEALTHY scans return corruption

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 3/6] xfs: fix attr fork block count checks in xrep_inode_blockcounts

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 4/6] xfs: release orphanage dir inode if chown fails

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 5/6] xfs: release AGFL after walking it during rmapbt repair

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 6/6] xfs: use correct jiffies comparison function in xchk_maybe_relax

[Christoph Hellwig]

On Wed, Sep 09, 2026 at 11:01:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that we're supposed to use time_after_eq, not a raw >=
> operation here, or else jiffies wraps can go unnoticed.  Fix this.

Looks good:

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