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

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

[Darrick J. Wong]

Hi all,

Here's an eleventh 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-11
---
Commits in this patchset:
 * xfs: snapshot scrub stats when rendering them
 * xfs: report healthy filesystem events in scrub stats
 * xfs: report runtime failures in scrub
 * xfs: remove redundant function declaration
 * xfs: snapshot old AGFL before rewriting it
 * xfs: bail out on bitmap errors in xrep_agfl_fill
---
 fs/xfs/scrub/dabtree.h         |    2 --
 fs/xfs/scrub/agheader_repair.c |   41 ++++++++++++++++++++++++++----------
 fs/xfs/scrub/stats.c           |   45 ++++++++++++++++++++++++++--------------
 3 files changed, 58 insertions(+), 30 deletions(-)

[PATCH 1/6] xfs: snapshot scrub stats when rendering them

[Darrick J. Wong]

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

LOLLM complains about concurrency problems in the scrub stats code
because xchk_stats_format doesn't synchronize in any way with updates.
These stats are only reported through debugfs so I don't think it really
matters, but I guess I exist to make bots happy now.

Cc: <stable@vger.kernel.org> # v6.6
Fixes: d7a74cad8f4513 ("xfs: track usage statistics of online fsck")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/stats.c |   32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)


diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
index 76f2515188d1b1..f3f1fbfb6d9955 100644
--- a/fs/xfs/scrub/stats.c
+++ b/fs/xfs/scrub/stats.c
@@ -99,25 +99,31 @@ xchk_stats_format(
 	int			ret = 0;
 
 	for (i = 0; i < XFS_SCRUB_TYPE_NR; i++, css++) {
+		struct xchk_scrub_stats	fss;
+
 		if (!name_map[i])
 			continue;
 
+		spin_lock(&css->css_lock);
+		memcpy(&fss, css, offsetof(struct xchk_scrub_stats, css_lock));
+		spin_unlock(&css->css_lock);
+
 		ret = scnprintf(buf, remaining,
  "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu\n",
 				name_map[i],
-				(unsigned int)css->invocations,
-				(unsigned int)css->clean,
-				(unsigned int)css->corrupt,
-				(unsigned int)css->preen,
-				(unsigned int)css->xfail,
-				(unsigned int)css->xcorrupt,
-				(unsigned int)css->incomplete,
-				(unsigned int)css->warning,
-				(unsigned int)css->retries,
-				(unsigned long long)css->checktime_us,
-				(unsigned int)css->repair_invocations,
-				(unsigned int)css->repair_success,
-				(unsigned long long)css->repairtime_us);
+				(unsigned int)fss.invocations,
+				(unsigned int)fss.clean,
+				(unsigned int)fss.corrupt,
+				(unsigned int)fss.preen,
+				(unsigned int)fss.xfail,
+				(unsigned int)fss.xcorrupt,
+				(unsigned int)fss.incomplete,
+				(unsigned int)fss.warning,
+				(unsigned int)fss.retries,
+				(unsigned long long)fss.checktime_us,
+				(unsigned int)fss.repair_invocations,
+				(unsigned int)fss.repair_success,
+				(unsigned long long)fss.repairtime_us);
 		if (ret <= 0)
 			break;
 

Re: [PATCH 1/6] xfs: snapshot scrub stats when rendering them

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:26:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains about concurrency problems in the scrub stats code
> because xchk_stats_format doesn't synchronize in any way with updates.
> These stats are only reported through debugfs so I don't think it really
> matters, but I guess I exist to make bots happy now.
> 
> Cc: <stable@vger.kernel.org> # v6.6
> Fixes: d7a74cad8f4513 ("xfs: track usage statistics of online fsck")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/stats.c |   32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
> index 76f2515188d1b1..f3f1fbfb6d9955 100644
> --- a/fs/xfs/scrub/stats.c
> +++ b/fs/xfs/scrub/stats.c
> @@ -99,25 +99,31 @@ xchk_stats_format(
>  	int			ret = 0;
>  
>  	for (i = 0; i < XFS_SCRUB_TYPE_NR; i++, css++) {
> +		struct xchk_scrub_stats	fss;
> +
>  		if (!name_map[i])
>  			continue;
>  
> +		spin_lock(&css->css_lock);
> +		memcpy(&fss, css, offsetof(struct xchk_scrub_stats, css_lock));
> +		spin_unlock(&css->css_lock);
> +
>  		ret = scnprintf(buf, remaining,
>   "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu\n",
>  				name_map[i],
> -				(unsigned int)css->invocations,
> -				(unsigned int)css->clean,
> -				(unsigned int)css->corrupt,
> -				(unsigned int)css->preen,
> -				(unsigned int)css->xfail,
> -				(unsigned int)css->xcorrupt,
> -				(unsigned int)css->incomplete,
> -				(unsigned int)css->warning,
> -				(unsigned int)css->retries,
> -				(unsigned long long)css->checktime_us,
> -				(unsigned int)css->repair_invocations,
> -				(unsigned int)css->repair_success,
> -				(unsigned long long)css->repairtime_us);
> +				(unsigned int)fss.invocations,
> +				(unsigned int)fss.clean,
> +				(unsigned int)fss.corrupt,
> +				(unsigned int)fss.preen,
> +				(unsigned int)fss.xfail,
> +				(unsigned int)fss.xcorrupt,
> +				(unsigned int)fss.incomplete,
> +				(unsigned int)fss.warning,
> +				(unsigned int)fss.retries,
> +				(unsigned long long)fss.checktime_us,
> +				(unsigned int)fss.repair_invocations,
> +				(unsigned int)fss.repair_success,
> +				(unsigned long long)fss.repairtime_us);
>  		if (ret <= 0)
>  			break;
>  
> 
> 

Re: [PATCH 1/6] xfs: snapshot scrub stats when rendering them

[Christoph Hellwig]

On Thu, Sep 03, 2026 at 11:26:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains about concurrency problems in the scrub stats code
> because xchk_stats_format doesn't synchronize in any way with updates.
> These stats are only reported through debugfs so I don't think it really
> matters, but I guess I exist to make bots happy now.

What concurrency problem?  There should be no need to have an exact
snapshot of all counters, so it's really READ_ONCE/WRITE_ONCE that
are needed here I think?

Either way maybe add a comment explaining the snapshotting if we want
to stick to it?

Re: [PATCH 1/6] xfs: snapshot scrub stats when rendering them

[Darrick J. Wong]

On Mon, Sep 07, 2026 at 07:58:33AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 03, 2026 at 11:26:02PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM complains about concurrency problems in the scrub stats code
> > because xchk_stats_format doesn't synchronize in any way with updates.
> > These stats are only reported through debugfs so I don't think it really
> > matters, but I guess I exist to make bots happy now.
> 
> What concurrency problem?  There should be no need to have an exact
> snapshot of all counters, so it's really READ_ONCE/WRITE_ONCE that
> are needed here I think?

Yeah, I suppose I could have gone with READ_ONCE instead of the
spinlock, but I also don't think the stats file is a hot path. :)

> Either way maybe add a comment explaining the snapshotting if we want
> to stick to it?

"Snapshot the entire stats object with a spinlock because this isn't a
hot path and we don't have to worry about users seeing slightly weird
numbers (e.g. invocations has incremented but none of the outcomes have)
if we race with xchk_stats_merge_one"?

--D

[PATCH 2/6] xfs: report healthy filesystem events in scrub stats

[Darrick J. Wong]

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

LOLLM also notices that I forgot to expose the "clean bill of health"
scrub stats.  Fix that.

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


diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
index f3f1fbfb6d9955..da0c05ffe5cd0b 100644
--- a/fs/xfs/scrub/stats.c
+++ b/fs/xfs/scrub/stats.c
@@ -84,6 +84,7 @@ static const char *name_map[XFS_SCRUB_TYPE_NR] = {
 	[XFS_SCRUB_TYPE_RGSUPER]	= "rgsuper",
 	[XFS_SCRUB_TYPE_RTRMAPBT]	= "rtrmapbt",
 	[XFS_SCRUB_TYPE_RTREFCBT]	= "rtrefcountbt",
+	[XFS_SCRUB_TYPE_HEALTHY]	= "healthy",
 };
 
 /* Format the scrub stats into a text buffer, similar to pcp style. */

Re: [PATCH 2/6] xfs: report healthy filesystem events in scrub stats

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:26:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM also notices that I forgot to expose the "clean bill of health"
> scrub stats.  Fix that.
> 
> 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/stats.c |    1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> 
> diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
> index f3f1fbfb6d9955..da0c05ffe5cd0b 100644
> --- a/fs/xfs/scrub/stats.c
> +++ b/fs/xfs/scrub/stats.c
> @@ -84,6 +84,7 @@ static const char *name_map[XFS_SCRUB_TYPE_NR] = {
>  	[XFS_SCRUB_TYPE_RGSUPER]	= "rgsuper",
>  	[XFS_SCRUB_TYPE_RTRMAPBT]	= "rtrmapbt",
>  	[XFS_SCRUB_TYPE_RTREFCBT]	= "rtrefcountbt",
> +	[XFS_SCRUB_TYPE_HEALTHY]	= "healthy",
>  };
>  
>  /* Format the scrub stats into a text buffer, similar to pcp style. */
> 
> 

Re: [PATCH 2/6] xfs: report healthy filesystem events in scrub stats

[Christoph Hellwig]

On Thu, Sep 03, 2026 at 11:26:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM also notices that I forgot to expose the "clean bill of health"
> scrub stats.  Fix that.

Looks good:

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

[PATCH 3/6] xfs: report runtime failures in scrub

[Darrick J. Wong]

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

Add a new counter so that we can track the number of runtime failures
encountered during scrubs.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/stats.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
index da0c05ffe5cd0b..3339cae4b39d14 100644
--- a/fs/xfs/scrub/stats.c
+++ b/fs/xfs/scrub/stats.c
@@ -29,6 +29,7 @@ struct xchk_scrub_stats {
 	uint32_t		incomplete;
 	uint32_t		warning;
 	uint32_t		retries;
+	uint32_t		runtime_errors;
 
 	/* repair stats */
 	uint32_t		repair_invocations;
@@ -110,7 +111,7 @@ xchk_stats_format(
 		spin_unlock(&css->css_lock);
 
 		ret = scnprintf(buf, remaining,
- "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu\n",
+ "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu %u\n",
 				name_map[i],
 				(unsigned int)fss.invocations,
 				(unsigned int)fss.clean,
@@ -124,7 +125,8 @@ xchk_stats_format(
 				(unsigned long long)fss.checktime_us,
 				(unsigned int)fss.repair_invocations,
 				(unsigned int)fss.repair_success,
-				(unsigned long long)fss.repairtime_us);
+				(unsigned long long)fss.repairtime_us,
+				(unsigned int)fss.runtime_errors);
 		if (ret <= 0)
 			break;
 
@@ -207,13 +209,17 @@ xchk_stats_merge_one(
 	}
 
 	/* caller applies this same transformation after we return */
-	if (error == -EFSCORRUPTED || error == -EFSBADCRC)
+	if (error == -EFSCORRUPTED || error == -EFSBADCRC) {
 		sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
+		error = 0;
+	}
 
 	css = &cs->cs_stats[sm->sm_type];
 	spin_lock(&css->css_lock);
 	css->invocations++;
-	if (!(sm_flags & XFS_SCRUB_OFLAG_UNCLEAN))
+	if (error)
+		css->runtime_errors++;
+	else if (!(sm_flags & XFS_SCRUB_OFLAG_UNCLEAN))
 		css->clean++;
 	if (sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 		css->corrupt++;

Re: [PATCH 3/6] xfs: report runtime failures in scrub

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:26:33PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Add a new counter so that we can track the number of runtime failures
> encountered during scrubs.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/scrub/stats.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> 
> diff --git a/fs/xfs/scrub/stats.c b/fs/xfs/scrub/stats.c
> index da0c05ffe5cd0b..3339cae4b39d14 100644
> --- a/fs/xfs/scrub/stats.c
> +++ b/fs/xfs/scrub/stats.c
> @@ -29,6 +29,7 @@ struct xchk_scrub_stats {
>  	uint32_t		incomplete;
>  	uint32_t		warning;
>  	uint32_t		retries;
> +	uint32_t		runtime_errors;
>  
>  	/* repair stats */
>  	uint32_t		repair_invocations;
> @@ -110,7 +111,7 @@ xchk_stats_format(
>  		spin_unlock(&css->css_lock);
>  
>  		ret = scnprintf(buf, remaining,
> - "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu\n",
> + "%s %u %u %u %u %u %u %u %u %u %llu %u %u %llu %u\n",
>  				name_map[i],
>  				(unsigned int)fss.invocations,
>  				(unsigned int)fss.clean,
> @@ -124,7 +125,8 @@ xchk_stats_format(
>  				(unsigned long long)fss.checktime_us,
>  				(unsigned int)fss.repair_invocations,
>  				(unsigned int)fss.repair_success,
> -				(unsigned long long)fss.repairtime_us);
> +				(unsigned long long)fss.repairtime_us,
> +				(unsigned int)fss.runtime_errors);
>  		if (ret <= 0)
>  			break;
>  
> @@ -207,13 +209,17 @@ xchk_stats_merge_one(
>  	}
>  
>  	/* caller applies this same transformation after we return */
> -	if (error == -EFSCORRUPTED || error == -EFSBADCRC)
> +	if (error == -EFSCORRUPTED || error == -EFSBADCRC) {
>  		sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
> +		error = 0;
> +	}
>  
>  	css = &cs->cs_stats[sm->sm_type];
>  	spin_lock(&css->css_lock);
>  	css->invocations++;
> -	if (!(sm_flags & XFS_SCRUB_OFLAG_UNCLEAN))
> +	if (error)
> +		css->runtime_errors++;
> +	else if (!(sm_flags & XFS_SCRUB_OFLAG_UNCLEAN))
>  		css->clean++;
>  	if (sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
>  		css->corrupt++;
> 
> 

Re: [PATCH 3/6] xfs: report runtime failures in scrub

[Christoph Hellwig]

Looks good:

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

[PATCH 4/6] xfs: remove redundant function declaration

[Darrick J. Wong]

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

Remove this useless code.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/dabtree.h |    2 --
 1 file changed, 2 deletions(-)


diff --git a/fs/xfs/scrub/dabtree.h b/fs/xfs/scrub/dabtree.h
index de291e3b77dd8e..d654c125feb4d8 100644
--- a/fs/xfs/scrub/dabtree.h
+++ b/fs/xfs/scrub/dabtree.h
@@ -37,8 +37,6 @@ bool xchk_da_process_error(struct xchk_da_btree *ds, int level, int *error);
 void xchk_da_set_corrupt(struct xchk_da_btree *ds, int level);
 void xchk_da_set_preen(struct xchk_da_btree *ds, int level);
 
-void xchk_da_set_preen(struct xchk_da_btree *ds, int level);
-
 int xchk_da_btree_hash(struct xchk_da_btree *ds, int level, __be32 *hashp);
 int xchk_da_btree(struct xfs_scrub *sc, int whichfork,
 		xchk_da_btree_rec_fn scrub_fn, void *private);

Re: [PATCH 4/6] xfs: remove redundant function declaration

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:26:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Remove this useless code.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/scrub/dabtree.h |    2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> 
> diff --git a/fs/xfs/scrub/dabtree.h b/fs/xfs/scrub/dabtree.h
> index de291e3b77dd8e..d654c125feb4d8 100644
> --- a/fs/xfs/scrub/dabtree.h
> +++ b/fs/xfs/scrub/dabtree.h
> @@ -37,8 +37,6 @@ bool xchk_da_process_error(struct xchk_da_btree *ds, int level, int *error);
>  void xchk_da_set_corrupt(struct xchk_da_btree *ds, int level);
>  void xchk_da_set_preen(struct xchk_da_btree *ds, int level);
>  
> -void xchk_da_set_preen(struct xchk_da_btree *ds, int level);
> -
>  int xchk_da_btree_hash(struct xchk_da_btree *ds, int level, __be32 *hashp);
>  int xchk_da_btree(struct xfs_scrub *sc, int whichfork,
>  		xchk_da_btree_rec_fn scrub_fn, void *private);
> 
> 

Re: [PATCH 4/6] xfs: remove redundant function declaration

[Christoph Hellwig]

Looks good:

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

[PATCH 5/6] xfs: snapshot old AGFL before rewriting it

[Darrick J. Wong]

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

LOLLM complains that we can't undo an attempt at fixing the AGFL if
anything goes wrong during the rewrite, so take a snapshot of the whole
buffer so that we can restore it.  Move the xrep_agfl_update_agf call so
that we only update the AGF if the AGFL update is 100% successful.

Cc: <stable@vger.kernel.org> # v4.19
Fixes: 0e93d3f43ec7d3 ("xfs: repair the AGFL")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/agheader_repair.c |   37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)


diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 2104512f1ee19a..46c95354ca6438 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -668,14 +668,16 @@ xrep_agfl_init_header(
 	struct xfs_scrub	*sc,
 	struct xfs_buf		*agfl_bp,
 	struct xagb_bitmap	*agfl_extents,
-	xfs_agblock_t		flcount)
+	xfs_agblock_t		flcount,
+	struct xfs_agfl		*old_agfl)
 {
 	struct xrep_agfl_fill	af = {
 		.sc		= sc,
 		.flcount	= flcount,
 	};
 	struct xfs_mount	*mp = sc->mp;
-	struct xfs_agfl		*agfl;
+	struct xfs_agfl		*agfl = XFS_BUF_TO_AGFL(agfl_bp);
+	const size_t		agfl_sz = BBTOB(agfl_bp->b_length);
 	int			error;
 
 	ASSERT(flcount <= xfs_agfl_size(mp));
@@ -684,8 +686,8 @@ xrep_agfl_init_header(
 	 * Start rewriting the header by setting the bno[] array to
 	 * NULLAGBLOCK, then setting AGFL header fields.
 	 */
-	agfl = XFS_BUF_TO_AGFL(agfl_bp);
-	memset(agfl, 0xFF, BBTOB(agfl_bp->b_length));
+	memcpy(old_agfl, agfl, agfl_sz);
+	memset(agfl, 0xFF, agfl_sz);
 	agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC);
 	agfl->agfl_seqno = cpu_to_be32(pag_agno(sc->sa.pag));
 	uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid);
@@ -700,13 +702,18 @@ xrep_agfl_init_header(
 	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
 	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
 	if (error)
-		return error;
+		goto err_undo;
 
 	/* Write new AGFL to disk. */
 	xfs_trans_buf_set_type(sc->tp, agfl_bp, XFS_BLFT_AGFL_BUF);
-	xfs_trans_log_buf(sc->tp, agfl_bp, 0, BBTOB(agfl_bp->b_length) - 1);
+	xfs_trans_log_buf(sc->tp, agfl_bp, 0, agfl_sz - 1);
 	xagb_bitmap_destroy(&af.used_extents);
 	return 0;
+
+err_undo:
+	xagb_bitmap_destroy(&af.used_extents);
+	memcpy(agfl, old_agfl, agfl_sz);
+	return error;
 }
 
 /* Repair the AGFL. */
@@ -718,6 +725,7 @@ xrep_agfl(
 	struct xfs_mount	*mp = sc->mp;
 	struct xfs_buf		*agf_bp;
 	struct xfs_buf		*agfl_bp;
+	struct xfs_agfl		*old_agfl;
 	xfs_agblock_t		flcount;
 	int			error;
 
@@ -725,6 +733,10 @@ xrep_agfl(
 	if (!xfs_has_rmapbt(mp))
 		return -EOPNOTSUPP;
 
+	old_agfl = kzalloc(BBTOB(XFS_FSS_TO_BB(mp, 1)), XCHK_GFP_FLAGS);
+	if (!old_agfl)
+		return -ENOMEM;
+
 	xagb_bitmap_init(&agfl_extents);
 
 	/*
@@ -734,7 +746,7 @@ xrep_agfl(
 	 */
 	error = xfs_alloc_read_agf(sc->sa.pag, sc->tp, 0, &agf_bp);
 	if (error)
-		return error;
+		goto err_old_agfl;
 
 	/*
 	 * Make sure we have the AGFL buffer, as scrub might have decided it
@@ -745,7 +757,7 @@ xrep_agfl(
 						XFS_AGFL_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), 0, &agfl_bp, NULL);
 	if (error)
-		return error;
+		goto err_old_agfl;
 	agfl_bp->b_ops = &xfs_agfl_buf_ops;
 
 	/* Gather all the extents we're going to put on the new AGFL. */
@@ -762,10 +774,11 @@ xrep_agfl(
 	 * we adjust the AGF flcount (which can fail) so avoid updating any
 	 * buffers until we know that part works.
 	 */
+	error = xrep_agfl_init_header(sc, agfl_bp, &agfl_extents, flcount,
+			old_agfl);
+	if (error)
+		goto err;
 	xrep_agfl_update_agf(sc, agf_bp, flcount);
-	error = xrep_agfl_init_header(sc, agfl_bp, &agfl_extents, flcount);
-	if (error)
-		goto err;
 
 	/*
 	 * Ok, the AGFL should be ready to go now.  Roll the transaction to
@@ -785,6 +798,8 @@ xrep_agfl(
 
 err:
 	xagb_bitmap_destroy(&agfl_extents);
+err_old_agfl:
+	kfree(old_agfl);
 	return error;
 }
 

Re: [PATCH 5/6] xfs: snapshot old AGFL before rewriting it

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:27:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that we can't undo an attempt at fixing the AGFL if
> anything goes wrong during the rewrite, so take a snapshot of the whole
> buffer so that we can restore it.  Move the xrep_agfl_update_agf call so
> that we only update the AGF if the AGFL update is 100% successful.
> 
> Cc: <stable@vger.kernel.org> # v4.19
> Fixes: 0e93d3f43ec7d3 ("xfs: repair the AGFL")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/agheader_repair.c |   37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> 
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index 2104512f1ee19a..46c95354ca6438 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -668,14 +668,16 @@ xrep_agfl_init_header(
>  	struct xfs_scrub	*sc,
>  	struct xfs_buf		*agfl_bp,
>  	struct xagb_bitmap	*agfl_extents,
> -	xfs_agblock_t		flcount)
> +	xfs_agblock_t		flcount,
> +	struct xfs_agfl		*old_agfl)
>  {
>  	struct xrep_agfl_fill	af = {
>  		.sc		= sc,
>  		.flcount	= flcount,
>  	};
>  	struct xfs_mount	*mp = sc->mp;
> -	struct xfs_agfl		*agfl;
> +	struct xfs_agfl		*agfl = XFS_BUF_TO_AGFL(agfl_bp);
> +	const size_t		agfl_sz = BBTOB(agfl_bp->b_length);
>  	int			error;
>  
>  	ASSERT(flcount <= xfs_agfl_size(mp));
> @@ -684,8 +686,8 @@ xrep_agfl_init_header(
>  	 * Start rewriting the header by setting the bno[] array to
>  	 * NULLAGBLOCK, then setting AGFL header fields.
>  	 */
> -	agfl = XFS_BUF_TO_AGFL(agfl_bp);
> -	memset(agfl, 0xFF, BBTOB(agfl_bp->b_length));
> +	memcpy(old_agfl, agfl, agfl_sz);
> +	memset(agfl, 0xFF, agfl_sz);
>  	agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC);
>  	agfl->agfl_seqno = cpu_to_be32(pag_agno(sc->sa.pag));
>  	uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid);
> @@ -700,13 +702,18 @@ xrep_agfl_init_header(
>  	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
>  	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
>  	if (error)
> -		return error;
> +		goto err_undo;
>  
>  	/* Write new AGFL to disk. */
>  	xfs_trans_buf_set_type(sc->tp, agfl_bp, XFS_BLFT_AGFL_BUF);
> -	xfs_trans_log_buf(sc->tp, agfl_bp, 0, BBTOB(agfl_bp->b_length) - 1);
> +	xfs_trans_log_buf(sc->tp, agfl_bp, 0, agfl_sz - 1);
>  	xagb_bitmap_destroy(&af.used_extents);
>  	return 0;
> +
> +err_undo:
> +	xagb_bitmap_destroy(&af.used_extents);
> +	memcpy(agfl, old_agfl, agfl_sz);
> +	return error;
>  }
>  
>  /* Repair the AGFL. */
> @@ -718,6 +725,7 @@ xrep_agfl(
>  	struct xfs_mount	*mp = sc->mp;
>  	struct xfs_buf		*agf_bp;
>  	struct xfs_buf		*agfl_bp;
> +	struct xfs_agfl		*old_agfl;
>  	xfs_agblock_t		flcount;
>  	int			error;
>  
> @@ -725,6 +733,10 @@ xrep_agfl(
>  	if (!xfs_has_rmapbt(mp))
>  		return -EOPNOTSUPP;
>  
> +	old_agfl = kzalloc(BBTOB(XFS_FSS_TO_BB(mp, 1)), XCHK_GFP_FLAGS);
> +	if (!old_agfl)
> +		return -ENOMEM;
> +
>  	xagb_bitmap_init(&agfl_extents);
>  
>  	/*
> @@ -734,7 +746,7 @@ xrep_agfl(
>  	 */
>  	error = xfs_alloc_read_agf(sc->sa.pag, sc->tp, 0, &agf_bp);
>  	if (error)
> -		return error;
> +		goto err_old_agfl;
>  
>  	/*
>  	 * Make sure we have the AGFL buffer, as scrub might have decided it
> @@ -745,7 +757,7 @@ xrep_agfl(
>  						XFS_AGFL_DADDR(mp)),
>  			XFS_FSS_TO_BB(mp, 1), 0, &agfl_bp, NULL);
>  	if (error)
> -		return error;
> +		goto err_old_agfl;
>  	agfl_bp->b_ops = &xfs_agfl_buf_ops;
>  
>  	/* Gather all the extents we're going to put on the new AGFL. */
> @@ -762,10 +774,11 @@ xrep_agfl(
>  	 * we adjust the AGF flcount (which can fail) so avoid updating any
>  	 * buffers until we know that part works.
>  	 */
> +	error = xrep_agfl_init_header(sc, agfl_bp, &agfl_extents, flcount,
> +			old_agfl);
> +	if (error)
> +		goto err;
>  	xrep_agfl_update_agf(sc, agf_bp, flcount);
> -	error = xrep_agfl_init_header(sc, agfl_bp, &agfl_extents, flcount);
> -	if (error)
> -		goto err;
>  
>  	/*
>  	 * Ok, the AGFL should be ready to go now.  Roll the transaction to
> @@ -785,6 +798,8 @@ xrep_agfl(
>  
>  err:
>  	xagb_bitmap_destroy(&agfl_extents);
> +err_old_agfl:
> +	kfree(old_agfl);
>  	return error;
>  }
>  
> 
> 

Re: [PATCH 5/6] xfs: snapshot old AGFL before rewriting it

[Christoph Hellwig]

Looks good:

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

[PATCH 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Darrick J. Wong]

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

LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
can fail, but we don't check the result of xagb_bitmap_walk, so we
silently drop the error and proceed with inconsistent incore data.
That shouldn't be allowed.

While we're at it, fix leaking the used_extents bitmap if the disunion
operation fails.

Cc: <stable@vger.kernel.org> # v6.6
Fixes: 014ad53732d2ba ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/agheader_repair.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 46c95354ca6438..a66b611588c47f 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -699,7 +699,9 @@ xrep_agfl_init_header(
 	 */
 	xagb_bitmap_init(&af.used_extents);
 	af.agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
-	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
+	error = xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
+	if (error && error != -ECANCELED)
+		goto err_undo;
 	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
 	if (error)
 		goto err_undo;

Re: [PATCH 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Carlos Maiolino]

On Thu, Sep 03, 2026 at 11:27:19PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
> can fail, but we don't check the result of xagb_bitmap_walk, so we
> silently drop the error and proceed with inconsistent incore data.
> That shouldn't be allowed.

Looks good.

> 
> While we're at it, fix leaking the used_extents bitmap if the disunion
> operation fails.

Smells this description belongs to the previous patch?

> 
> Cc: <stable@vger.kernel.org> # v6.6
> Fixes: 014ad53732d2ba ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/agheader_repair.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index 46c95354ca6438..a66b611588c47f 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -699,7 +699,9 @@ xrep_agfl_init_header(
>  	 */
>  	xagb_bitmap_init(&af.used_extents);
>  	af.agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
> -	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> +	error = xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> +	if (error && error != -ECANCELED)
> +		goto err_undo;
>  	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
>  	if (error)
>  		goto err_undo;
> 
> 

Re: [PATCH 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Darrick J. Wong]

On Fri, Sep 04, 2026 at 09:38:59AM +0200, Carlos Maiolino wrote:
> On Thu, Sep 03, 2026 at 11:27:19PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
> > can fail, but we don't check the result of xagb_bitmap_walk, so we
> > silently drop the error and proceed with inconsistent incore data.
> > That shouldn't be allowed.
> 
> Looks good.
> 
> > 
> > While we're at it, fix leaking the used_extents bitmap if the disunion
> > operation fails.
> 
> Smells this description belongs to the previous patch?

Yeah, that belongs in "xfs: snapshot old AGFL before rewriting it".
I'll move this sentence to there.

--D

> > 
> > Cc: <stable@vger.kernel.org> # v6.6
> > Fixes: 014ad53732d2ba ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/scrub/agheader_repair.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> > index 46c95354ca6438..a66b611588c47f 100644
> > --- a/fs/xfs/scrub/agheader_repair.c
> > +++ b/fs/xfs/scrub/agheader_repair.c
> > @@ -699,7 +699,9 @@ xrep_agfl_init_header(
> >  	 */
> >  	xagb_bitmap_init(&af.used_extents);
> >  	af.agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
> > -	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> > +	error = xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> > +	if (error && error != -ECANCELED)
> > +		goto err_undo;
> >  	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
> >  	if (error)
> >  		goto err_undo;
> > 
> > 
> 

[PATCH v1.1 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Darrick J. Wong]

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

LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
can fail, but we don't check the result of xagb_bitmap_walk, so we
silently drop the error and proceed with inconsistent incore data.
That shouldn't be allowed.

Cc: <stable@vger.kernel.org> # v6.6
Fixes: 014ad53732d2ba ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
--
v1.1: fix commit message
---
 fs/xfs/scrub/agheader_repair.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 46c95354ca6438..a66b611588c47f 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -699,7 +699,9 @@ xrep_agfl_init_header(
 	 */
 	xagb_bitmap_init(&af.used_extents);
 	af.agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
-	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
+	error = xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
+	if (error && error != -ECANCELED)
+		goto err_undo;
 	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
 	if (error)
 		goto err_undo;

Re: [PATCH v1.1 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Carlos Maiolino]

On Fri, Sep 04, 2026 at 09:13:02AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
> can fail, but we don't check the result of xagb_bitmap_walk, so we
> silently drop the error and proceed with inconsistent incore data.
> That shouldn't be allowed.
> 
> Cc: <stable@vger.kernel.org> # v6.6
> Fixes: 014ad53732d2ba ("xfs: use per-AG bitmaps to reap unused AG metadata blocks during repair")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> --
> v1.1: fix commit message


Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/scrub/agheader_repair.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index 46c95354ca6438..a66b611588c47f 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -699,7 +699,9 @@ xrep_agfl_init_header(
>  	 */
>  	xagb_bitmap_init(&af.used_extents);
>  	af.agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
> -	xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> +	error = xagb_bitmap_walk(agfl_extents, xrep_agfl_fill, &af);
> +	if (error && error != -ECANCELED)
> +		goto err_undo;
>  	error = xagb_bitmap_disunion(agfl_extents, &af.used_extents);
>  	if (error)
>  		goto err_undo;
> 

Re: [PATCH v1.1 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Christoph Hellwig]

Looks good:

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

Re: [PATCH v1.1 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Carlos Maiolino]

On Fri, 04 Sep 2026 09:13:02 -0700, Darrick J. Wong wrote:
> LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
> can fail, but we don't check the result of xagb_bitmap_walk, so we
> silently drop the error and proceed with inconsistent incore data.
> That shouldn't be allowed.
> 
> --
> 
> [...]

Applied to for-next, thanks!

[6/6] xfs: bail out on bitmap errors in xrep_agfl_fill
      (no commit info)

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH v1.1 6/6] xfs: bail out on bitmap errors in xrep_agfl_fill

[Carlos Maiolino]

On Mon, Sep 07, 2026 at 08:28:47AM +0200, Carlos Maiolino wrote:
> On Fri, 04 Sep 2026 09:13:02 -0700, Darrick J. Wong wrote:
> > LOLLM also points out that the xagb_bitmap_set call in xrep_agfl_fill
> > can fail, but we don't check the result of xagb_bitmap_walk, so we
> > silently drop the error and proceed with inconsistent incore data.
> > That shouldn't be allowed.
> > 
> > --
> > 
> > [...]
> 
> Applied to for-next, thanks!
> 
> [6/6] xfs: bail out on bitmap errors in xrep_agfl_fill
>       (no commit info)
> 
> Best regards,

This was a mistake, sorry. This patch ain't included yet

> -- 
> Carlos Maiolino <cem@kernel.org>
> 
>