[PATCH v3 0/6] xfs: fix filesystem shutdown from parent pointer reservation underflow

[PATCH v3 0/6] xfs: fix filesystem shutdown from parent pointer reservation underflow

[Javier Tia]

The first parent pointer update that has to grow the attribute fork
twice in one operation can shut the filesystem down with

    XFS (loop0): Corruption of in-memory data (0x8) detected at
    xfs_defer_finish_noroll+0x29a/0x4b0 (fs/xfs/libxfs/xfs_defer.c:721)

The message names in-core corruption, but nothing is corrupt.  The
deferred work that failed is a block allocation that returned a bare
-ENOSPC on a filesystem with hundreds of gigabytes free, and
xfs_defer_finish_noroll() treats any non-EAGAIN error as fatal.

xfs_parent_da_args_init() builds an xfs_da_args from a zeroed
xfs_parent_args (kmem_cache_zalloc), so a runtime parent pointer update
reaches the block allocator with args->total == 0.  That field is not a
constant.  xfs_da_grow_inode_int() treats it as a running remainder and
subtracts from it, and xfs_da_args.total is an xfs_extlen_t (uint32_t),
so subtracting the first block the attr fork gains wraps it to
0xffffffff.  It reaches the minimum-free-space test in
xfs_alloc_space_available() as (int)max(args->total, alloc_len), where
the cast turns ~0U back into -1 and the test can no longer fail.
Execution falls into the clamp below it, args->maxlen = available, and
when available is 0 the result is maxlen == 0.  Both ASSERTs guarding
that clamp are compiled out without CONFIG_XFS_DEBUG.
xfs_alloc_vextent_check_args() then rejects minlen(1) > maxlen(0) with
-ENOSPC, and the filesystem goes down.

The log recovery path already does this correctly, which is the
clearest statement that the runtime path is wrong.
xfs_attri_recover_work() does args->total = xfs_attr_calc_size(args,
&local) for PPTR_SET and PPTR_REPLACE, so replaying a parent pointer
insert from the log runs with a correct total while performing the same
insert at runtime runs with zero.  Patch 5 sets the field the same way,
in the add and replace paths that can grow the fork.

One detail is worth stating because it is not obvious from the diff.
In the captured trace total changes from 0xffffffff to 1 one step
before the failure: xfs_bmap_btalloc_low_space() sets args->total =
ap->minlen before its last-ditch sweep of every AG.  Nothing resets
args->maxlen, so the clamped zero survives into check_args.  The
fallback that exists to rescue an over-large reservation cannot rescue a
clamped maxlen.

Two conditions have to coincide, which is why the bug looks rare.  The
first is two xfs_da_grow_inode() calls sharing one xfs_da_args; that
happens whenever a leaf-to-node conversion is followed by a node split
in the same operation, which is ordinary rather than a corner case.
The second is an AG at the later growth whose available block count,
pagf_freeblks + agflcount - reservation - min_free - minleft, is exactly
zero.  At available == -1 the same code takes a different path and at
available >= 1 the allocation succeeds; only the exact-zero coincidence
is fatal.  The underflow itself is common and usually harmless.  In one
capture an earlier link() carried total 4294967295 with maxlen 1 and
allocated successfully a few hundred milliseconds before the fatal one.

I reproduced this deterministically on the same machine, same script,
only the kernel differing.  Unpatched, the filesystem shuts down within
0.2 s of link activity and eight "total 4294967295" allocator events are
recorded; patched, 400 steps and 25,024 links complete clean with zero
such events.  The attr fork counter goes 0 -> 4294967295 unpatched and
25 -> 24 patched, while the data fork counter goes 78 -> 77 in both:
that last row is the control, the same subtraction happening correctly
and unchanged by the patch.  A second machine on different hardware, on
its own unpatched kernel, produced a byte-identical event distribution.
The patched kernel has since carried ordinary Yocto build load for eight
days with no filesystem shutdown; before the patch the same machine died
within five to seven minutes once the triggering conditions coincided.

I ran ./check -g parent -g attr on the patched kernel, 53 of 55 passing.
The two failures are environmental, a setfattr deprecation warning newer
than the golden output and an O_TMPFILE EOVERFLOW in the harness, and
neither mentions parent pointers, allocation or ENOSPC.  I have not run
the same tests against an unpatched kernel, so I am not claiming those
two are unrelated to this series.  A dedicated regression test is posted
separately to fstests as tests/xfs/842; it fails on an unpatched kernel
and passes on a patched one.

The workload that first hit this is a Yocto/BitBake do_package run, which
hardlinks one file into many package staging directories so that every
link() writes another parent pointer, on a 1.9 TB filesystem with 862
GiB free.

Related work is in flight.  On 2026-07-29 Dave Chinner posted an RFC,
"XFS: Atomic multi-extent operations via rolling transactions"
(20260729100629.1943710-1-dgc@kernel.org), which is not merged.  Its
patch 18, "xfs: add block reservation renewal to xfs_defer_finish",
touches fs/xfs/libxfs/xfs_defer.c, which patches 1 to 3 here also
modify; the hunks are unrelated, so if it lands first this series needs
a rebase rather than a redesign.  The overlap is also conceptual, and
worth naming because a reviewer will see it anyway: that RFC addresses a
transaction reservation carried forward by xfs_trans_dup() and depleted
over successive rolls even though each iteration needs the same amount.
This bug is the same shape one level down, with xfs_da_args.total
depleting across the xfs_da_grow_inode_int() calls that span one of
those rolls.  The two are independent, and patch 5 stands whether or not
the RFC is merged.

Changes since v2:
- Patch 3: shortened the commit message and reworded the shutdown log
  line to name the operation ("deferred agfl_free work failed ...", or
  "deferred chain work failed ..." for the roll and create-intents
  paths) (Darrick J. Wong).
- Collected Reviewed-by from Darrick J. Wong on patches 5 and 6.

Changes since v1:
- Patch 5: set args->total only in the add and replace paths rather than
  unconditionally in the shared init, so removals and lookups leave it
  alone, matching the xfs_attri_recover_work() switch (Darrick J. Wong).
- New patch 6: assert in xfs_da_grow_inode_int() that the remaining
  reservation still covers each fork growth, so this underflow class
  trips in debug builds instead of wrapping silently (suggested by
  Darrick J. Wong).
- Patch 5: added Cc: stable # v6.10 (Darrick J. Wong).
- Trimmed the patch 1, 4 and 5 commit messages (Darrick J. Wong).
- Collected Reviewed-by from Darrick J. Wong on patches 1, 2 and 4.

Link to v2:
https://lore.kernel.org/linux-xfs/20260810164312.960721-8-floss@jetm.me/
Link to v1:
https://lore.kernel.org/linux-xfs/20260808234016.246054-7-floss@jetm.me/

Only patch 5 is the fix.  The other five are independent and a
maintainer can take them separately:

  1. xfs: initialise error in xfs_defer_finish_one() is a separate bug.
     error is used uninitialised on the item-less barrier path reachable
     via xfs_defer_add_barrier() under CONFIG_XFS_ONLINE_REPAIR, so a
     successful barrier can be reported as corruption depending on stack
     contents.  It carries a Fixes: tag and Cc: stable.
  2. xfs: give the deferred barrier op type a name fills the only
     xfs_defer_op_type with a NULL .name.
  3. xfs: report the error that made deferred work shut down the fs moves
     the tracepoint ahead of the shutdown and logs the errno and the
     remaining reservation, so the failure is diagnosable from the log
     alone.  This is the patch that would have saved most of this
     investigation.
  4. xfs: correct the parent pointer space reservation comment.
  6. xfs: assert the reservation covers each da fork growth is the
     debug-build guard suggested during v1 review.

For anyone who wants to watch it happen.  Root required; it creates and
destroys a 512 MiB loop filesystem under /var/tmp.  If fs.xfs.panic_mask
is non-zero the shutdown becomes a BUG() and the machine reboots instead
of reporting, so lower it for the run.

    truncate -s 512M /var/tmp/pptr.img
    mkfs.xfs -q -f -m crc=1 -n parent=1 -d agcount=2 /var/tmp/pptr.img
    mkdir -p /mnt/pptr && mount -o loop /var/tmp/pptr.img /mnt/pptr

    mkdir /mnt/pptr/d
    dd if=/dev/urandom of=/mnt/pptr/src bs=4096 count=1 status=none

    # Fill to within ~460 free blocks, then walk that margin down one
    # block per step, hardlinking with 240-byte names so the attr fork
    # leaves shortform quickly.
    free=$(stat -f -c '%a' /mnt/pptr)
    fallocate -l $(( (free - 464) * 4096 )) /mnt/pptr/ballast
    name=$(printf 'x%.0s' $(seq 1 240))

    for step in $(seq 0 400); do
        fallocate --punch-hole --keep-size -o $((step * 4096)) -l 4096 \
            /mnt/pptr/ballast
        for i in $(seq 1 64); do
            ln /mnt/pptr/src "/mnt/pptr/d/${step}_${i}_$name" \
                2>/dev/null || break
        done
        rm -f /mnt/pptr/d/* 2>/dev/null
        touch /mnt/pptr/.alive 2>/dev/null \
            || { echo "shut down at step $step"; break; }
    done

Javier Tia (6):
  xfs: initialise error in xfs_defer_finish_one()
  xfs: give the deferred barrier op type a name
  xfs: report the error that made deferred work shut down the fs
  xfs: correct the parent pointer space reservation comment
  xfs: initialise args->total for parent pointer updates
  xfs: assert the reservation covers each da fork growth

 fs/xfs/libxfs/xfs_da_btree.c    |  1 +
 fs/xfs/libxfs/xfs_defer.c       | 18 ++++++++++++++++--
 fs/xfs/libxfs/xfs_parent.c      | 12 ++++++++++--
 fs/xfs/libxfs/xfs_trans_space.c | 19 +++++++++++++++++--
 4 files changed, 44 insertions(+), 6 deletions(-)


base-commit: 155b42bec9cbb6b8cdc47dd9bd09503a81fbe493
-- 
Javier Tia

[PATCH v3 1/6] xfs: initialise error in xfs_defer_finish_one()

[Javier Tia]

xfs_defer_finish_one() declares error without an initialiser and only
assigns it inside the loop over dfp->dfp_work.  When that list is empty
the loop body never runs, control falls through to the "Done with the
dfp, free it" path, and the function returns an indeterminate value.

An item-less pending item reaches this through xfs_defer_add_barrier(),
which xfs_reap_ag_blocks() uses on any CONFIG_XFS_ONLINE_REPAIR kernel.
xfs_defer_finish_noroll() treats any non-EAGAIN return as fatal, so a
non-zero stack value turns a successful barrier into a
SHUTDOWN_CORRUPT_INCORE in the middle of a repair.  Zero is the correct
result: reaching the free path means the item loop drained without a
non-zero error.

Fixes: 3f3cec031099 ("xfs: force small EFIs for reaping btree extents")
Cc: <stable@vger.kernel.org>
Signed-off-by: Javier Tia <floss@jetm.me>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_defer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 89501e8bd2f8..843c33304441 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -583,7 +583,7 @@ xfs_defer_finish_one(
 	const struct xfs_defer_op_type	*ops = dfp->dfp_ops;
 	struct xfs_btree_cur		*state = NULL;
 	struct list_head		*li, *n;
-	int				error;
+	int				error = 0;
 
 	trace_xfs_defer_pending_finish(tp->t_mountp, dfp);
 
-- 
Javier Tia

[PATCH v3 2/6] xfs: give the deferred barrier op type a name

[Javier Tia]

xfs_barrier_defer_type is the only xfs_defer_op_type with no .name.
Every other one carries a short string used for tracing and reporting:
attr, bmap, extent_free, agfl_free, rtextent_free, refcount,
rtrefcount, rmap, rtrmap and exchmaps.

That has been harmless because nothing dereferences the field, but it
leaves a NULL in a table where every other entry is populated, so the
first caller to print it gets "(null)" in the kernel and undefined
behaviour in the userspace libxfs build of this file, where xfs_alert
lands in fprintf.  xfs_defer_add() already treats a missing member of
this table as worth shutting the filesystem down for, so an unpopulated
one is out of step with how the file handles its own ops tables.

Signed-off-by: Javier Tia <floss@jetm.me>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_defer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 843c33304441..75f0d37914d5 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -229,6 +229,7 @@ xfs_defer_barrier_cancel_item(
 }
 
 static const struct xfs_defer_op_type xfs_barrier_defer_type = {
+	.name		= "barrier",
 	.max_items	= 1,
 	.create_intent	= xfs_defer_barrier_create_intent,
 	.abort_intent	= xfs_defer_barrier_abort_intent,
-- 
Javier Tia

[PATCH v3 3/6] xfs: report the error that made deferred work shut down the fs

[Javier Tia]

When a deferred operation fails and shuts the filesystem down,
xfs_defer_finish_noroll() reports neither the errno nor which operation
originated it, so the log cannot tell a transient -ENOSPC from real
corruption.  Report the operation type, errno and remaining reservation.

trace_xfs_defer_finish_error() runs after xfs_force_shutdown(), which
BUGs under fs.xfs.panic_mask and so never fires for the first failure;
move it ahead of the shutdown and mirror it to xfs_alert() for systems
without tracing armed.  Capture the op name while the item is live (dfp
is freed once its work list drains) and suppress the alert once the fs is
already down.

Signed-off-by: Javier Tia <floss@jetm.me>
---
 fs/xfs/libxfs/xfs_defer.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 75f0d37914d5..3152acdc335d 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -656,6 +656,7 @@ xfs_defer_finish_noroll(
 	struct xfs_trans		**tp)
 {
 	struct xfs_defer_pending	*dfp = NULL;
+	const char			*what = "chain";
 	int				error = 0;
 	LIST_HEAD(dop_pending);
 	LIST_HEAD(dop_paused);
@@ -705,9 +706,17 @@ xfs_defer_finish_noroll(
 				struct xfs_defer_pending, dfp_list);
 		if (!dfp)
 			break;
+		what = dfp->dfp_ops->name;
 		error = xfs_defer_finish_one(*tp, dfp);
 		if (error && error != -EAGAIN)
 			goto out_shutdown;
+		/*
+		 * A finished item is no longer a candidate for a later
+		 * failure.  An -EAGAIN one is not finished, so it keeps the
+		 * attribution across the roll that completes it.
+		 */
+		if (!error)
+			what = "chain";
 	}
 
 	/* Requeue the paused items in the outgoing transaction. */
@@ -719,8 +728,12 @@ xfs_defer_finish_noroll(
 out_shutdown:
 	list_splice_tail_init(&dop_paused, &dop_pending);
 	xfs_defer_trans_abort(*tp, &dop_pending);
-	xfs_force_shutdown((*tp)->t_mountp, SHUTDOWN_CORRUPT_INCORE);
 	trace_xfs_defer_finish_error(*tp, error);
+	if (!xfs_is_shutdown((*tp)->t_mountp))
+		xfs_alert((*tp)->t_mountp,
+			  "deferred %s work failed, error %d, %u blocks reserved",
+			  what, error, (*tp)->t_blk_res);
+	xfs_force_shutdown((*tp)->t_mountp, SHUTDOWN_CORRUPT_INCORE);
 	xfs_defer_cancel_list((*tp)->t_mountp, &dop_pending);
 	xfs_defer_cancel(*tp);
 	return error;
-- 
Javier Tia

[PATCH v3 4/6] xfs: correct the parent pointer space reservation comment

[Javier Tia]

The comment on xfs_parent_calc_space_res() claims parent pointers are
"always the first attr in an attr tree".  They are not: a parent pointer
is recorded per dirent, so by the Nth hardlink the attr fork is already
in leaf or node format.  The reservation is still correct, because
XFS_DAENTER_SPACE_RES() covers a split at every level of a maximum-depth
attr dabtree whatever format the fork is in, but anyone auditing a
shortfall here is led by the comment to look for a bug that is not
there.

Rewrite the comment to state what actually bounds the result, and record
why the double split allowance and the extent-add term differ from
xfs_attr_calc_size().

Signed-off-by: Javier Tia <floss@jetm.me>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_trans_space.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_trans_space.c b/fs/xfs/libxfs/xfs_trans_space.c
index 9b8f495c9049..c4cd547033e5 100644
--- a/fs/xfs/libxfs/xfs_trans_space.c
+++ b/fs/xfs/libxfs/xfs_trans_space.c
@@ -22,8 +22,23 @@ xfs_parent_calc_space_res(
 	unsigned int		namelen)
 {
 	/*
-	 * Parent pointers are always the first attr in an attr tree, and never
-	 * larger than a block
+	 * A parent pointer is recorded per dirent, so an inode with N links
+	 * carries N of them and the attr fork can already be in leaf or node
+	 * format when one is added.  That does not affect the reservation:
+	 * XFS_DAENTER_SPACE_RES covers a split at every level of a
+	 * maximum-depth attr dabtree, whatever format the fork is in now.
+	 *
+	 * The name is a dirent name and the value is a struct xfs_parent_rec,
+	 * so the leaf entry is always local and never exceeds 272 bytes.
+	 * Parent pointers require V5, hence a 1k minimum block size, so the
+	 * entry always stays under half a block and this needs none of the
+	 * double split allowance that xfs_attr_calc_size() makes.
+	 *
+	 * The second term hands a byte count to a macro whose parameter counts
+	 * mappings, so it asks for more extent-add allowance than the single
+	 * mapping a parent pointer adds - how much more depends on the block
+	 * size.  It over-reserves either way, which is why it is left alone:
+	 * correcting the unit would shrink a reservation that is only generous.
 	 */
 	return XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK) +
 	       XFS_NEXTENTADD_SPACE_RES(mp, namelen, XFS_ATTR_FORK);
-- 
Javier Tia

[PATCH v3 5/6] xfs: initialise args->total for parent pointer updates

[Javier Tia]

xfs_parent_da_args_init() builds an xfs_da_args from a zeroed
xfs_parent_args (kmem_cache_zalloc), leaving args->total == 0.
xfs_da_grow_inode_int() treats that field as a running block reservation
and subtracts from it; because it is an xfs_extlen_t (uint32_t), the
first attr-fork growth wraps it to ~0U.  That defeats the free-space
check in xfs_alloc_space_available(), and when it coincides with an AG
that has exactly zero available blocks the allocation is clamped to
maxlen 0 and returns -ENOSPC, which xfs_defer_finish_noroll() escalates
to a filesystem shutdown.

Set args->total the way the log recovery path does
(xfs_attri_recover_work(), xfs_attr_item.c:706), in the add and replace
paths that can grow the fork.  Removals and lookups never grow it, so
they leave the field alone, matching that switch.

Fixes: b7c62d90c12c ("xfs: parent pointer attribute creation")
Cc: <stable@vger.kernel.org> # v6.10
Signed-off-by: Javier Tia <floss@jetm.me>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_parent.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index 3509cc4b2175..312f33086d53 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -194,7 +194,7 @@ xfs_parent_addname(
 	const struct xfs_name	*parent_name,
 	struct xfs_inode	*child)
 {
-	int			error;
+	int			error, local;
 
 	error = xfs_parent_iread_extents(tp, child);
 	if (error)
@@ -204,6 +204,10 @@ xfs_parent_addname(
 	xfs_parent_da_args_init(&ppargs->args, tp, &ppargs->rec, child,
 			child->i_ino, parent_name);
 
+	/* Growing the attr fork needs a real reservation in args->total. */
+	ppargs->args.total = xfs_attr_calc_size(&ppargs->args, &local);
+	ASSERT(local);
+
 	return xfs_attr_setname(&ppargs->args, 0);
 }
 
@@ -240,7 +244,7 @@ xfs_parent_replacename(
 	const struct xfs_name	*new_name,
 	struct xfs_inode	*child)
 {
-	int			error;
+	int			error, local;
 
 	error = xfs_parent_iread_extents(tp, child);
 	if (error)
@@ -250,6 +254,10 @@ xfs_parent_replacename(
 	xfs_parent_da_args_init(&ppargs->args, tp, &ppargs->rec, child,
 			child->i_ino, old_name);
 
+	/* Growing the attr fork needs a real reservation in args->total. */
+	ppargs->args.total = xfs_attr_calc_size(&ppargs->args, &local);
+	ASSERT(local);
+
 	xfs_inode_to_parent_rec(&ppargs->new_rec, new_dp);
 
 	ppargs->args.new_name = new_name->name;
-- 
Javier Tia

[PATCH v3 6/6] xfs: assert the reservation covers each da fork growth

[Javier Tia]

xfs_da_grow_inode_int() subtracts the blocks it just allocated from
args->total, the caller's remaining block reservation.  The subtraction
is unsigned, so a caller that reaches it with too small a total wraps
the field instead of failing, and every allocation afterwards runs with
a bogus reservation.  Assert the remaining reservation still covers the
step, so an under-reserved or uninitialised total trips in debug builds
instead of silently wrapping.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Javier Tia <floss@jetm.me>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_da_btree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index ad801b7bd2dd..9be407affc6e 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2385,6 +2385,7 @@ xfs_da_grow_inode_int(
 	}
 
 	/* account for newly allocated blocks in reserved blocks total */
+	ASSERT(args->total >= dp->i_nblocks - nblks);
 	args->total -= dp->i_nblocks - nblks;
 
 out_free_map:
-- 
Javier Tia