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

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

[Darrick J. Wong]

Hi all,

Here's a sixteenth 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-16
---
Commits in this patchset:
 * xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair
 * xfs: online quotacheck must dirty dquot if enforcement adjustments needed
 * xfs: fix buffer overruns in xfs_ioc_attr_list
 * xfs: clean up after failed metafile relinking
 * xfs: pass xfs_trans_resv object to reservation calculation helpers
 * xfs: fix xfs_rename_space_res for non-pptr filesystems
 * xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork
 * xfs: fix maximum atomic cow length computation
 * xfs: add missing healthmon trace strings
 * xfarray: don't crash when sorting if array element crosses a folio
 * xfarray: don't allow users to unset in the middle of an array
 * xfs: don't allow sorting sparse arrays
 * xfs: simply the free space btree repair code
 * xfarray: warn against sorting arrays with identical elements
---
 fs/xfs/libxfs/xfs_metadir.h                        |    5 -
 fs/xfs/libxfs/xfs_symlink_remote.h                 |    2 
 fs/xfs/scrub/xfarray.h                             |    9 +
 fs/xfs/xfs_dquot.h                                 |    3 
 fs/xfs/xfs_trace.h                                 |   28 ++++
 .../filesystems/xfs/xfs-online-fsck-design.rst     |   13 --
 fs/xfs/libxfs/xfs_dquot_buf.c                      |   13 --
 fs/xfs/libxfs/xfs_inode_fork.c                     |    4 -
 fs/xfs/libxfs/xfs_metadir.c                        |   30 ++++-
 fs/xfs/libxfs/xfs_symlink_remote.c                 |    8 +
 fs/xfs/libxfs/xfs_trans_resv.c                     |   38 +++---
 fs/xfs/libxfs/xfs_trans_space.c                    |    6 -
 fs/xfs/scrub/alloc_repair.c                        |   15 +-
 fs/xfs/scrub/inode_repair.c                        |   26 ++++
 fs/xfs/scrub/quota_repair.c                        |    5 -
 fs/xfs/scrub/quotacheck_repair.c                   |   54 ++++++++
 fs/xfs/scrub/xfarray.c                             |  133 ++++++--------------
 fs/xfs/xfs_dquot.c                                 |   27 +++-
 fs/xfs/xfs_handle.c                                |    2 
 fs/xfs/xfs_qm.c                                    |    6 -
 fs/xfs/xfs_qm_syscalls.c                           |   17 +--
 fs/xfs/xfs_reflink.c                               |    7 +
 fs/xfs/xfs_trans_dquot.c                           |    6 -
 23 files changed, 252 insertions(+), 205 deletions(-)

[PATCH 01/14] xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair

[Darrick J. Wong]

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

LOLLM noticed that everywhere else in the kernel, a call to
xfs_qm_adjust_dqlimits precedes every call to xfs_qm_adjust_dqtimers.
In particular, mount-time quotacheck does this, but online quotacheck
does not.

Looking at xfs_qm_adjust_dqlimits, that function is in charge of
conveying default limits to a dquot if that dquot's limits have been
zeroed.  That's quite possible in a repair, so we actually need to do
that.

However, there's a pre-existing pattern in the kernel -- for non-root
dquots, first we call xfs_qm_adjust_dqlimits to set the dquot's limits
to the defaults if they are zero, and then xfs_qm_adjust_dqtimers to
start grace periods if the dquot's usage is above the softlimit.  The
grace period decision cannot be made correctly if we forget to import
the default limits.

Therefore, let's combine both into a single xfs_qm_adjust_dqenforcement
helper that takes care of both pieces, which fixes quotacheck and makes
it hard to repeat this mistake.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: 96ed2ae4a9b06b ("xfs: repair dquots based on live quotacheck results")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_dquot.h               |    3 +--
 fs/xfs/scrub/quota_repair.c      |    5 +----
 fs/xfs/scrub/quotacheck_repair.c |    3 +--
 fs/xfs/xfs_dquot.c               |   27 +++++++++++++++++----------
 fs/xfs/xfs_qm.c                  |    6 +-----
 fs/xfs/xfs_qm_syscalls.c         |   17 +++++++----------
 fs/xfs/xfs_trans_dquot.c         |    6 +-----
 7 files changed, 29 insertions(+), 38 deletions(-)


diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index bbb824adca82ce..326f85d2b4116c 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -204,8 +204,7 @@ void xfs_dquot_to_disk(struct xfs_disk_dquot *ddqp, struct xfs_dquot *dqp);
 void		xfs_qm_dqdestroy(struct xfs_dquot *dqp);
 int		xfs_qm_dqflush(struct xfs_dquot *dqp, struct xfs_buf *bp);
 void		xfs_qm_dqunpin_wait(struct xfs_dquot *dqp);
-void		xfs_qm_adjust_dqtimers(struct xfs_dquot *d);
-void		xfs_qm_adjust_dqlimits(struct xfs_dquot *d);
+void		xfs_qm_adjust_dqenforcement(struct xfs_dquot *d);
 xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
 				xfs_dqtype_t type);
 int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index 59302e8afc7ef0..89f7ea4f92ef4a 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -248,10 +248,7 @@ xrep_quota_item(
 
 	dq->q_flags |= XFS_DQFLAG_DIRTY;
 	xfs_trans_dqjoin(sc->tp, dq);
-	if (dq->q_id) {
-		xfs_qm_adjust_dqlimits(dq);
-		xfs_qm_adjust_dqtimers(dq);
-	}
+	xfs_qm_adjust_dqenforcement(dq);
 	xfs_trans_log_dquot(sc->tp, dq);
 	return xfs_trans_roll(&sc->tp);
 
diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
index dbb522e1513b0b..48ee08df302a42 100644
--- a/fs/xfs/scrub/quotacheck_repair.c
+++ b/fs/xfs/scrub/quotacheck_repair.c
@@ -110,8 +110,7 @@ xqcheck_commit_dquot(
 
 	/* Commit the dirty dquot to disk. */
 	dq->q_flags |= XFS_DQFLAG_DIRTY;
-	if (dq->q_id)
-		xfs_qm_adjust_dqtimers(dq);
+	xfs_qm_adjust_dqenforcement(dq);
 	xfs_trans_log_dquot(xqc->sc->tp, dq);
 	return xrep_trans_commit(xqc->sc);
 
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index e696ee36c2e8d2..56d990eb2d02ba 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -115,18 +115,15 @@ xfs_qm_dqdestroy(
  * We overwrite the dquot limits only if they are zero and this
  * is not the root dquot.
  */
-void
+static void
 xfs_qm_adjust_dqlimits(
 	struct xfs_dquot	*dq)
 {
 	struct xfs_mount	*mp = dq->q_mount;
 	struct xfs_quotainfo	*q = mp->m_quotainfo;
-	struct xfs_def_quota	*defq;
+	struct xfs_def_quota	*defq = xfs_get_defquota(q, xfs_dquot_type(dq));
 	int			prealloc = 0;
 
-	ASSERT(dq->q_id);
-	defq = xfs_get_defquota(q, xfs_dquot_type(dq));
-
 	if (!dq->q_blk.softlimit) {
 		dq->q_blk.softlimit = defq->blk.soft;
 		prealloc = 1;
@@ -207,22 +204,32 @@ xfs_qm_adjust_res_timer(
  * get reset to zero, however, when we find the count to be under
  * the soft limit (they are only ever set non-zero via userspace).
  */
-void
+static void
 xfs_qm_adjust_dqtimers(
 	struct xfs_dquot	*dq)
 {
 	struct xfs_mount	*mp = dq->q_mount;
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
-	struct xfs_def_quota	*defq;
-
-	ASSERT(dq->q_id);
-	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
+	struct xfs_def_quota	*defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
 
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_blk, &defq->blk);
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_ino, &defq->ino);
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_rtb, &defq->rtb);
 }
 
+/* Adjust enforcement limits and timers after a change in usage. */
+void
+xfs_qm_adjust_dqenforcement(
+	struct xfs_dquot	*dq)
+{
+	if (dq->q_id == 0)
+		return;
+
+	xfs_qm_adjust_dqlimits(dq);
+	xfs_qm_adjust_dqtimers(dq);
+	dq->q_flags |= XFS_DQFLAG_DIRTY;
+}
+
 /*
  * initialize a buffer full of dquots and log the whole thing
  */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 54d00d543b513a..008fed8624be2c 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1294,11 +1294,7 @@ xfs_qm_quotacheck_dqadjust(
 	 *
 	 * There are no timers for the default values set in the root dquot.
 	 */
-	if (dqp->q_id) {
-		xfs_qm_adjust_dqlimits(dqp);
-		xfs_qm_adjust_dqtimers(dqp);
-	}
-
+	xfs_qm_adjust_dqenforcement(dqp);
 	dqp->q_flags |= XFS_DQFLAG_DIRTY;
 out_unlock:
 	mutex_unlock(&dqp->q_qlock);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 21a7849868288f..e2c5f57a7ba86c 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -371,16 +371,13 @@ xfs_qm_scall_setqlim(
 	if (newlim->d_fieldmask & QC_INO_TIMER)
 		xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
 
-	if (id != 0) {
-		/*
-		 * If the user is now over quota, start the timelimit.
-		 * The user will not be 'warned'.
-		 * Note that we keep the timers ticking, whether enforcement
-		 * is on or off. We don't really want to bother with iterating
-		 * over all ondisk dquots and turning the timers on/off.
-		 */
-		xfs_qm_adjust_dqtimers(dqp);
-	}
+	/*
+	 * If the user is now over quota, start the timelimit.  The user will
+	 * not be 'warned'.  Note that we keep the timers ticking, whether
+	 * enforcement is on or off. We don't really want to bother with
+	 * iterating over all ondisk dquots and turning the timers on/off.
+	 */
+	xfs_qm_adjust_dqenforcement(dqp);
 	dqp->q_flags |= XFS_DQFLAG_DIRTY;
 	xfs_trans_log_dquot(tp, dqp);
 
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 1606c614f205ae..93ec876792cc76 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -566,11 +566,7 @@ xfs_trans_apply_dquot_deltas(
 			 * Get any default limits in use.
 			 * Start/reset the timer(s) if needed.
 			 */
-			if (dqp->q_id) {
-				xfs_qm_adjust_dqlimits(dqp);
-				xfs_qm_adjust_dqtimers(dqp);
-			}
-
+			xfs_qm_adjust_dqenforcement(dqp);
 			dqp->q_flags |= XFS_DQFLAG_DIRTY;
 			/*
 			 * add this to the list of items to get logged

Re: [PATCH 01/14] xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair

[Christoph Hellwig]

Looks good:

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

[PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Darrick J. Wong]

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

LOLLM noticed that we have no way to force xchk_commit_dquot to call
xfs_qm_adjust_dqenforcement if nothing else is wrong with the dquot.
Therefore, add a new predicate to force the dirty flag if the dquot has
zero limits and there are default limits; or if the grace period timer
needs adjusting.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: 96ed2ae4a9b06b ("xfs: repair dquots based on live quotacheck results")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/quotacheck_repair.c |   51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)


diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
index 48ee08df302a42..e2208510939f08 100644
--- a/fs/xfs/scrub/quotacheck_repair.c
+++ b/fs/xfs/scrub/quotacheck_repair.c
@@ -39,6 +39,54 @@
  * dquot is locked.
  */
 
+static bool
+xqcheck_dqres_force_dirty(
+	const struct xfs_dquot_res	*res,
+	const struct xfs_quota_limits	*qlim)
+{
+	/* zero limits mean that we should set the default limits */
+	if (res->softlimit == 0 && qlim->soft != 0)
+		return true;
+	if (res->hardlimit == 0 && qlim->hard != 0)
+		return true;
+
+	/* do we need to adjust the timer setting? */
+	if ((res->softlimit && res->count > res->softlimit) ||
+	    (res->hardlimit && res->count > res->hardlimit)) {
+		if (res->timer == 0 && qlim->time != 0)
+			return true;
+	} else {
+		if (res->timer)
+			return true;
+	}
+
+	return false;
+}
+
+/* Decide if we need to adjust the dquot limits or timers */
+static bool
+xqcheck_dquot_force_dirty(
+	const struct xfs_dquot	*dq)
+{
+	struct xfs_quotainfo	*qi = dq->q_mount->m_quotainfo;
+	struct xfs_def_quota	*defq;
+
+	/* root dquot does not enforce limits */
+	if (dq->q_id == 0)
+		return false;
+
+	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
+
+	if (xqcheck_dqres_force_dirty(&dq->q_blk, &defq->blk))
+		return true;
+	if (xqcheck_dqres_force_dirty(&dq->q_ino, &defq->ino))
+		return true;
+	if (xqcheck_dqres_force_dirty(&dq->q_rtb, &defq->rtb))
+		return true;
+
+	return false;
+}
+
 /* Commit new counters to a dquot. */
 static int
 xqcheck_commit_dquot(
@@ -91,6 +139,9 @@ xqcheck_commit_dquot(
 		dirty = true;
 	}
 
+	if (!dirty && xqcheck_dquot_force_dirty(dq))
+		dirty = true;
+
 	xcdq.flags |= (XQCHECK_DQUOT_REPAIR_SCANNED | XQCHECK_DQUOT_WRITTEN);
 	error = xfarray_store(counts, dq->q_id, &xcdq);
 	if (error == -EFBIG) {

Re: [PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Darrick J. Wong]

On Sun, Sep 20, 2026 at 11:15:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we have no way to force xchk_commit_dquot to call
> xfs_qm_adjust_dqenforcement if nothing else is wrong with the dquot.
> Therefore, add a new predicate to force the dirty flag if the dquot has
> zero limits and there are default limits; or if the grace period timer
> needs adjusting.
> 
> Cc: <stable@vger.kernel.org> # v6.9
> Fixes: 96ed2ae4a9b06b ("xfs: repair dquots based on live quotacheck results")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/quotacheck_repair.c |   51 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
> index 48ee08df302a42..e2208510939f08 100644
> --- a/fs/xfs/scrub/quotacheck_repair.c
> +++ b/fs/xfs/scrub/quotacheck_repair.c
> @@ -39,6 +39,54 @@
>   * dquot is locked.
>   */
>  
> +static bool
> +xqcheck_dqres_force_dirty(
> +	const struct xfs_dquot_res	*res,
> +	const struct xfs_quota_limits	*qlim)
> +{
> +	/* zero limits mean that we should set the default limits */
> +	if (res->softlimit == 0 && qlim->soft != 0)
> +		return true;
> +	if (res->hardlimit == 0 && qlim->hard != 0)
> +		return true;
> +
> +	/* do we need to adjust the timer setting? */
> +	if ((res->softlimit && res->count > res->softlimit) ||
> +	    (res->hardlimit && res->count > res->hardlimit)) {
> +		if (res->timer == 0 && qlim->time != 0)

Shashiko says:

"Does this skip repairing missing grace period timers when the default grace
period is configured to 0?
If a quota usage exceeds its limit but has a missing or corrupt timer
(res->timer == 0) and the default grace period (qlim->time) is 0, this
condition evaluates to false. This causes the repair code to skip dirtying
the dquot, bypassing xfs_qm_adjust_dqenforcement().
When the unfixed dquot is subsequently flushed (e.g., via background
writeback or a log force), xfs_qm_dqflush_check() will find count > softlimit
but timer == 0. This is treated as a fatal inconsistency and triggers
SHUTDOWN_CORRUPT_INCORE, effectively turning a reparable condition into a
denial of service."

Yes, that should just be "if (!res->timer) return true;" since we want
to force xfs_qm_adjust_dqtimers to reset it.  I forgot that !qlim->time
just means "expires immediately", not "expires never".

Will fix in next revision.

--D

> +			return true;
> +	} else {
> +		if (res->timer)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/* Decide if we need to adjust the dquot limits or timers */
> +static bool
> +xqcheck_dquot_force_dirty(
> +	const struct xfs_dquot	*dq)
> +{
> +	struct xfs_quotainfo	*qi = dq->q_mount->m_quotainfo;
> +	struct xfs_def_quota	*defq;
> +
> +	/* root dquot does not enforce limits */
> +	if (dq->q_id == 0)
> +		return false;
> +
> +	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
> +
> +	if (xqcheck_dqres_force_dirty(&dq->q_blk, &defq->blk))
> +		return true;
> +	if (xqcheck_dqres_force_dirty(&dq->q_ino, &defq->ino))
> +		return true;
> +	if (xqcheck_dqres_force_dirty(&dq->q_rtb, &defq->rtb))
> +		return true;
> +
> +	return false;
> +}
> +
>  /* Commit new counters to a dquot. */
>  static int
>  xqcheck_commit_dquot(
> @@ -91,6 +139,9 @@ xqcheck_commit_dquot(
>  		dirty = true;
>  	}
>  
> +	if (!dirty && xqcheck_dquot_force_dirty(dq))
> +		dirty = true;
> +
>  	xcdq.flags |= (XQCHECK_DQUOT_REPAIR_SCANNED | XQCHECK_DQUOT_WRITTEN);
>  	error = xfarray_store(counts, dq->q_id, &xcdq);
>  	if (error == -EFBIG) {
> 
> 

[PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Darrick J. Wong]

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

When we converted the al_offset array in struct xfs_attrlist into a VLA,
the size of the object shrank by 4 bytes.  Unfortunately, the buffer
size validation in the attrlist ioctl wasn't updated to notice this, so
the al_offset[0] assignment blindly writes off the end of the buffer.
LOLLM noticed the omitted check and complained.  Probably should've left
working code alone but for everyone wanting these ***n static checkers.

Cc: <stable@vger.kernel.org> # v6.5
Fixes: 371baf5c9750a2 ("xfs: convert flex-array declarations in struct xfs_attrlist*")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_handle.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_handle.c b/fs/xfs/xfs_handle.c
index 0689cade8f74c2..fd9d4d8258fff2 100644
--- a/fs/xfs/xfs_handle.c
+++ b/fs/xfs/xfs_handle.c
@@ -409,7 +409,7 @@ xfs_ioc_attr_list(
 	void				*buffer;
 	int				error;
 
-	if (bufsize < sizeof(struct xfs_attrlist) ||
+	if (bufsize < struct_size(alist, al_offset, 1) ||
 	    bufsize > XFS_XATTR_LIST_MAX)
 		return -EINVAL;
 

Re: [PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:15:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> When we converted the al_offset array in struct xfs_attrlist into a VLA,
> the size of the object shrank by 4 bytes.  Unfortunately, the buffer
> size validation in the attrlist ioctl wasn't updated to notice this, so
> the al_offset[0] assignment blindly writes off the end of the buffer.
> LOLLM noticed the omitted check and complained.  Probably should've left
> working code alone but for everyone wanting these ***n static checkers.

It's not really static checkers, but fundamental semantics.  Arrays of
size 1 used for VLAs always were a bad idea and should have never been
used.

The fix looks good:

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

Re: [PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:15:32PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:15:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > When we converted the al_offset array in struct xfs_attrlist into a VLA,
> > the size of the object shrank by 4 bytes.  Unfortunately, the buffer
> > size validation in the attrlist ioctl wasn't updated to notice this, so
> > the al_offset[0] assignment blindly writes off the end of the buffer.
> > LOLLM noticed the omitted check and complained.  Probably should've left
> > working code alone but for everyone wanting these ***n static checkers.
> 
> It's not really static checkers, but fundamental semantics.  Arrays of
> size 1 used for VLAs always were a bad idea and should have never been
> used.

Ok I'll just delete the whole sentence.

> The fix looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

--D

[PATCH 04/14] xfs: clean up after failed metafile relinking

[Darrick J. Wong]

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

Once we start a metadir update to link in a file, we have to commit or
cancel it, just like any other operation.  LOLLM pointed out that I
forgot that, so fix it.  This fixes a bug in xfs_repair.

However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
call to xfs_dqinode_metadir_link.

Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
takes only the xfs_metadir_update object, and handles everything from
start to finish.  This enables us to make xfs_metadir_commit a static
function too.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_metadir.h   |    5 +----
 fs/xfs/libxfs/xfs_dquot_buf.c |   13 +------------
 fs/xfs/libxfs/xfs_metadir.c   |   30 +++++++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 19 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_metadir.h b/fs/xfs/libxfs/xfs_metadir.h
index e434b9d1c93200..b64f9fc5ca7867 100644
--- a/fs/xfs/libxfs/xfs_metadir.h
+++ b/fs/xfs/libxfs/xfs_metadir.h
@@ -38,10 +38,7 @@ int xfs_metadir_create_file(struct xfs_metadir_update *upd, umode_t mode,
 		xfs_metadir_createfn create, void *priv,
 		struct xfs_inode **ipp);
 
-int xfs_metadir_start_link(struct xfs_metadir_update *upd);
-int xfs_metadir_link(struct xfs_metadir_update *upd);
-
-int xfs_metadir_commit(struct xfs_metadir_update *upd);
+int xfs_metadir_link_file(struct xfs_metadir_update *upd);
 
 int xfs_metadir_mkdir(struct xfs_inode *dp, const char *path,
 		struct xfs_inode **ipp);
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 77954d1d924cc3..6c8a9afdfb1c0f 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -454,19 +454,8 @@ xfs_dqinode_metadir_link(
 		.path			= xfs_dqinode_path(type),
 		.ip			= ip,
 	};
-	int				error;
 
-	error = xfs_metadir_start_link(&upd);
-	if (error)
-		return error;
-
-	error = xfs_metadir_link(&upd);
-	if (error)
-		return error;
-
-	xfs_trans_log_inode(upd.tp, upd.ip, XFS_ILOG_CORE);
-
-	return xfs_metadir_commit(&upd);
+	return xfs_metadir_link_file(&upd);
 }
 #endif /* __KERNEL__ */
 
diff --git a/fs/xfs/libxfs/xfs_metadir.c b/fs/xfs/libxfs/xfs_metadir.c
index 7c6b086b73db61..1ff26e55b1864f 100644
--- a/fs/xfs/libxfs/xfs_metadir.c
+++ b/fs/xfs/libxfs/xfs_metadir.c
@@ -317,7 +317,7 @@ xfs_metadir_create(
  * Begin the process of linking a metadata file by allocating transactions
  * and locking whatever resources we're going to need.
  */
-int
+static int
 xfs_metadir_start_link(
 	struct xfs_metadir_update	*upd)
 {
@@ -364,7 +364,7 @@ xfs_metadir_start_link(
  * The path (up to the final component) must already exist, but the final
  * component must not already exist.
  */
-int
+static int
 xfs_metadir_link(
 	struct xfs_metadir_update	*upd)
 {
@@ -409,7 +409,7 @@ xfs_metadir_link(
 #endif /* ! __KERNEL__ */
 
 /* Commit a metadir update and unlock/drop all resources. */
-int
+static int
 xfs_metadir_commit(
 	struct xfs_metadir_update	*upd)
 {
@@ -499,3 +499,27 @@ xfs_metadir_mkdir(
 
 	return xfs_metadir_create_file(&upd, S_IFDIR, NULL, NULL, ipp);
 }
+
+#ifndef __KERNEL__
+/* Link a metadata file into a metadata directory. */
+int
+xfs_metadir_link_file(
+	struct xfs_metadir_update	*upd)
+{
+	int				error;
+
+	error = xfs_metadir_start_link(upd);
+	if (error)
+		return error;
+
+	error = xfs_metadir_link(upd);
+	if (error) {
+		xfs_metadir_cancel(upd, error);
+		return error;
+	}
+
+	xfs_trans_log_inode(upd->tp, upd->ip, XFS_ILOG_CORE);
+
+	return xfs_metadir_commit(upd);
+}
+#endif /* ! __KERNEL__ */

Re: [PATCH 04/14] xfs: clean up after failed metafile relinking

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Once we start a metadir update to link in a file, we have to commit or
> cancel it, just like any other operation.  LOLLM pointed out that I
> forgot that, so fix it.  This fixes a bug in xfs_repair.
> 
> However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
> function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
> call to xfs_dqinode_metadir_link.
> 
> Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
> takes only the xfs_metadir_update object, and handles everything from
> start to finish.  This enables us to make xfs_metadir_commit a static
> function too.
> 
> Cc: <stable@vger.kernel.org> # v6.13
> Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes

AFAIK all this isn not used at all in the kernel, so a stable tag
feels a bit odd.

Otherwise looks good:

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

Re: [PATCH 04/14] xfs: clean up after failed metafile relinking

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:17:11PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Once we start a metadir update to link in a file, we have to commit or
> > cancel it, just like any other operation.  LOLLM pointed out that I
> > forgot that, so fix it.  This fixes a bug in xfs_repair.
> > 
> > However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
> > function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
> > call to xfs_dqinode_metadir_link.
> > 
> > Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
> > takes only the xfs_metadir_update object, and handles everything from
> > start to finish.  This enables us to make xfs_metadir_commit a static
> > function too.
> > 
> > Cc: <stable@vger.kernel.org> # v6.13
> > Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes
> 
> AFAIK all this isn not used at all in the kernel, so a stable tag
> feels a bit odd.

Will remove.

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

Thanks!

--D

[PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

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

xfs_calc_namespace_reservations computes the directory tree related
transaction reservations for a given xfs_trans_resv object.  The helpers
it relies on, however, read the live one from the xfs_mount even if
we're doing this for minlogsize calculations.  In practice this
shouldn't be a big deal since the minlogsize and live reservation
objects don't differ in a meaningful way, but LOLLM complained about the
inconsistency so let's fix it anyway.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 7dba4a5fe1c5cd ("xfs: extend transaction reservations for parent attributes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_trans_resv.c |   38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 3151e97ca8ff6d..1c20a7c27aa1bf 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -606,10 +606,10 @@ static inline unsigned int xfs_calc_pptr_replace_overhead(void)
  */
 STATIC uint
 xfs_calc_rename_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		t1, t2, t3 = 0;
 
 	t1 = xfs_calc_inode_res(mp, 5) +
@@ -715,10 +715,10 @@ xfs_link_log_count(
  */
 STATIC uint
 xfs_calc_link_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		t1, t2, t3 = 0;
 
 	overhead += xfs_calc_iunlink_remove_reservation(mp);
@@ -777,10 +777,10 @@ xfs_remove_log_count(
  */
 STATIC uint
 xfs_calc_remove_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int            overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv   *resp = M_RES(mp);
 	unsigned int            t1, t2, t3 = 0;
 
 	overhead += xfs_calc_iunlink_add_reservation(mp);
@@ -862,9 +862,9 @@ xfs_icreate_log_count(
 
 STATIC uint
 xfs_calc_icreate_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
 	unsigned int		t1, t2, t3 = 0;
 
@@ -911,9 +911,10 @@ xfs_mkdir_log_count(
  */
 STATIC uint
 xfs_calc_mkdir_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	return xfs_calc_icreate_reservation(mp);
+	return xfs_calc_icreate_reservation(mp, resp);
 }
 
 static inline unsigned int
@@ -940,9 +941,10 @@ xfs_symlink_log_count(
  */
 STATIC uint
 xfs_calc_symlink_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	return xfs_calc_icreate_reservation(mp) +
+	return xfs_calc_icreate_reservation(mp, resp) +
 	       xfs_calc_buf_res(1, XFS_SYMLINK_MAXLEN);
 }
 
@@ -1265,27 +1267,27 @@ xfs_calc_namespace_reservations(
 {
 	ASSERT(resp->tr_attrsetm.tr_logres > 0);
 
-	resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp);
+	resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp, resp);
 	resp->tr_rename.tr_logcount = xfs_rename_log_count(mp, resp);
 	resp->tr_rename.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_link.tr_logres = xfs_calc_link_reservation(mp);
+	resp->tr_link.tr_logres = xfs_calc_link_reservation(mp, resp);
 	resp->tr_link.tr_logcount = xfs_link_log_count(mp, resp);
 	resp->tr_link.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_remove.tr_logres = xfs_calc_remove_reservation(mp);
+	resp->tr_remove.tr_logres = xfs_calc_remove_reservation(mp, resp);
 	resp->tr_remove.tr_logcount = xfs_remove_log_count(mp, resp);
 	resp->tr_remove.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_symlink.tr_logres = xfs_calc_symlink_reservation(mp);
+	resp->tr_symlink.tr_logres = xfs_calc_symlink_reservation(mp, resp);
 	resp->tr_symlink.tr_logcount = xfs_symlink_log_count(mp, resp);
 	resp->tr_symlink.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_create.tr_logres = xfs_calc_icreate_reservation(mp);
+	resp->tr_create.tr_logres = xfs_calc_icreate_reservation(mp, resp);
 	resp->tr_create.tr_logcount = xfs_icreate_log_count(mp, resp);
 	resp->tr_create.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
+	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp, resp);
 	resp->tr_mkdir.tr_logcount = xfs_mkdir_log_count(mp, resp);
 	resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 }

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfs_calc_namespace_reservations computes the directory tree related
> transaction reservations for a given xfs_trans_resv object.  The helpers
> it relies on, however, read the live one from the xfs_mount even if
> we're doing this for minlogsize calculations.  In practice this
> shouldn't be a big deal since the minlogsize and live reservation
> objects don't differ in a meaningful way, but LOLLM complained about the
> inconsistency so let's fix it anyway.
> 
> Cc: <stable@vger.kernel.org> # v6.10

Is this really a stable candidate?  Same for the current merge windos,
at some point we need to split these fixes into those that could
cause real issues and those who don't to not flood later -rcs with
lots of updates.  And this looks like a very clear candidate to not
rush.

Otherwise looks good:

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

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:18:52PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs_calc_namespace_reservations computes the directory tree related
> > transaction reservations for a given xfs_trans_resv object.  The helpers
> > it relies on, however, read the live one from the xfs_mount even if
> > we're doing this for minlogsize calculations.  In practice this
> > shouldn't be a big deal since the minlogsize and live reservation
> > objects don't differ in a meaningful way, but LOLLM complained about the
> > inconsistency so let's fix it anyway.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> 
> Is this really a stable candidate?  Same for the current merge windos,
> at some point we need to split these fixes into those that could
> cause real issues and those who don't to not flood later -rcs with
> lots of updates.  And this looks like a very clear candidate to not
> rush.

I asked cem if we could just queue them all for 7.4 this morning and it
sounded like he was ok with that, what with ALPS and LPC coming up soon
anyway.

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

Thanks!

--D

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:18:52PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs_calc_namespace_reservations computes the directory tree related
> > transaction reservations for a given xfs_trans_resv object.  The helpers
> > it relies on, however, read the live one from the xfs_mount even if
> > we're doing this for minlogsize calculations.  In practice this
> > shouldn't be a big deal since the minlogsize and live reservation
> > objects don't differ in a meaningful way, but LOLLM complained about the
> > inconsistency so let's fix it anyway.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> 
> Is this really a stable candidate?  Same for the current merge windos,
> at some point we need to split these fixes into those that could
> cause real issues and those who don't to not flood later -rcs with
> lots of updates.  And this looks like a very clear candidate to not
> rush.

<nod> I'll drop this trailer then.

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

Thanks!

--D

[PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Darrick J. Wong]

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

We don't need to reserve space for a parent pointer update for an
existing target if parent pointers are disabled.  Fix this regression
(which LOLLM noticed) so that rename reservations go back to what they
were before parent pointers.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_trans_space.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_trans_space.c b/fs/xfs/libxfs/xfs_trans_space.c
index c4cd547033e584..7edd0d86f0bdb2 100644
--- a/fs/xfs/libxfs/xfs_trans_space.c
+++ b/fs/xfs/libxfs/xfs_trans_space.c
@@ -127,10 +127,10 @@ xfs_rename_space_res(
 		if (has_whiteout)
 			ret += xfs_parent_calc_space_res(mp, src_namelen);
 		ret += 2 * xfs_parent_calc_space_res(mp, target_namelen);
+
+		if (target_exists)
+			ret += xfs_parent_calc_space_res(mp, target_namelen);
 	}
 
-	if (target_exists)
-		ret += xfs_parent_calc_space_res(mp, target_namelen);
-
 	return ret;
 }

Re: [PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> We don't need to reserve space for a parent pointer update for an
> existing target if parent pointers are disabled.  Fix this regression
> (which LOLLM noticed) so that rename reservations go back to what they
> were before parent pointers.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")

This just relaxed a reservation.  I don't think this is a stable/7.3
candidate and probably should not have a fixes tag that causes folks
to backport it.

Otherwise looks good:

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

Re: [PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:20:01PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:32PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > We don't need to reserve space for a parent pointer update for an
> > existing target if parent pointers are disabled.  Fix this regression
> > (which LOLLM noticed) so that rename reservations go back to what they
> > were before parent pointers.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> > Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")
> 
> This just relaxed a reservation.  I don't think this is a stable/7.3
> candidate and probably should not have a fixes tag that causes folks
> to backport it.

Yes it relaxes a reservation, but only for non-parent pointers
filesystems running on newer kernels.  IOWs, it reduces the reservation
back to what it was before parent pointers, so it actually has some
performance implications for old filesystems.

OTOH nobody's complained about the dip in performance so <shrug> I don't
really care that much either way.

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

Thanks!

--D

[PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Darrick J. Wong]

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

LOLLM noticed that online repair of a broken symlink file could fail
unnecessarily if a local-format symlink target isn't null terminated.
The ondisk target isn't required to be null terminated, but repair
enforces that anyway because it uses the validator for the incore
symlink target.  (The incore buffer is always null-terminated).  Fix
this by reverting the changes to xfs_symlink_shortform_verify and adding
an ondisk-specific helper in inode_repair.c.

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/libxfs/xfs_symlink_remote.h |    2 +-
 fs/xfs/libxfs/xfs_inode_fork.c     |    4 +---
 fs/xfs/libxfs/xfs_symlink_remote.c |    8 ++++++--
 fs/xfs/scrub/inode_repair.c        |   26 +++++++++++++++++++++++++-
 4 files changed, 33 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_symlink_remote.h b/fs/xfs/libxfs/xfs_symlink_remote.h
index c1672fe1f17bb2..3f6590602473d0 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.h
+++ b/fs/xfs/libxfs/xfs_symlink_remote.h
@@ -18,7 +18,7 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset,
 void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp,
 				 struct xfs_inode *ip, struct xfs_ifork *ifp,
 				 void *priv);
-xfs_failaddr_t xfs_symlink_shortform_verify(void *sfp, int64_t size);
+xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip);
 int xfs_symlink_remote_read(struct xfs_inode *ip, char *link);
 int xfs_symlink_write_target(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_ino_t owner, const char *target_path, int pathlen,
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 606a36526ce245..486fe7ab8b6110 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -683,9 +683,7 @@ xfs_ifork_verify_local_data(
 		break;
 	}
 	case S_IFLNK: {
-		struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
-
-		fa = xfs_symlink_shortform_verify(ifp->if_data, ifp->if_bytes);
+		fa = xfs_symlink_shortform_verify(ip);
 		break;
 	}
 	default:
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index b0dc3888bf1b40..0201a3d59b1a24 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -208,11 +208,15 @@ xfs_symlink_local_to_remote(
  */
 xfs_failaddr_t
 xfs_symlink_shortform_verify(
-	void			*sfp,
-	int64_t			size)
+	struct xfs_inode	*ip)
 {
+	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
+	char			*sfp = (char *)ifp->if_data;
+	int			size = ifp->if_bytes;
 	char			*endp = sfp + size;
 
+	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
+
 	/*
 	 * Zero length symlinks should never occur in memory as they are
 	 * never allowed to exist on disk.
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index b87c2214623383..fab4015f6a9506 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1024,6 +1024,30 @@ xrep_dinode_bad_metabt_fork(
 	return false;
 }
 
+static xfs_failaddr_t
+xrep_symlink_shortform_verify(
+	void			*sfp,
+	int64_t			size)
+{
+	/*
+	 * Zero length symlinks should never occur in memory as they are
+	 * never allowed to exist on disk.
+	 */
+	if (!size)
+		return __this_address;
+
+	/* No negative sizes or overly long symlink targets. */
+	if (size < 0 || size > XFS_SYMLINK_MAXLEN)
+		return __this_address;
+
+	/* No NULLs in the target either. */
+	if (memchr(sfp, 0, size))
+		return __this_address;
+
+	/* ondisk symlink target isn't null terminated, unlike incore */
+	return NULL;
+}
+
 /*
  * Check the data fork for things that will fail the ifork verifiers or the
  * ifork formatters.
@@ -1099,7 +1123,7 @@ xrep_dinode_check_dfork(
 			return true;
 		/* symlink structure must pass verification. */
 		if (S_ISLNK(mode) &&
-		    xfs_symlink_shortform_verify(dfork_ptr, data_size) != NULL)
+		    xrep_symlink_shortform_verify(dfork_ptr, data_size) != NULL)
 			return true;
 		break;
 	case XFS_DINODE_FMT_EXTENTS:

Re: [PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Dave Chinner]

On Sun, Sep 20, 2026 at 11:16:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that online repair of a broken symlink file could fail
> unnecessarily if a local-format symlink target isn't null terminated.
> The ondisk target isn't required to be null terminated, but repair
> enforces that anyway because it uses the validator for the incore
> symlink target.  (The incore buffer is always null-terminated).  Fix
> this by reverting the changes to xfs_symlink_shortform_verify and adding
> an ondisk-specific helper in inode_repair.c.
> 
> 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/libxfs/xfs_symlink_remote.h |    2 +-
>  fs/xfs/libxfs/xfs_inode_fork.c     |    4 +---
>  fs/xfs/libxfs/xfs_symlink_remote.c |    8 ++++++--
>  fs/xfs/scrub/inode_repair.c        |   26 +++++++++++++++++++++++++-
>  4 files changed, 33 insertions(+), 7 deletions(-)

Just tripped over this independently looking at local format symlink
verification for inode log item recovery. Fix looks good.

Reviewed-by: Dave Chinner <dgc@kernel.org>

-- 
Dave Chinner
dgc@kernel.org

[PATCH 08/14] xfs: fix maximum atomic cow length computation

[Darrick J. Wong]

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

LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
to catch this possibility because maybe the filesystem doesn't support
software atomic writes at all.

Cc: <stable@vger.kernel.org> # v6.16
Fixes: 0c438dcc31504b ("xfs: add xfs_calc_atomic_write_unit_max()")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_reflink.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 4801361366359b..489ecd4cf10aff 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1081,6 +1081,8 @@ xfs_extlen_t
 xfs_reflink_max_atomic_cow(
 	struct xfs_mount	*mp)
 {
+	xfs_extlen_t		fsb;
+
 	/* We cannot do any atomic writes without out of place writes. */
 	if (!xfs_can_sw_atomic_write(mp))
 		return 0;
@@ -1089,7 +1091,10 @@ xfs_reflink_max_atomic_cow(
 	 * Atomic write limits must always be a power-of-2, according to
 	 * generic_atomic_write_valid.
 	 */
-	return rounddown_pow_of_two(xfs_calc_max_atomic_write_fsblocks(mp));
+	fsb = xfs_calc_max_atomic_write_fsblocks(mp);
+	if (!fsb)
+		return 0;
+	return rounddown_pow_of_two(fsb);
 }
 
 /*

Re: [PATCH 08/14] xfs: fix maximum atomic cow length computation

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
> to catch this possibility because maybe the filesystem doesn't support
> software atomic writes at all.

Can this happen?  Shouldn't the xfs_can_sw_atomic_write() above guard
against this?

Re: [PATCH 08/14] xfs: fix maximum atomic cow length computation

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:21:21PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:04PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
> > to catch this possibility because maybe the filesystem doesn't support
> > software atomic writes at all.
> 
> Can this happen?  Shouldn't the xfs_can_sw_atomic_write() above guard
> against this?

Hmm.  Yes it can.  I think I'll drop this one.

--D

[PATCH 09/14] xfs: add missing healthmon trace strings

[Darrick J. Wong]

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

LOLLM noticed that we don't have trace strings for all known healthmon
types and domains.  Fix that.

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


diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 6aa379c2cf0cd1..0fc8927339b588 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -6030,32 +6030,54 @@ DEFINE_HEALTHMON_EVENT(xfs_healthmon_detach);
 DEFINE_HEALTHMON_EVENT(xfs_healthmon_report_unmount);
 
 #define XFS_HEALTHMON_TYPE_STRINGS \
+	{ XFS_HEALTHMON_RUNNING,	"run" }, \
 	{ XFS_HEALTHMON_LOST,		"lost" }, \
 	{ XFS_HEALTHMON_UNMOUNT,	"unmount" }, \
+	{ XFS_HEALTHMON_SHUTDOWN,	"shutdown" }, \
 	{ XFS_HEALTHMON_SICK,		"sick" }, \
 	{ XFS_HEALTHMON_CORRUPT,	"corrupt" }, \
 	{ XFS_HEALTHMON_HEALTHY,	"healthy" }, \
-	{ XFS_HEALTHMON_SHUTDOWN,	"shutdown" }
+	{ XFS_HEALTHMON_MEDIA_ERROR,	"media" }, \
+	{ XFS_HEALTHMON_BUFREAD,	"bufread" }, \
+	{ XFS_HEALTHMON_BUFWRITE,	"bufwrite" }, \
+	{ XFS_HEALTHMON_DIOREAD,	"dioread" }, \
+	{ XFS_HEALTHMON_DIOWRITE,	"diowrite" }, \
+	{ XFS_HEALTHMON_DATALOST,	"datalost" }
 
 #define XFS_HEALTHMON_DOMAIN_STRINGS \
 	{ XFS_HEALTHMON_MOUNT,		"mount" }, \
 	{ XFS_HEALTHMON_FS,		"fs" }, \
 	{ XFS_HEALTHMON_AG,		"ag" }, \
 	{ XFS_HEALTHMON_INODE,		"inode" }, \
-	{ XFS_HEALTHMON_RTGROUP,	"rtgroup" }
+	{ XFS_HEALTHMON_RTGROUP,	"rtgroup" }, \
+	{ XFS_HEALTHMON_DATADEV,	"datadev" }, \
+	{ XFS_HEALTHMON_RTDEV,		"rtdev" }, \
+	{ XFS_HEALTHMON_LOGDEV,		"logdev" }, \
+	{ XFS_HEALTHMON_FILERANGE,	"filerange" }
 
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_RUNNING);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_LOST);
-TRACE_DEFINE_ENUM(XFS_HEALTHMON_SHUTDOWN);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_UNMOUNT);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_SHUTDOWN);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_SICK);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_CORRUPT);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_HEALTHY);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_MEDIA_ERROR);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_BUFREAD);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_BUFWRITE);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_DIOREAD);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_DIOWRITE);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_DATALOST);
 
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_MOUNT);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_FS);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_AG);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_INODE);
 TRACE_DEFINE_ENUM(XFS_HEALTHMON_RTGROUP);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_DATADEV);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_RTDEV);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_LOGDEV);
+TRACE_DEFINE_ENUM(XFS_HEALTHMON_FILERANGE);
 
 DECLARE_EVENT_CLASS(xfs_healthmon_event_class,
 	TP_PROTO(const struct xfs_healthmon *hm,

Re: [PATCH 09/14] xfs: add missing healthmon trace strings

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:19PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we don't have trace strings for all known healthmon
> types and domains.  Fix that.

Looks good:

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

(proably another 7.4 candidate..)

[PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Darrick J. Wong]

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

LOLLM points out that if an array element crosses a folio boundary,
xfile_get_folio returns a NULL folio pointer.  If this happens,
si->folio is also set to NULL, and calling folio_pos/folio_address will
just crash the kernel.  Teach this function to handle this condition by
falling back to reading the array element into scratchpad memory.

Cc: <stable@vger.kernel.org> # v6.6
Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index 2ce24bfe4c0fab..30a58e9d4378e4 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -830,22 +830,24 @@ xfarray_sort_scan(
 			return PTR_ERR(folio);
 		si->folio = folio;
 
-		si->first_folio_idx = xfarray_idx(si->array,
-				folio_pos(si->folio) + si->array->obj_size - 1);
+		if (si->folio) {
+			si->first_folio_idx = xfarray_idx(si->array,
+					folio_pos(si->folio) + si->array->obj_size - 1);
 
-		next_pos = folio_next_pos(si->folio);
-		si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
-		if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
-			si->last_folio_idx--;
+			next_pos = folio_next_pos(si->folio);
+			si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
+			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
+				si->last_folio_idx--;
 
-		trace_xfarray_sort_scan(si, idx);
+			trace_xfarray_sort_scan(si, idx);
+		}
 	}
 
 	/*
 	 * If this folio still doesn't cover the desired element, it must cross
 	 * a folio boundary.  Read into the scratchpad and we're done.
 	 */
-	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
+	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {
 		void		*temp = xfarray_scratch(si->array);
 
 		error = xfile_load(si->array->xfile, temp, si->array->obj_size,

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:35PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that if an array element crosses a folio boundary,
> xfile_get_folio returns a NULL folio pointer.  If this happens,
> si->folio is also set to NULL, and calling folio_pos/folio_address will
> just crash the kernel.  Teach this function to handle this condition by
> falling back to reading the array element into scratchpad memory.
> 
> Cc: <stable@vger.kernel.org> # v6.6
> Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> index 2ce24bfe4c0fab..30a58e9d4378e4 100644
> --- a/fs/xfs/scrub/xfarray.c
> +++ b/fs/xfs/scrub/xfarray.c
> @@ -830,22 +830,24 @@ xfarray_sort_scan(
>  			return PTR_ERR(folio);
>  		si->folio = folio;
>  
> -		si->first_folio_idx = xfarray_idx(si->array,
> -				folio_pos(si->folio) + si->array->obj_size - 1);
> +		if (si->folio) {
> +			si->first_folio_idx = xfarray_idx(si->array,
> +					folio_pos(si->folio) + si->array->obj_size - 1);

Overly long line.

> +			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)

Another one.

> +	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {

And one more.

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:22:38PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:35PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM points out that if an array element crosses a folio boundary,
> > xfile_get_folio returns a NULL folio pointer.  If this happens,
> > si->folio is also set to NULL, and calling folio_pos/folio_address will
> > just crash the kernel.  Teach this function to handle this condition by
> > falling back to reading the array element into scratchpad memory.
> > 
> > Cc: <stable@vger.kernel.org> # v6.6
> > Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> > index 2ce24bfe4c0fab..30a58e9d4378e4 100644
> > --- a/fs/xfs/scrub/xfarray.c
> > +++ b/fs/xfs/scrub/xfarray.c
> > @@ -830,22 +830,24 @@ xfarray_sort_scan(
> >  			return PTR_ERR(folio);
> >  		si->folio = folio;
> >  
> > -		si->first_folio_idx = xfarray_idx(si->array,
> > -				folio_pos(si->folio) + si->array->obj_size - 1);
> > +		if (si->folio) {
> > +			si->first_folio_idx = xfarray_idx(si->array,
> > +					folio_pos(si->folio) + si->array->obj_size - 1);
> 
> Overly long line.
> 
> > +			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
> 
> Another one.
> 
> > +	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {
> 
> And one more.

On second glance, I think a cleaner way to fix this is to move the logic
that loads the folio, computes the new {first,last}_folio_idx, and
validates them into a new helper:

static int
xfarray_sort_load_folio(
	struct xfarray_sortinfo	*si,
	xfarray_idx_t		idx,
	loff_t			idx_pos)
{
	struct folio		*folio;
	loff_t			next_pos;

	folio = xfile_get_folio(si->array->xfile, idx_pos, si->array->obj_size,
			XFILE_ALLOC);
	if (IS_ERR(folio))
		return PTR_ERR(folio);
	si->folio = folio;

	/* No folio?  Get the caller to read into the scratchpad. */
	if (!si->folio)
		return 0;

	si->first_folio_idx = xfarray_idx(si->array,
			folio_pos(si->folio) + si->array->obj_size - 1);

	next_pos = folio_next_pos(si->folio);
	si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
	if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
		si->last_folio_idx--;

	/*
	 * If this folio still doesn't cover the desired element, it must cross
	 * a folio boundary.  Get the caller to read into the scratchpad.
	 */
	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
		xfarray_sort_scan_done(si);
		return 0;
	}

	trace_xfarray_sort_scan(si, idx);
	return 0;
}

--D

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Christoph Hellwig]

On Tue, Sep 22, 2026 at 10:53:13AM -0700, Darrick J. Wong wrote:
> On second glance, I think a cleaner way to fix this is to move the logic
> that loads the folio, computes the new {first,last}_folio_idx, and
> validates them into a new helper:

Heh, I was about to suggest a new helper, but didn't feel like I'd
want to cause you too much work..

[PATCH 11/14] xfarray: don't allow users to unset in the middle of an array

[Darrick J. Wong]

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

Now that we've merged online fsck, the only user of xfarray_unset is the
free space btree repair code, and it only needs to be able to remove
records from the end of the array.  Let's remove all the code that
handles "unset" array elements that are not at the end, because we can
just reduce the array element count.

Remove the "store anywhere" function because it was only ever used by
the callers who used unset to remove elements in the middle of the
array.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/xfarray.h                             |    6 -
 .../filesystems/xfs/xfs-online-fsck-design.rst     |   13 +--
 fs/xfs/scrub/alloc_repair.c                        |    2 
 fs/xfs/scrub/xfarray.c                             |   92 ++------------------
 4 files changed, 14 insertions(+), 99 deletions(-)


diff --git a/fs/xfs/scrub/xfarray.h b/fs/xfs/scrub/xfarray.h
index 5eeeeed13ae24a..d55225c7885b25 100644
--- a/fs/xfs/scrub/xfarray.h
+++ b/fs/xfs/scrub/xfarray.h
@@ -27,9 +27,6 @@ struct xfarray {
 	/* Maximum possible array size. */
 	xfarray_idx_t	max_nr;
 
-	/* Number of unset slots in the array below @nr. */
-	uint64_t	unset_slots;
-
 	/* Size of an array element. */
 	size_t		obj_size;
 
@@ -41,9 +38,8 @@ int xfarray_create(const char *descr, unsigned long long required_capacity,
 		size_t obj_size, struct xfarray **arrayp);
 void xfarray_destroy(struct xfarray *array);
 int xfarray_load(struct xfarray *array, xfarray_idx_t idx, void *ptr);
-int xfarray_unset(struct xfarray *array, xfarray_idx_t idx);
+int xfarray_trim(struct xfarray *array, unsigned long long nr);
 int xfarray_store(struct xfarray *array, xfarray_idx_t idx, const void *ptr);
-int xfarray_store_anywhere(struct xfarray *array, const void *ptr);
 bool xfarray_element_is_null(struct xfarray *array, const void *ptr);
 void xfarray_truncate(struct xfarray *array);
 unsigned long long xfarray_bytes(struct xfarray *array);
diff --git a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
index 3d9233f403dbb1..14767ce9fad43f 100644
--- a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
+++ b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
@@ -1973,8 +1973,7 @@ provide loading and storing of array elements at arbitrary array indices.
 Gaps are defined to be null records, and null records are defined to be a
 sequence of all zero bytes.
 Null records are detected by calling ``xfarray_element_is_null``.
-They are created either by calling ``xfarray_unset`` to null out an existing
-record or by never storing anything to an array index.
+They are created by never storing anything to an array index.
 
 The second type of caller handles records that are not indexed by position
 and do not require multiple updates to a record.
@@ -1991,9 +1990,7 @@ The typical use case here is constructing space extent reference counts from
 reverse mapping information.
 Records can be put in the bag in any order, they can be removed from the bag
 at any time, and uniqueness of records is left to callers.
-The ``xfarray_store_anywhere`` function is used to insert a record in any
-null record slot in the bag; and the ``xfarray_unset`` function removes a
-record from the bag.
+Note: Bags are now implemented with in-memory btrees for faster access.
 
 Iterating Array Elements
 ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -2643,11 +2640,7 @@ generate refcount information from reverse mapping records.
       refcount record associating the block number range that we just walked to
       the size of the bag.
 
-The bag-like structure in this case is a type 2 xfarray as discussed in the
-:ref:`xfarray access patterns<xfarray_access_patterns>` section.
-Reverse mappings are added to the bag using ``xfarray_store_anywhere`` and
-removed via ``xfarray_unset``.
-Bag members are examined through ``xfarray_iter`` loops.
+The bag-like structure in this case is an in-memory btree.
 
 Case Study: Rebuilding File Fork Mapping Indices
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 95e318e4f3a6c7..b37b80af52a03b 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -517,7 +517,7 @@ xrep_abt_reserve_space(
 		 * records (but doesn't break the sorting order), so we must
 		 * go around the loop once more to re-run _bload_init.
 		 */
-		error = xfarray_unset(ra->free_records, record_nr);
+		error = xfarray_trim(ra->free_records, 1);
 		if (error)
 			break;
 		ra->nr_real_records--;
diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index 30a58e9d4378e4..ee3f0bf87433f1 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -149,9 +149,6 @@ xfarray_is_unset(
 	void		*temp = xfarray_scratch(array);
 	int		error;
 
-	if (array->unset_slots == 0)
-		return false;
-
 	error = xfile_load(array->xfile, temp, array->obj_size, pos);
 	if (!error && xfarray_element_is_null(array, temp))
 		return true;
@@ -159,36 +156,18 @@ xfarray_is_unset(
 	return false;
 }
 
-/*
- * Unset an array element.  If @idx is the last element in the array, the
- * array will be truncated.  Otherwise, the entry will be zeroed.
- */
+/* Remove the elements at the end of an array. */
 int
-xfarray_unset(
-	struct xfarray	*array,
-	xfarray_idx_t	idx)
+xfarray_trim(
+	struct xfarray		*array,
+	unsigned long long	nr)
 {
-	void		*temp = xfarray_scratch(array);
-	loff_t		pos = xfarray_pos(array, idx);
-	int		error;
-
-	if (idx >= array->nr)
+	if (nr > array->nr)
 		return -ENODATA;
 
-	if (idx == array->nr - 1) {
-		array->nr--;
-		return 0;
-	}
-
-	if (xfarray_is_unset(array, pos))
-		return 0;
-
-	memset(temp, 0, array->obj_size);
-	error = xfile_store(array->xfile, temp, array->obj_size, pos);
-	if (error)
-		return error;
-
-	array->unset_slots++;
+	array->nr -= nr;
+	xfile_discard(array->xfile, xfarray_pos(array, array->nr),
+			MAX_LFS_FILESIZE);
 	return 0;
 }
 
@@ -227,43 +206,6 @@ xfarray_element_is_null(
 	return !memchr_inv(ptr, 0, array->obj_size);
 }
 
-/*
- * Store an element anywhere in the array that is unset.  If there are no
- * unset slots, append the element to the array.
- */
-int
-xfarray_store_anywhere(
-	struct xfarray	*array,
-	const void	*ptr)
-{
-	void		*temp = xfarray_scratch(array);
-	loff_t		endpos = xfarray_pos(array, array->nr);
-	loff_t		pos;
-	int		error;
-
-	/* Find an unset slot to put it in. */
-	for (pos = 0;
-	     pos < endpos && array->unset_slots > 0;
-	     pos += array->obj_size) {
-		error = xfile_load(array->xfile, temp, array->obj_size,
-				pos);
-		if (error || !xfarray_element_is_null(array, temp))
-			continue;
-
-		error = xfile_store(array->xfile, ptr, array->obj_size,
-				pos);
-		if (error)
-			return error;
-
-		array->unset_slots--;
-		return 0;
-	}
-
-	/* No unset slots found; attach it on the end. */
-	array->unset_slots = 0;
-	return xfarray_append(array, ptr);
-}
-
 /* Return length of array. */
 uint64_t
 xfarray_length(
@@ -677,26 +619,10 @@ xfarray_qsort_pivot(
 
 	/* Load the selected xfarray records into the pivot array. */
 	for (i = 0; i < XFARRAY_QSORT_PIVOT_NR; i++) {
-		xfarray_idx_t	idx;
-
 		recp = xfarray_pivot_array_rec(parray, pivot_rec_sz, i);
 		idxp = xfarray_pivot_array_idx(parray, pivot_rec_sz, i);
 
-		/* No unset records; load directly into the array. */
-		if (likely(si->array->unset_slots == 0)) {
-			error = xfarray_sort_load(si, *idxp, recp);
-			if (error)
-				return error;
-			continue;
-		}
-
-		/*
-		 * Load non-null records into the scratchpad without changing
-		 * the xfarray_idx_t in the pivot array.
-		 */
-		idx = *idxp;
-		xfarray_sort_bump_loads(si);
-		error = xfarray_load_next(si->array, &idx, recp);
+		error = xfarray_sort_load(si, *idxp, recp);
 		if (error)
 			return error;
 	}

Re: [PATCH 11/14] xfarray: don't allow users to unset in the middle of an array

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:51PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Now that we've merged online fsck, the only user of xfarray_unset is the
> free space btree repair code, and it only needs to be able to remove
> records from the end of the array.

I don't get the " Now that we've merged" part.  Without that we would
not have any user, right?

Otherwise looks good:

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

Re: [PATCH 11/14] xfarray: don't allow users to unset in the middle of an array

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:23:56PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:51PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Now that we've merged online fsck, the only user of xfarray_unset is the
> > free space btree repair code, and it only needs to be able to remove
> > records from the end of the array.
> 
> I don't get the " Now that we've merged" part.  Without that we would
> not have any user, right?

Right.  That could have said "Now that we've merged online repair and
scraped out some clunky parts of the original online check code..."

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

Thanks!

--D

Re: [PATCH 11/14] xfarray: don't allow users to unset in the middle of an array

[Darrick J. Wong]

On Sun, Sep 20, 2026 at 11:17:51PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Now that we've merged online fsck, the only user of xfarray_unset is the
> free space btree repair code, and it only needs to be able to remove
> records from the end of the array.  Let's remove all the code that
> handles "unset" array elements that are not at the end, because we can
> just reduce the array element count.
> 
> Remove the "store anywhere" function because it was only ever used by
> the callers who used unset to remove elements in the middle of the
> array.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/scrub/xfarray.h                             |    6 -
>  .../filesystems/xfs/xfs-online-fsck-design.rst     |   13 +--
>  fs/xfs/scrub/alloc_repair.c                        |    2 
>  fs/xfs/scrub/xfarray.c                             |   92 ++------------------
>  4 files changed, 14 insertions(+), 99 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/xfarray.h b/fs/xfs/scrub/xfarray.h
> index 5eeeeed13ae24a..d55225c7885b25 100644
> --- a/fs/xfs/scrub/xfarray.h
> +++ b/fs/xfs/scrub/xfarray.h
> @@ -27,9 +27,6 @@ struct xfarray {
>  	/* Maximum possible array size. */
>  	xfarray_idx_t	max_nr;
>  
> -	/* Number of unset slots in the array below @nr. */
> -	uint64_t	unset_slots;
> -
>  	/* Size of an array element. */
>  	size_t		obj_size;
>  
> @@ -41,9 +38,8 @@ int xfarray_create(const char *descr, unsigned long long required_capacity,
>  		size_t obj_size, struct xfarray **arrayp);
>  void xfarray_destroy(struct xfarray *array);
>  int xfarray_load(struct xfarray *array, xfarray_idx_t idx, void *ptr);
> -int xfarray_unset(struct xfarray *array, xfarray_idx_t idx);
> +int xfarray_trim(struct xfarray *array, unsigned long long nr);
>  int xfarray_store(struct xfarray *array, xfarray_idx_t idx, const void *ptr);
> -int xfarray_store_anywhere(struct xfarray *array, const void *ptr);
>  bool xfarray_element_is_null(struct xfarray *array, const void *ptr);
>  void xfarray_truncate(struct xfarray *array);
>  unsigned long long xfarray_bytes(struct xfarray *array);
> diff --git a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
> index 3d9233f403dbb1..14767ce9fad43f 100644
> --- a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
> +++ b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst
> @@ -1973,8 +1973,7 @@ provide loading and storing of array elements at arbitrary array indices.
>  Gaps are defined to be null records, and null records are defined to be a
>  sequence of all zero bytes.
>  Null records are detected by calling ``xfarray_element_is_null``.
> -They are created either by calling ``xfarray_unset`` to null out an existing
> -record or by never storing anything to an array index.
> +They are created by never storing anything to an array index.
>  
>  The second type of caller handles records that are not indexed by position
>  and do not require multiple updates to a record.
> @@ -1991,9 +1990,7 @@ The typical use case here is constructing space extent reference counts from
>  reverse mapping information.
>  Records can be put in the bag in any order, they can be removed from the bag
>  at any time, and uniqueness of records is left to callers.
> -The ``xfarray_store_anywhere`` function is used to insert a record in any
> -null record slot in the bag; and the ``xfarray_unset`` function removes a
> -record from the bag.
> +Note: Bags are now implemented with in-memory btrees for faster access.
>  
>  Iterating Array Elements
>  ^^^^^^^^^^^^^^^^^^^^^^^^
> @@ -2643,11 +2640,7 @@ generate refcount information from reverse mapping records.
>        refcount record associating the block number range that we just walked to
>        the size of the bag.
>  
> -The bag-like structure in this case is a type 2 xfarray as discussed in the
> -:ref:`xfarray access patterns<xfarray_access_patterns>` section.
> -Reverse mappings are added to the bag using ``xfarray_store_anywhere`` and
> -removed via ``xfarray_unset``.
> -Bag members are examined through ``xfarray_iter`` loops.
> +The bag-like structure in this case is an in-memory btree.
>  
>  Case Study: Rebuilding File Fork Mapping Indices
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
> index 95e318e4f3a6c7..b37b80af52a03b 100644
> --- a/fs/xfs/scrub/alloc_repair.c
> +++ b/fs/xfs/scrub/alloc_repair.c
> @@ -517,7 +517,7 @@ xrep_abt_reserve_space(
>  		 * records (but doesn't break the sorting order), so we must
>  		 * go around the loop once more to re-run _bload_init.
>  		 */
> -		error = xfarray_unset(ra->free_records, record_nr);
> +		error = xfarray_trim(ra->free_records, 1);
>  		if (error)
>  			break;
>  		ra->nr_real_records--;
> diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> index 30a58e9d4378e4..ee3f0bf87433f1 100644
> --- a/fs/xfs/scrub/xfarray.c
> +++ b/fs/xfs/scrub/xfarray.c
> @@ -149,9 +149,6 @@ xfarray_is_unset(

Shashiko writes:

"Is this function now entirely dead code? The patch removes xfarray_unset(),
which was the only caller of the xfarray_is_unset() static inline helper.
Should the entire xfarray_is_unset() function be removed?"

Yes.

>  	void		*temp = xfarray_scratch(array);
>  	int		error;
>  
> -	if (array->unset_slots == 0)
> -		return false;
> -
>  	error = xfile_load(array->xfile, temp, array->obj_size, pos);
>  	if (!error && xfarray_element_is_null(array, temp))
>  		return true;
> @@ -159,36 +156,18 @@ xfarray_is_unset(
>  	return false;
>  }
>  
> -/*
> - * Unset an array element.  If @idx is the last element in the array, the
> - * array will be truncated.  Otherwise, the entry will be zeroed.
> - */
> +/* Remove the elements at the end of an array. */
>  int
> -xfarray_unset(
> -	struct xfarray	*array,
> -	xfarray_idx_t	idx)
> +xfarray_trim(
> +	struct xfarray		*array,
> +	unsigned long long	nr)
>  {
> -	void		*temp = xfarray_scratch(array);
> -	loff_t		pos = xfarray_pos(array, idx);
> -	int		error;
> -
> -	if (idx >= array->nr)
> +	if (nr > array->nr)
>  		return -ENODATA;
>  
> -	if (idx == array->nr - 1) {
> -		array->nr--;
> -		return 0;
> -	}
> -
> -	if (xfarray_is_unset(array, pos))
> -		return 0;
> -
> -	memset(temp, 0, array->obj_size);
> -	error = xfile_store(array->xfile, temp, array->obj_size, pos);
> -	if (error)
> -		return error;
> -
> -	array->unset_slots++;
> +	array->nr -= nr;
> +	xfile_discard(array->xfile, xfarray_pos(array, array->nr),
> +			MAX_LFS_FILESIZE);

"Can passing MAX_LFS_FILESIZE here cause a silent memory leak on 32-bit
architectures?
In xfarray_trim(), MAX_LFS_FILESIZE is passed as the count to
xfile_discard(), which passes it directly to shmem_truncate_range().
On 32-bit systems, MAX_LFS_FILESIZE is 0xFFFFFFFF000. When pos >= 0x1000
(4KB), the lend calculation (pos + count - 1) inside shmem_undo_range()
evaluates to at least 0x100000000000.
The truncation endpoint is calculated as:
end = (lend + 1) >> PAGE_SHIFT;
This results in an end value of 0x100000000. Since pgoff_t is a 32-bit
unsigned long on 32-bit systems, this truncates to exactly 0.
Because start would be at least 1, the loop "while (index < end)" evaluates
1 < 0, which is false. The loop terminates immediately without discarding any
pages, causing a memory leak during XFS free space btree repair.
Should count be clamped, such as passing MAX_LFS_FILESIZE - pos?"

Yes.

--D

>  	return 0;
>  }
>  
> @@ -227,43 +206,6 @@ xfarray_element_is_null(
>  	return !memchr_inv(ptr, 0, array->obj_size);
>  }
>  
> -/*
> - * Store an element anywhere in the array that is unset.  If there are no
> - * unset slots, append the element to the array.
> - */
> -int
> -xfarray_store_anywhere(
> -	struct xfarray	*array,
> -	const void	*ptr)
> -{
> -	void		*temp = xfarray_scratch(array);
> -	loff_t		endpos = xfarray_pos(array, array->nr);
> -	loff_t		pos;
> -	int		error;
> -
> -	/* Find an unset slot to put it in. */
> -	for (pos = 0;
> -	     pos < endpos && array->unset_slots > 0;
> -	     pos += array->obj_size) {
> -		error = xfile_load(array->xfile, temp, array->obj_size,
> -				pos);
> -		if (error || !xfarray_element_is_null(array, temp))
> -			continue;
> -
> -		error = xfile_store(array->xfile, ptr, array->obj_size,
> -				pos);
> -		if (error)
> -			return error;
> -
> -		array->unset_slots--;
> -		return 0;
> -	}
> -
> -	/* No unset slots found; attach it on the end. */
> -	array->unset_slots = 0;
> -	return xfarray_append(array, ptr);
> -}
> -
>  /* Return length of array. */
>  uint64_t
>  xfarray_length(
> @@ -677,26 +619,10 @@ xfarray_qsort_pivot(
>  
>  	/* Load the selected xfarray records into the pivot array. */
>  	for (i = 0; i < XFARRAY_QSORT_PIVOT_NR; i++) {
> -		xfarray_idx_t	idx;
> -
>  		recp = xfarray_pivot_array_rec(parray, pivot_rec_sz, i);
>  		idxp = xfarray_pivot_array_idx(parray, pivot_rec_sz, i);
>  
> -		/* No unset records; load directly into the array. */
> -		if (likely(si->array->unset_slots == 0)) {
> -			error = xfarray_sort_load(si, *idxp, recp);
> -			if (error)
> -				return error;
> -			continue;
> -		}
> -
> -		/*
> -		 * Load non-null records into the scratchpad without changing
> -		 * the xfarray_idx_t in the pivot array.
> -		 */
> -		idx = *idxp;
> -		xfarray_sort_bump_loads(si);
> -		error = xfarray_load_next(si->array, &idx, recp);
> +		error = xfarray_sort_load(si, *idxp, recp);
>  		if (error)
>  			return error;
>  	}
> 
> 

[PATCH 12/14] xfs: don't allow sorting sparse arrays

[Darrick J. Wong]

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

Now that we've reduced the functionality of xfarray_unset, let's add a
new safeguard: no sorting of xfarrays with sparse holes in them.  It's
not clear what that even means, and nobody actually does this, so we're
really just eliminating subtle logic bombs.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/xfarray.h |    3 +++
 fs/xfs/scrub/xfarray.c |   11 +++++++++++
 2 files changed, 14 insertions(+)


diff --git a/fs/xfs/scrub/xfarray.h b/fs/xfs/scrub/xfarray.h
index d55225c7885b25..05ff65b09fcf41 100644
--- a/fs/xfs/scrub/xfarray.h
+++ b/fs/xfs/scrub/xfarray.h
@@ -32,6 +32,9 @@ struct xfarray {
 
 	/* log2 of array element size, if possible. */
 	int		obj_size_log;
+
+	/* Might there be sparse holes in this array? */
+	bool		possibly_sparse;
 };
 
 int xfarray_create(const char *descr, unsigned long long required_capacity,
diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index ee3f0bf87433f1..78274ad53097e1 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -193,6 +193,8 @@ xfarray_store(
 	if (ret)
 		return ret;
 
+	if (idx > array->nr)
+		array->possibly_sparse = true;
 	array->nr = max(array->nr, idx + 1);
 	return 0;
 }
@@ -844,6 +846,14 @@ xfarray_sort(
 		return 0;
 	if (array->nr >= QSORT_MAX_RECS)
 		return -E2BIG;
+	if (array->possibly_sparse) {
+		/*
+		 * What does it mean to sort an array with holes in it?
+		 * Currently none of the users need this ability.
+		 */
+		ASSERT(array->possibly_sparse);
+		return -EINVAL;
+	}
 
 	error = xfarray_sortinfo_alloc(array, cmp_fn, flags, &si);
 	if (error)
@@ -997,4 +1007,5 @@ xfarray_truncate(
 {
 	xfile_discard(array->xfile, 0, MAX_LFS_FILESIZE);
 	array->nr = 0;
+	array->possibly_sparse = false;
 }

Re: [PATCH 12/14] xfs: don't allow sorting sparse arrays

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 12/14] xfs: don't allow sorting sparse arrays

[Darrick J. Wong]

On Sun, Sep 20, 2026 at 11:18:06PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Now that we've reduced the functionality of xfarray_unset, let's add a
> new safeguard: no sorting of xfarrays with sparse holes in them.  It's
> not clear what that even means, and nobody actually does this, so we're
> really just eliminating subtle logic bombs.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/scrub/xfarray.h |    3 +++
>  fs/xfs/scrub/xfarray.c |   11 +++++++++++
>  2 files changed, 14 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/xfarray.h b/fs/xfs/scrub/xfarray.h
> index d55225c7885b25..05ff65b09fcf41 100644
> --- a/fs/xfs/scrub/xfarray.h
> +++ b/fs/xfs/scrub/xfarray.h
> @@ -32,6 +32,9 @@ struct xfarray {
>  
>  	/* log2 of array element size, if possible. */
>  	int		obj_size_log;
> +
> +	/* Might there be sparse holes in this array? */
> +	bool		possibly_sparse;
>  };
>  
>  int xfarray_create(const char *descr, unsigned long long required_capacity,
> diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> index ee3f0bf87433f1..78274ad53097e1 100644
> --- a/fs/xfs/scrub/xfarray.c
> +++ b/fs/xfs/scrub/xfarray.c
> @@ -193,6 +193,8 @@ xfarray_store(
>  	if (ret)
>  		return ret;
>  
> +	if (idx > array->nr)
> +		array->possibly_sparse = true;
>  	array->nr = max(array->nr, idx + 1);
>  	return 0;
>  }
> @@ -844,6 +846,14 @@ xfarray_sort(
>  		return 0;
>  	if (array->nr >= QSORT_MAX_RECS)
>  		return -E2BIG;
> +	if (array->possibly_sparse) {
> +		/*
> +		 * What does it mean to sort an array with holes in it?
> +		 * Currently none of the users need this ability.
> +		 */
> +		ASSERT(array->possibly_sparse);

Shashiko says:

"Will this assertion always evaluate to true?
Since it is located inside the if (array->possibly_sparse) block, it acts
as a no-op instead of catching sparse array sorting in debug builds. Should
this be ASSERT(!array->possibly_sparse) before the condition, or ASSERT(0)
inside it?"

Yes.  The logic is backwards.  Will fix.

> +		return -EINVAL;
> +	}
>  
>  	error = xfarray_sortinfo_alloc(array, cmp_fn, flags, &si);
>  	if (error)
> @@ -997,4 +1007,5 @@ xfarray_truncate(
>  {
>  	xfile_discard(array->xfile, 0, MAX_LFS_FILESIZE);
>  	array->nr = 0;
> +	array->possibly_sparse = false;
>  }

"Since the new possibly_sparse flag is cleared here when the array is
truncated to 0 elements, does xfarray_trim() need a similar update?
Looking at fs/xfs/scrub/xfarray.c:xfarray_trim(), it can also reduce
the array to 0 elements:
int
xfarray_trim(
	struct xfarray		*array,
	unsigned long long	nr)
{
	if (nr > array->nr)
		return -ENODATA;
	array->nr -= nr;
	xfile_discard(array->xfile, xfarray_pos(array, array->nr),
			MAX_LFS_FILESIZE);
	return 0;
}
If an array is sparse, completely trimmed to 0 elements via
xfarray_trim(array, array->nr), and then populated densely, it appears
the possibly_sparse flag would remain true. Could this cause future
calls to xfarray_sort() on the dense array to fail with a spurious
-EINVAL?"

Yes, the trim function needs it too.  Nobody does that, but I agree not
to leave a logic bomb.

--D

[PATCH 13/14] xfs: simply the free space btree repair code

[Darrick J. Wong]

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

Now that the xfarray always knows how many valid records there are
stored inside of it, get rid of the shadow variable.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/alloc_repair.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index b37b80af52a03b..bdd58738777072 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -108,9 +108,6 @@ struct xrep_abt {
 
 	struct xfs_scrub	*sc;
 
-	/* Number of non-null records in @free_records. */
-	uint64_t		nr_real_records;
-
 	/* get_records()'s position in the free space record array. */
 	xfarray_idx_t		array_cur;
 
@@ -403,7 +400,6 @@ xrep_abt_find_freespace(
 	if (error)
 		goto err_agfl;
 
-	ra->nr_real_records = xfarray_length(ra->free_records);
 err_agfl:
 	xfs_trans_brelse(sc->tp, agfl_bp);
 err:
@@ -446,15 +442,17 @@ xrep_abt_reserve_space(
 		uint64_t		required;
 		unsigned int		desired;
 		unsigned int		len;
+		const uint64_t		nr_records =
+			xfarray_length(ra->free_records);
 
 		/* Compute how many blocks we'll need. */
 		error = xfs_btree_bload_compute_geometry(cnt_cur,
-				&ra->new_cntbt.bload, ra->nr_real_records);
+				&ra->new_cntbt.bload, nr_records);
 		if (error)
 			break;
 
 		error = xfs_btree_bload_compute_geometry(bno_cur,
-				&ra->new_bnobt.bload, ra->nr_real_records);
+				&ra->new_bnobt.bload, nr_records);
 		if (error)
 			break;
 
@@ -470,7 +468,7 @@ xrep_abt_reserve_space(
 		desired = required - allocated;
 
 		/* We need space but there's none left; bye! */
-		if (ra->nr_real_records == 0) {
+		if (nr_records == 0) {
 			error = -ENOSPC;
 			break;
 		}
@@ -520,7 +518,6 @@ xrep_abt_reserve_space(
 		error = xfarray_trim(ra->free_records, 1);
 		if (error)
 			break;
-		ra->nr_real_records--;
 		record_nr--;
 	} while (1);
 

Re: [PATCH 13/14] xfs: simply the free space btree repair code

[Christoph Hellwig]

Looks good:

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

[PATCH 14/14] xfarray: warn against sorting arrays with identical elements

[Darrick J. Wong]

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

LOLLM complains about a potential underflow here if xfarray_qsort_push
is called with lo==0.  However, this isn't possible in most cases
because filesystem metadata records cannot be identical.

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


diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index 78274ad53097e1..ab15a73b7d3b10 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -693,6 +693,18 @@ xfarray_qsort_push(
 		return -EFSCORRUPTED;
 	}
 
+	/*
+	 * Avoid the integer underflow below in (lo - 1).  This shouldn't
+	 * be possible because the pivot is the median of nine distinct
+	 * filesystem metadata records, so at least four records will be less
+	 * than the pivot, which means the pivot will not be in the low end of
+	 * the range by the time we get here.
+	 */
+	if (lo == 0) {
+		ASSERT(lo != 0);
+		return -EFSCORRUPTED;
+	}
+
 	si->max_stack_used = max_t(uint8_t, si->max_stack_used,
 					    si->stack_depth + 2);
 

Re: [PATCH 14/14] xfarray: warn against sorting arrays with identical elements

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:18:38PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains about a potential underflow here if xfarray_qsort_push
> is called with lo==0.  However, this isn't possible in most cases
> because filesystem metadata records cannot be identical.

Looks good:

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

Re: [PATCH 14/14] xfarray: warn against sorting arrays with identical elements

[Darrick J. Wong]

On Sun, Sep 20, 2026 at 11:18:38PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains about a potential underflow here if xfarray_qsort_push
> is called with lo==0.  However, this isn't possible in most cases
> because filesystem metadata records cannot be identical.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/xfarray.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> index 78274ad53097e1..ab15a73b7d3b10 100644
> --- a/fs/xfs/scrub/xfarray.c
> +++ b/fs/xfs/scrub/xfarray.c
> @@ -693,6 +693,18 @@ xfarray_qsort_push(
>  		return -EFSCORRUPTED;
>  	}
>  
> +	/*
> +	 * Avoid the integer underflow below in (lo - 1).  This shouldn't
> +	 * be possible because the pivot is the median of nine distinct
> +	 * filesystem metadata records, so at least four records will be less
> +	 * than the pivot, which means the pivot will not be in the low end of
> +	 * the range by the time we get here.
> +	 */
> +	if (lo == 0) {
> +		ASSERT(lo != 0);
> +		return -EFSCORRUPTED;
> +	}

Shashiko has a big long comment:

"Can this assertion be triggered if a corrupted filesystem actually contains
identical elements, causing a denial of service?
Looking at the partition loop in xfarray_sort(), it uses a >= 0 check:
fs/xfs/scrub/xfarray.c:xfarray_sort() {
    ...
			while (xfarray_sort_cmp(si, p, pivot) >= 0 && lo < hi) {
				hi--;
    ...
}
If the array contains duplicate elements, the comparison returns 0, and hi is
decremented down to lo. Then xfarray_qsort_push() is called with the
unmodified lo.
If lo is 0, won't this trigger the new assertion and crash the kernel?
Furthermore, if lo > 0, does this cause linear stack growth and another crash?
Because hi was decremented down to lo, the left partition size calculation
underflows:
fs/xfs/scrub/xfarray.c:xfarray_qsort_push() {
    ...
	si_hi[si->stack_depth++] = lo - 1;
	/*
	 * Always start with the smaller of the two partitions to keep the
	 * amount of recursion in check.
	 */
	if (si_hi[si->stack_depth]     - si_lo[si->stack_depth] >
	    si_hi[si->stack_depth - 1] - si_lo[si->stack_depth - 1]) {
    ...
}
The left partition size calculated via (lo - 1) - lo underflows to a very
large unsigned value, bypassing the size optimization.
Will the algorithm then process the N-1 right partition first, pushing empty
left partitions to the stack until it hits max_stack_depth and fails via
the stack overflow assertion?
fs/xfs/scrub/xfarray.c:xfarray_qsort_push() {
	/* Check for stack overflows */
	if (si->stack_depth >= si->max_stack_depth - 1) {
		ASSERT(si->stack_depth < si->max_stack_depth - 1);
		return -EFSCORRUPTED;
	}
}"

To which I say that xfs isn't supposed to store duplicate elements in a
metadata structure, so repair code should never create duplicate
elements in a staging array.  We're not going to exhaustively verify
that for every repair since most repair code is structured so that it
can't create duplicate elements, but we can at least bail out when we
detect the obvious signs of this.

--D

> +
>  	si->max_stack_used = max_t(uint8_t, si->max_stack_used,
>  					    si->stack_depth + 2);
>  
> 
>