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

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

[Darrick J. Wong]

Hi all,

Here's a seventh 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-7
---
Commits in this patchset:
 * xfs: don't spin forever on zero-length dirents when salvaging them
 * xfs: don't stash removename operations with unknown ftype
 * xfs: log the tempip after we convert it to extents format
 * xfs: fix parent rec lookup initialization in xrep_metapath_unlink
 * xfs: handle reconnecting metadir subdirectories
 * xfs: lock the healthmon when inserting unmount event
---
 fs/xfs/scrub/dir_repair.c     |   27 +++++++++++++-
 fs/xfs/scrub/metapath.c       |   78 ++++++++++++++++++++++++++++++++++++++++-
 fs/xfs/scrub/symlink_repair.c |    2 +
 fs/xfs/xfs_healthmon.c        |    2 +
 4 files changed, 103 insertions(+), 6 deletions(-)

[PATCH 1/6] xfs: don't spin forever on zero-length dirents when salvaging them

[Darrick J. Wong]

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

LOLLM noticed that xrep_dir_recover_data can spin forever if it
encounters an unused dirent that claims to have length zero.  Fix that,
and prevent the same thing from happening with a zero-length entry.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: b1991ee3e7cf85 ("xfs: online repair of directories")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir_repair.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 1c088cfba10ea9..0c1224d05d579a 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -484,18 +484,24 @@ xrep_dir_recover_data(
 	while (offset < end) {
 		struct xfs_dir2_data_unused	*dup = bp->b_addr + offset;
 		struct xfs_dir2_data_entry	*dep = bp->b_addr + offset;
+		unsigned int			advance;
 
 		if (xchk_should_terminate(rd->sc, &error))
 			return error;
 
 		/* Skip unused entries. */
 		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
+			if (!dup->length)
+				break;
 			offset += be16_to_cpu(dup->length);
 			continue;
 		}
 
 		/* Don't walk off the end of the block. */
-		offset += xfs_dir2_data_entsize(rd->sc->mp, dep->namelen);
+		advance = xfs_dir2_data_entsize(rd->sc->mp, dep->namelen);
+		if (!advance)
+			break;
+		offset += advance;
 		if (offset > end)
 			break;
 

Re: [PATCH 1/6] xfs: don't spin forever on zero-length dirents when salvaging them

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:34:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xrep_dir_recover_data can spin forever if it
> encounters an unused dirent that claims to have length zero.  Fix that,
> and prevent the same thing from happening with a zero-length entry.

xfstests for this?

The fix itself looks good:

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

Re: [PATCH 1/6] xfs: don't spin forever on zero-length dirents when salvaging them

[Darrick J. Wong]

On Tue, Aug 25, 2026 at 08:26:44AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:34:39PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that xrep_dir_recover_data can spin forever if it
> > encounters an unused dirent that claims to have length zero.  Fix that,
> > and prevent the same thing from happening with a zero-length entry.
> 
> xfstests for this?

Working on it, thanks for reviewing!

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

--D

[PATCH 2/6] xfs: don't stash removename operations with unknown ftype

[Darrick J. Wong]

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

LOLLM notices that the behavior of xrep_dir_replay_update changes based
on the ftype recorded in the stashed removename information.  It also
notices that the unlink iops sometimes set that ftype to FT_UNKNOWN
because the regular directory tree update code paths don't need to know
the ftype of the child.

Unfortunately, this results in incorrect link counts, which eventually
trips link count errors in later phases of xfs_scrub, or in xfs_repair.
Fix this.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 8559b21a64d983 ("xfs: implement live updates for directory repairs")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir_repair.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 0c1224d05d579a..31a23c5f386ae6 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1381,9 +1381,24 @@ xrep_dir_live_update(
 		if (p->delta > 0)
 			error = xrep_dir_stash_createname(rd, p->name,
 					I_INO(p->ip));
-		else
-			error = xrep_dir_stash_removename(rd, p->name,
+		else {
+			/*
+			 * xfs_dentry_to_name in unlink or rename-exchange can
+			 * pass us names with ftype FT_UNKNOWN, but we really
+			 * must know the ftype of the child that is being
+			 * removed so that we can do nlink updates correctly
+			 * without holding inode references.
+			 */
+			struct xfs_name	name = {
+				.name	= p->name->name,
+				.len	= p->name->len,
+				.type	= xfs_mode_to_ftype(
+						VFS_IC(p->ip)->i_mode),
+			};
+
+			error = xrep_dir_stash_removename(rd, &name,
 					I_INO(p->ip));
+		}
 		mutex_unlock(&rd->pscan.lock);
 		if (error)
 			goto out_abort;

Re: [PATCH 2/6] xfs: don't stash removename operations with unknown ftype

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:34:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notices that the behavior of xrep_dir_replay_update changes based
> on the ftype recorded in the stashed removename information.  It also
> notices that the unlink iops sometimes set that ftype to FT_UNKNOWN
> because the regular directory tree update code paths don't need to know
> the ftype of the child.
> 
> Unfortunately, this results in incorrect link counts, which eventually
> trips link count errors in later phases of xfs_scrub, or in xfs_repair.
> Fix this.

Fix what?

> @@ -1381,9 +1381,24 @@ xrep_dir_live_update(
>  		if (p->delta > 0)
>  			error = xrep_dir_stash_createname(rd, p->name,
>  					I_INO(p->ip));
> -		else
> -			error = xrep_dir_stash_removename(rd, p->name,
> +		else {
> +			/*
> +			 * xfs_dentry_to_name in unlink or rename-exchange can
> +			 * pass us names with ftype FT_UNKNOWN, but we really
> +			 * must know the ftype of the child that is being
> +			 * removed so that we can do nlink updates correctly
> +			 * without holding inode references.
> +			 */
> +			struct xfs_name	name = {
> +				.name	= p->name->name,
> +				.len	= p->name->len,
> +				.type	= xfs_mode_to_ftype(
> +						VFS_IC(p->ip)->i_mode),
> +			};

.. fix up the xfs_name to always have the correct type is I guess what
the commit log should say.

With that:

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

Re: [PATCH 2/6] xfs: don't stash removename operations with unknown ftype

[Darrick J. Wong]

On Tue, Aug 25, 2026 at 08:28:00AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:34:54PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM notices that the behavior of xrep_dir_replay_update changes based
> > on the ftype recorded in the stashed removename information.  It also
> > notices that the unlink iops sometimes set that ftype to FT_UNKNOWN
> > because the regular directory tree update code paths don't need to know
> > the ftype of the child.
> > 
> > Unfortunately, this results in incorrect link counts, which eventually
> > trips link count errors in later phases of xfs_scrub, or in xfs_repair.
> > Fix this.
> 
> Fix what?
> 
> > @@ -1381,9 +1381,24 @@ xrep_dir_live_update(
> >  		if (p->delta > 0)
> >  			error = xrep_dir_stash_createname(rd, p->name,
> >  					I_INO(p->ip));
> > -		else
> > -			error = xrep_dir_stash_removename(rd, p->name,
> > +		else {
> > +			/*
> > +			 * xfs_dentry_to_name in unlink or rename-exchange can
> > +			 * pass us names with ftype FT_UNKNOWN, but we really
> > +			 * must know the ftype of the child that is being
> > +			 * removed so that we can do nlink updates correctly
> > +			 * without holding inode references.
> > +			 */
> > +			struct xfs_name	name = {
> > +				.name	= p->name->name,
> > +				.len	= p->name->len,
> > +				.type	= xfs_mode_to_ftype(
> > +						VFS_IC(p->ip)->i_mode),
> > +			};
> 
> .. fix up the xfs_name to always have the correct type is I guess what
> the commit log should say.

I'll expand that last sentence to read:
"Fix this by creating a second xfs_name with the type set correctly."

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

Thanks!

--D

[PATCH 3/6] xfs: log the tempip after we convert it to extents format

[Darrick J. Wong]

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

LOLLM points out that xrep_symlink_swap_prep converts sc->tempip to an
extents format file prior to the atomic swap, but incorrectly logs
sc->ip immediately afterwards.  Fix that, and the other problem that
we're supposed to tell xfs_trans_log_inode what to log and don't.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 2651923d8d8db0 ("xfs: online repair of symbolic links")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/symlink_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/symlink_repair.c b/fs/xfs/scrub/symlink_repair.c
index 91c86ea0e0f155..18196136423336 100644
--- a/fs/xfs/scrub/symlink_repair.c
+++ b/fs/xfs/scrub/symlink_repair.c
@@ -291,7 +291,7 @@ xrep_symlink_swap_prep(
 		if (error)
 			return error;
 
-		xfs_trans_log_inode(sc->tp, sc->ip, 0);
+		xfs_trans_log_inode(sc->tp, sc->tempip, logflags);
 
 		error = xfs_defer_finish(&sc->tp);
 		if (error)

Re: [PATCH 3/6] xfs: log the tempip after we convert it to extents format

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:35:10PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that xrep_symlink_swap_prep converts sc->tempip to an
> extents format file prior to the atomic swap, but incorrectly logs
> sc->ip immediately afterwards.  Fix that, and the other problem that
> we're supposed to tell xfs_trans_log_inode what to log and don't.

Looks good:

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

[PATCH 4/6] xfs: fix parent rec lookup initialization in xrep_metapath_unlink

[Darrick J. Wong]

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

LOLLM notices that xrep_metapath_unlink looks for a parent pointer in
the child metafile that it's removing, but initializes the parent handle
using the child.  This is obviously incorrect, so fix that.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: 0d2c636e489c11 ("xfs: repair metadata directory file path connectivity")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/metapath.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/metapath.c b/fs/xfs/scrub/metapath.c
index ff1ff762b3003c..9f44c82910eaef 100644
--- a/fs/xfs/scrub/metapath.c
+++ b/fs/xfs/scrub/metapath.c
@@ -397,7 +397,7 @@ xrep_metapath_unlink(
 
 	/* Figure out if we're removing a parent pointer too. */
 	if (xfs_has_parent(mp)) {
-		xfs_inode_to_parent_rec(&rec, ip);
+		xfs_inode_to_parent_rec(&rec, mpath->dp);
 		error = xfs_parent_lookup(sc->tp, ip, &mpath->xname, &rec,
 				&mpath->pptr_args);
 		switch (error) {

Re: [PATCH 4/6] xfs: fix parent rec lookup initialization in xrep_metapath_unlink

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:35:26PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notices that xrep_metapath_unlink looks for a parent pointer in
> the child metafile that it's removing, but initializes the parent handle
> using the child.  This is obviously incorrect, so fix that.

Eww.  Can we come up with a reproducer for this?

The fix looks good:

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

Re: [PATCH 4/6] xfs: fix parent rec lookup initialization in xrep_metapath_unlink

[Darrick J. Wong]

On Tue, Aug 25, 2026 at 08:31:01AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:35:26PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM notices that xrep_metapath_unlink looks for a parent pointer in
> > the child metafile that it's removing, but initializes the parent handle
> > using the child.  This is obviously incorrect, so fix that.
> 
> Eww.  Can we come up with a reproducer for this?

At the moment it would be rather difficult to do that because all the
metadir directory tree paths have to be working correctly for the
filesystem to mount.   I'll see if there's some evil way to do that,
like ... zeroing qflags, corrupting /quotas, and trying to truncate the
(unused) quota files while quota is offline.

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

Thanks!

--D

[PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[Darrick J. Wong]

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

A longstanding weakness of the metapath repair code is that it can only
reattach non-directories to the metadata directory tree.  Let's fix that


Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/metapath.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/metapath.c b/fs/xfs/scrub/metapath.c
index 9f44c82910eaef..3e59ebbe1bcf32 100644
--- a/fs/xfs/scrub/metapath.c
+++ b/fs/xfs/scrub/metapath.c
@@ -23,6 +23,7 @@
 #include "xfs_rtgroup.h"
 #include "xfs_rtrmap_btree.h"
 #include "xfs_rtrefcount_btree.h"
+#include "xfs_ag.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
 #include "scrub/trace.h"
@@ -348,12 +349,78 @@ xchk_metapath(
 }
 
 #ifdef CONFIG_XFS_ONLINE_REPAIR
+/*
+ * Given a directory @dp, an existing inode @ip, and a @name, link @ip into @dp
+ * under the given @name.
+ */
+static int
+xrep_metadir_add_child(
+	struct xchk_metapath	*mpath,
+	xfs_ino_t		old_dotdot)
+{
+	struct xfs_trans	*tp = mpath->sc->tp;
+	struct xfs_dir_update	*du = &mpath->du;
+	struct xfs_inode	*dp = du->dp;
+	const struct xfs_name	*name = du->name;
+	struct xfs_inode	*ip = du->ip;
+	struct xfs_mount	*mp = tp->t_mountp;
+	const unsigned int	resblks = mpath->link_resblks;
+	int			error;
+
+	/*
+	 * The metadata file shouldn't be on the unlinked list, but we'll fix
+	 * it if that is the case.
+	 */
+	if (VFS_I(ip)->i_nlink == 0) {
+		struct xfs_perag	*pag;
+
+		pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, I_INO(ip)));
+		error = xfs_iunlink_remove(tp, pag, ip);
+		xfs_perag_put(pag);
+		if (error)
+			return error;
+	}
+
+	error = xfs_dir_createname(tp, dp, name, I_INO(ip), resblks);
+	if (error)
+		return error;
+
+	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
+
+	xfs_bumplink(tp, ip);
+
+	/* update dotdot entry in child */
+	if (S_ISDIR(VFS_I(ip)->i_mode)) {
+		xfs_bumplink(tp, dp);
+
+		/* Replace the dotdot entry in the child */
+		if (old_dotdot != I_INO(dp)) {
+			error = xfs_dir_replace(tp, ip, &xfs_name_dotdot,
+					I_INO(dp), resblks);
+			if (error)
+				return error;
+		}
+	}
+
+	/* Update the child's parent pointer */
+	if (du->ppargs) {
+		error = xfs_parent_addname(tp, du->ppargs, dp, name, ip);
+		if (error)
+			return error;
+	}
+
+	xfs_dir_update_hook(dp, ip, 1, name);
+	return 0;
+}
+
 /* Create the dirent represented by the final component of the path. */
 STATIC int
 xrep_metapath_link(
 	struct xchk_metapath	*mpath)
 {
 	struct xfs_scrub	*sc = mpath->sc;
+	xfs_ino_t		old_dotdot = NULLFSINO;
+	int			error;
 
 	mpath->du.dp = mpath->dp;
 	mpath->du.name = &mpath->xname;
@@ -366,7 +433,14 @@ xrep_metapath_link(
 
 	trace_xrep_metapath_link(sc, mpath->path, mpath->dp, I_INO(sc->ip));
 
-	return xfs_dir_add_child(sc->tp, mpath->link_resblks, &mpath->du);
+	if (S_ISDIR(VFS_I(sc->ip)->i_mode)) {
+		error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot,
+				&old_dotdot);
+		if (error && error != -ENOENT)
+			return error;
+	}
+
+	return xrep_metadir_add_child(mpath, old_dotdot);
 }
 
 /* Remove the dirent at the final component of the path. */

Re: [PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:35:41PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> A longstanding weakness of the metapath repair code is that it can only
> reattach non-directories to the metadata directory tree.  Let's fix that

Why was this not done before?  Why is it possible now?  Testcase?

Re: [PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[Darrick J. Wong]

On Tue, Aug 25, 2026 at 08:32:14AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:35:41PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > A longstanding weakness of the metapath repair code is that it can only
> > reattach non-directories to the metadata directory tree.  Let's fix that
> 
> Why was this not done before?  Why is it possible now?  Testcase?

I straight up forgot to implement it.  And right now there's no way to
mount a filesystem with a broken subdirectory (your choices are
/rtgroups or /quota) so it didn't stand out.

Some day there might be a need to handle deeper metadata directory
trees, so I figured that I should perhaps go implement that now.

Still working on how to make a testcase work though. :/

--D

Re: [PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[Christoph Hellwig]

On Tue, Aug 25, 2026 at 04:40:01PM -0700, Darrick J. Wong wrote:
> On Tue, Aug 25, 2026 at 08:32:14AM +0200, Christoph Hellwig wrote:
> > On Mon, Aug 24, 2026 at 10:35:41PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > A longstanding weakness of the metapath repair code is that it can only
> > > reattach non-directories to the metadata directory tree.  Let's fix that
> > 
> > Why was this not done before?  Why is it possible now?  Testcase?
> 
> I straight up forgot to implement it.

Heh..

> And right now there's no way to
> mount a filesystem with a broken subdirectory (your choices are
> /rtgroups or /quota) so it didn't stand out.

Maybe add this to the commit log?

Re: [PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[Darrick J. Wong]

On Wed, Aug 26, 2026 at 06:46:01AM +0200, Christoph Hellwig wrote:
> On Tue, Aug 25, 2026 at 04:40:01PM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 25, 2026 at 08:32:14AM +0200, Christoph Hellwig wrote:
> > > On Mon, Aug 24, 2026 at 10:35:41PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > A longstanding weakness of the metapath repair code is that it can only
> > > > reattach non-directories to the metadata directory tree.  Let's fix that
> > > 
> > > Why was this not done before?  Why is it possible now?  Testcase?
> > 
> > I straight up forgot to implement it.
> 
> Heh..
> 
> > And right now there's no way to
> > mount a filesystem with a broken subdirectory (your choices are
> > /rtgroups or /quota) so it didn't stand out.
> 
> Maybe add this to the commit log?

Ok will do.

--D

[PATCH 6/6] xfs: lock the healthmon when inserting unmount event

[Darrick J. Wong]

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

LOLLM complains that xfs_healthmon_unmount does an unlocked insert of
the unmount event into the health monitor's event list.  Fix that.

Cc: <stable@vger.kernel.org> # v7.0
Fixes: 25ca57fa3624ca ("xfs: convey filesystem unmount events to the health monitor")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_healthmon.c |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
index 4521ffdab9f1ae..d8b95af33a3e9f 100644
--- a/fs/xfs/xfs_healthmon.c
+++ b/fs/xfs/xfs_healthmon.c
@@ -415,8 +415,10 @@ xfs_healthmon_unmount(
 	 * There's nothing actionable for userspace after an unmount.  Once
 	 * we've inserted the unmount event, hm no longer owns that event.
 	 */
+	mutex_lock(&hm->lock);
 	__xfs_healthmon_insert(hm, hm->unmount_event);
 	hm->unmount_event = NULL;
+	mutex_unlock(&hm->lock);
 
 	xfs_healthmon_detach(hm);
 	xfs_healthmon_put(hm);

Re: [PATCH 6/6] xfs: lock the healthmon when inserting unmount event

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 10:35:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that xfs_healthmon_unmount does an unlocked insert of
> the unmount event into the health monitor's event list.  Fix that.

Looks good:

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

Can you also add a lockdep_assert_held to __xfs_healthmon_insert?

Re: [PATCH 6/6] xfs: lock the healthmon when inserting unmount event

[Darrick J. Wong]

On Tue, Aug 25, 2026 at 08:33:12AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:35:57PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM complains that xfs_healthmon_unmount does an unlocked insert of
> > the unmount event into the health monitor's event list.  Fix that.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Can you also add a lockdep_assert_held to __xfs_healthmon_insert?

Yes, I'll add one each to the _insert and _push functions.

--D

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

[Darrick J. Wong]

Hi all,

Here's a seventh batch of xfs fixes resulting from a LLaMma.  Mwa mwa
mwa...

v2: add rvbs, tweak some of the commit messages

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-7
---
Commits in this patchset:
 * xfs: don't spin forever on zero-length dirents when salvaging them
 * xfs: don't stash removename operations with unknown ftype
 * xfs: log the tempip after we convert it to extents format
 * xfs: fix parent rec lookup initialization in xrep_metapath_unlink
 * xfs: handle reconnecting metadir subdirectories
 * xfs: lock the healthmon when inserting unmount event
---
 fs/xfs/scrub/dir_repair.c     |   27 ++++++++++++-
 fs/xfs/scrub/metapath.c       |   85 ++++++++++++++++++++++++++++++++++++++++-
 fs/xfs/scrub/symlink_repair.c |    2 -
 fs/xfs/xfs_healthmon.c        |    6 +++
 4 files changed, 114 insertions(+), 6 deletions(-)

Unreviewed patches in this series:

[PATCHSET v2 1/2] xfs: LLM-inspired bug fixes, part 7
  [PATCH 5/6] xfs: handle reconnecting metadir subdirectories

[PATCH 6/6] xfs: lock the healthmon when inserting unmount event

[Darrick J. Wong]

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

LOLLM complains that xfs_healthmon_unmount does an unlocked insert of
the unmount event into the health monitor's event list.  Fix that.

Cc: <stable@vger.kernel.org> # v7.0
Fixes: 25ca57fa3624ca ("xfs: convey filesystem unmount events to the health monitor")
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_healthmon.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
index 4521ffdab9f1ae..3ae5f4496ad1aa 100644
--- a/fs/xfs/xfs_healthmon.c
+++ b/fs/xfs/xfs_healthmon.c
@@ -272,6 +272,8 @@ __xfs_healthmon_insert(
 {
 	struct timespec64		now;
 
+	lockdep_assert_held(&hm->lock);
+
 	ktime_get_coarse_real_ts64(&now);
 	event->time_ns = (now.tv_sec * NSEC_PER_SEC) + now.tv_nsec;
 
@@ -294,6 +296,8 @@ __xfs_healthmon_push(
 {
 	struct timespec64		now;
 
+	lockdep_assert_held(&hm->lock);
+
 	ktime_get_coarse_real_ts64(&now);
 	event->time_ns = (now.tv_sec * NSEC_PER_SEC) + now.tv_nsec;
 
@@ -415,8 +419,10 @@ xfs_healthmon_unmount(
 	 * There's nothing actionable for userspace after an unmount.  Once
 	 * we've inserted the unmount event, hm no longer owns that event.
 	 */
+	mutex_lock(&hm->lock);
 	__xfs_healthmon_insert(hm, hm->unmount_event);
 	hm->unmount_event = NULL;
+	mutex_unlock(&hm->lock);
 
 	xfs_healthmon_detach(hm);
 	xfs_healthmon_put(hm);