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

[PATCHSET v2 1/2] 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...

v2: add RVB tags, integrate some review feedback from humans and LOLLM

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: 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                                 |    2 
 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                             |  201 ++++++++------------
 fs/xfs/xfs_dquot.c                                 |   20 +-
 fs/xfs/xfs_handle.c                                |    2 
 fs/xfs/xfs_qm.c                                    |    6 -
 fs/xfs/xfs_trans_dquot.c                           |    6 -
 21 files changed, 277 insertions(+), 216 deletions(-)

Unreviewed patches in this series:

[PATCHSET v2 1/2] xfs: LLM-inspired bug fixes, part 16
  [PATCH 09/13] xfarray: don't crash when sorting if array element

[PATCH 01/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_dquot.h               |    2 +-
 fs/xfs/scrub/quota_repair.c      |    5 +----
 fs/xfs/scrub/quotacheck_repair.c |    3 +--
 fs/xfs/xfs_dquot.c               |   20 +++++++++++++++-----
 fs/xfs/xfs_qm.c                  |    6 +-----
 fs/xfs/xfs_trans_dquot.c         |    6 +-----
 6 files changed, 20 insertions(+), 22 deletions(-)


diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index bbb824adca82ce..28c09704acc261 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -205,7 +205,7 @@ 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..6d22ead562da58 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;
@@ -223,6 +220,19 @@ xfs_qm_adjust_dqtimers(
 	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_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

[PATCH 02/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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..b8334edf380c7f 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)
+			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/13] 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.

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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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;
 

[PATCH 04/13] 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.

Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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__ */

[PATCH 05/13] 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.

Fixes: 7dba4a5fe1c5cd ("xfs: extend transaction reservations for parent attributes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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;
 }

[PATCH 06/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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;
 }

[PATCH 07/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dgc@kernel.org>
---
 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:

[PATCH 08/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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,

[PATCH 09/13] 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 |   69 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 22 deletions(-)


diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index 2ce24bfe4c0fab..5ea3ce4bf8d846 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -794,6 +794,46 @@ xfarray_sort_scan_done(
 	si->folio = NULL;
 }
 
+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;
+}
+
 /*
  * Cache the folio backing the start of the given array element.  If the array
  * element is contained entirely within the folio, return a pointer to the
@@ -819,33 +859,18 @@ xfarray_sort_scan(
 	    (idx < si->first_folio_idx || idx > si->last_folio_idx))
 		xfarray_sort_scan_done(si);
 
-	/* Grab the first folio that backs this array element. */
+	/* Grab the folio that backs this array element. */
 	if (!si->folio) {
-		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;
-
-		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--;
-
-		trace_xfarray_sort_scan(si, idx);
+		error = xfarray_sort_load_folio(si, idx, idx_pos);
+		if (error)
+			return error;
 	}
 
 	/*
-	 * 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 we don't have a folio mapping the entire array element, read into
+	 * the scratchpad and we're done.
 	 */
-	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
+	if (!si->folio) {
 		void		*temp = xfarray_scratch(si->array);
 
 		error = xfile_load(si->array->xfile, temp, si->array->obj_size,

[PATCH 10/13] 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 repair and removed some clunky parts of the
original online checking code, 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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                             |  108 ++------------------
 4 files changed, 15 insertions(+), 114 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 5ea3ce4bf8d846..685824efdb93f6 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -140,55 +140,20 @@ xfarray_load(
 			xfarray_pos(array, idx));
 }
 
-/* Is this array element potentially unset? */
-static inline bool
-xfarray_is_unset(
-	struct xfarray	*array,
-	loff_t		pos)
-{
-	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;
-
-	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;
+	loff_t			new_eof;
 
-	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;
+	new_eof = xfarray_pos(array, array->nr);
+	xfile_discard(array->xfile, new_eof, MAX_LFS_FILESIZE - new_eof);
 	return 0;
 }
 
@@ -227,43 +192,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 +605,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 11/13] 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/xfarray.h |    3 +++
 fs/xfs/scrub/xfarray.c |   14 ++++++++++++++
 2 files changed, 17 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 685824efdb93f6..7f85855588ed54 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -152,6 +152,9 @@ xfarray_trim(
 		return -ENODATA;
 
 	array->nr -= nr;
+	if (!array->nr)
+		array->possibly_sparse = false;
+
 	new_eof = xfarray_pos(array, array->nr);
 	xfile_discard(array->xfile, new_eof, MAX_LFS_FILESIZE - new_eof);
 	return 0;
@@ -179,6 +182,8 @@ xfarray_store(
 	if (ret)
 		return ret;
 
+	if (idx > array->nr)
+		array->possibly_sparse = true;
 	array->nr = max(array->nr, idx + 1);
 	return 0;
 }
@@ -853,6 +858,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)
@@ -1006,4 +1019,5 @@ xfarray_truncate(
 {
 	xfile_discard(array->xfile, 0, MAX_LFS_FILESIZE);
 	array->nr = 0;
+	array->possibly_sparse = false;
 }

[PATCH 12/13] 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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);
 

[PATCH 13/13] 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
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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 7f85855588ed54..c94f355607790e 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -682,6 +682,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 09/13] xfarray: don't crash when sorting if array element crosses a folio

[Christoph Hellwig]

Looks good:

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

Re: [PATCHSET v2 1/2] xfs: LLM-inspired bug fixes, part 16

[Carlos Maiolino]

On Tue, 22 Sep 2026 22:57:26 -0700, Darrick J. Wong wrote:
> Here's a sixteenth batch of xfs fixes resulting from a LLaMma.  Mwa mwa
> mwa...
> 
> v2: add RVB tags, integrate some review feedback from humans and LOLLM
> 
> If you're going to start using this code, I strongly recommend pulling
> from my git trees, which are linked below.
> 
> [...]

Applied to for-next, thanks!

[01/13] xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair
        commit: da483b8a9c2c3e7b23655699680feb57a194000c
[02/13] xfs: online quotacheck must dirty dquot if enforcement adjustments needed
        commit: 244425cf63d7e3a0ff5736b41f0e0afe807f7031
[03/13] xfs: fix buffer overruns in xfs_ioc_attr_list
        commit: 0b9081a774ee7b9dff1e9ac3765b73c1d6bba5b2
[04/13] xfs: clean up after failed metafile relinking
        commit: 07c73d1f0be610a8c14a2d73ed5cbda8ea9749d7
[05/13] xfs: pass xfs_trans_resv object to reservation calculation helpers
        commit: 035d27cb2ae7f9a2f2e2625671086995c6b45433
[06/13] xfs: fix xfs_rename_space_res for non-pptr filesystems
        commit: a800b926346c3aa2482421f5c0272280ee7d32e8
[07/13] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork
        commit: aafe21c23ad8263251085291019d8860ddc51da9
[08/13] xfs: add missing healthmon trace strings
        commit: 23067b4f98f536ff3341fde04d52a331b2ebdf76
[09/13] xfarray: don't crash when sorting if array element crosses a folio
        commit: 329750292e9dfbb594b375d7b30cf5ddb58b7bb9
[10/13] xfarray: don't allow users to unset in the middle of an array
        commit: 3a94b91aef22b6091cdad17160105070cbf2d509
[11/13] xfs: don't allow sorting sparse arrays
        commit: 76ac15cafb29ef11098e3db7fb955922f48c823a
[12/13] xfs: simply the free space btree repair code
        commit: ebd528d1987e133fd33f51f51d76294c9a4cc196
[13/13] xfarray: warn against sorting arrays with identical elements
        commit: ddeb5fe2d8a8645d033d868117571649184d64ab

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