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

[PATCH 0/5] 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() fills in every field of the xfs_da_args it
builds except total, and the containing xfs_parent_args comes from
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
at init time.

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.

xfs_attr_calc_size() returns 25 blocks on a 4 KiB-block filesystem and
does not depend on name length, since a parent pointer's value is a
struct xfs_parent_rec and the leaf entry is always local.  I fixed the
caller rather than the unsigned subtraction in xfs_da_grow_inode_int()
to keep the runtime and recovery paths consistent.  This bounds the
underflow rather than eliminating the class: args->total is still a
monotonically decreasing unsigned counter that is never re-derived
across the deferred state machine.  XFS_DA_NODE_MAXDEPTH is 5, so the 26
growths on one xfs_da_args that underflowing from 25 would take are
unreachable in practice, but I put it on the record so the bound is
explicit.

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.

Only patch 5 is the fix.  The other four 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.

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 (5):
  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

 fs/xfs/libxfs/xfs_defer.c       | 18 ++++++++++++++++--
 fs/xfs/libxfs/xfs_parent.c      | 13 +++++++++++++
 fs/xfs/libxfs/xfs_trans_space.c | 19 +++++++++++++++++--
 3 files changed, 46 insertions(+), 4 deletions(-)

base-commit: 155b42bec9cbb6b8cdc47dd9bd09503a81fbe493
-- 
Javier Tia

[PATCH 1/5] 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 is not hypothetical.  Of the three
xfs_defer_alloc() callers, xfs_defer_add() always follows with
xfs_defer_add_item(), but the other two do not.
xfs_defer_start_recovery() is harmless because it adds to a
caller-supplied r_dfops list rather than to tp->t_dfops, so its items
never enter this path at all, and they are driven by
xfs_defer_finish_recovery() and ops->recover_work() rather than by
xfs_defer_finish_one().  xfs_defer_add_barrier() is neither:
xfs_defer_create_intents() walks tp->t_dfops without filtering
item-less entries, so a barrier is spliced onto the pending list and is
eligible to be picked by xfs_defer_finish_noroll().
xfs_reap_ag_blocks() adds one every other extent, so online repair
reaches this on any filesystem built with CONFIG_XFS_ONLINE_REPAIR.

The consequence is a filesystem shutdown that depends on stack
contents.  xfs_defer_finish_noroll() treats any non--EAGAIN return as
fatal and calls xfs_force_shutdown(SHUTDOWN_CORRUPT_INCORE), so
whenever the uninitialised value happens to be non-zero a successful
barrier is reported as in-core corruption and the filesystem is taken
down in the middle of a repair.  ops->finish_cleanup() also receives
the same value where an op type provides one, though no op type that
can reach the empty-list path defines one.

Returning zero is the correct result rather than a papered-over error,
and not only because the barrier type deliberately has no work items:
reaching the free path at all means the item loop drained without a
non-zero error, so zero is the truthful value for any op type.

The uninitialised declaration is older than the Fixes: commit below,
but that commit is where the bug became reachable - it added
xfs_defer_add_barrier(), the barrier op type and the only caller of it
in one go, and before it no item-less pending item could exist.

Fixes: 3f3cec031099 ("xfs: force small EFIs for reaping btree extents")
Cc: <stable@vger.kernel.org>
Signed-off-by: Javier Tia <floss@jetm.me>
---
 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 2/5] 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>
---
 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 3/5] xfs: report the error that made deferred work shut down the fs

[Javier Tia]

When xfs_defer_finish_one() fails with anything other than -EAGAIN,
xfs_defer_finish_noroll() shuts the filesystem down from a generic
out_shutdown: label.  SHUTDOWN_CORRUPT_INCORE makes that surface as
"Corruption of in-memory data (0x8) detected at
xfs_defer_finish_noroll+0x29a/0x4b0 (fs/xfs/libxfs/xfs_defer.c:721)",
naming neither the errno nor the deferred op that produced it.  Any
error from any deferred work item lands on that one line, so the report
is equally consistent with a transient -ENOSPC, an -EIO on a metadata
buffer, or genuine in-core corruption, and there is no way to tell
which from the log.

trace_xfs_defer_finish_error() records the errno, but it is called
after xfs_force_shutdown().  With fs.xfs.panic_mask carrying
XFS_PTAG_SHUTDOWN_CORRUPT (16), the first shutdown reaches
_xfs_alert_tag(), which BUGs, so the tracepoint does not fire for it.
Later racers do reach it, because xfs_do_force_shutdown() returns early
once xfs_set_shutdown() has fired, but by then the errno belongs to a
secondary failure.  The informative one is lost, and that is the
configuration used to capture a crash dump: recovering the errno from a
vmcore means an ORC unwind of the xfs_defer_finish_noroll frame to read
the callee-saved %rbp that happens to still hold the value.

Move the tracepoint ahead of xfs_force_shutdown() so it is reachable
for the first failure, and report the same information through the log,
because the systems that hit this do not have tracing armed in advance.
Report t_blk_res as well as the errno: how much of the reservation is
left separates a transaction that ran out of blocks from one that never
came close, which is the difference between suspecting whichever
xfs_*_space_res() fed it and moving the search to the allocator or to
the buffer that returned the error.  It cannot say more than that,
since xfs_trans_dup() hands each rolled transaction the unused
remainder, so a small value is also what a correctly sized reservation
looks like several rolls in.  t_blk_res_used is not worth printing
beside it: the new transaction starts at zero because xfs_trans_dup()
allocates it with kmem_cache_zalloc(), so it reads zero on the roll
paths and counts only the current segment on the others.

Take the op name in a local read before the call rather than from dfp
afterwards.  dfp is freed once its work list drains, so the name has to
be captured while the item is known live, and it has to outlive the
item to be available at out_shutdown for the paths that do not come
from xfs_defer_finish_one() at all.  dfp_ops points into a static const
table, so the string itself outlives everything.

Clear the attribution once an item finishes.  Three of the four paths to
out_shutdown - the create_intents failure and both trans_roll failures -
are reached at the top of a later loop iteration, before any item has
been picked, so a name left over from an item that already succeeded
would blame it for a log commit that failed afterwards.  That is worse
than the generic message this replaces, because it invents a lead where
there was none.  An -EAGAIN item keeps its name, since the roll that
follows is part of completing it.

Skip the alert once the filesystem is already down.  Only the first
failure is informative; everything after it is a consequence, and
xfs_do_force_shutdown() suppresses its own message for exactly that
reason.  Testing xfs_is_shutdown() rather than rate-limiting keeps the
first report unconditionally and drops the ones that follow, instead of
a token bucket that could spend itself on another mount's failures and
discard the one that mattered.

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..bbf2f4ca3c2e 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 = "deferred";
 	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 = "deferred";
 	}
 
 	/* 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,
+			  "%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 4/5] 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 an inode with N hardlinks carries N of them,
and `xfs_io -c "parent -p"` on a 31-link file lists 31.  By the Nth link
the attr fork is in leaf or node format and the insert is not into a
fresh tree.

The reservation itself is fine, which is what makes the comment worth
fixing rather than the code.  XFS_DAENTER_SPACE_RES() reserves
XFS_DA_NODE_MAXDEPTH blocks plus a bmap allowance for each, i.e. enough
to split every level of a maximum-depth attr dabtree.  That depth is a
format ceiling, not a runtime property, so the result cannot depend on
the format the fork happens to be in.  Anyone auditing a reservation
shortfall here reads the comment, concludes the sizing rests on an
assumption that demonstrably does not hold, and goes looking for a bug
that is not there.

Record why no double split allowance is needed either, since that is one
of two visible differences from xfs_attr_calc_size() and is not obvious
from the expression: a parent pointer's name is a dirent name and its
value is a struct xfs_parent_rec, so the leaf entry is local and at most
round_up(3 + 255 + 12, 4) = 272 bytes.  Parent pointers require V5 and
therefore XFS_MIN_CRC_BLOCKSIZE, so the smallest half-block this can be
compared against is 512 and the double split branch is unreachable on
every mountable geometry.  Locality is decided against a different
threshold, xfs_attr_leaf_entsize_local_max() at three quarters of a
block, which the 272 bytes also clears.

Record the other difference too.  The second term hands a byte count to
XFS_NEXTENTADD_SPACE_RES(), whose parameter counts mappings, so it asks
for more extent-add allowance than the one mapping a parent pointer
adds.  The factor depends on the block size, because the macro divides
by XFS_MAX_CONTIG_EXTENTS_PER_BLOCK(), so the comment says only that it
over-reserves - a patch whose whole point is that the old comment stated
a geometry-dependent thing as invariant should not do the same.  That it
over-reserves is why it is not a bug and why this patch leaves it alone.

Signed-off-by: Javier Tia <floss@jetm.me>
---
 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 5/5] xfs: initialise args->total for parent pointer updates

[Javier Tia]

xfs_parent_da_args_init() fills in every field of its xfs_da_args except
total, and the containing struct xfs_parent_args is allocated with
kmem_cache_zalloc() (xfs_parent.h:66), so runtime parent pointer updates
reach the block allocator with args->total == 0.

The log recovery path already gets this right, which is the clearest
statement of the bug.  xfs_attri_recover_work() reconstructs the same
operation from a recovered intent and does

	args->total = xfs_attr_calc_size(args, &local);	/* xfs_attr_item.c:706 */

for PPTR_SET and PPTR_REPLACE, and deliberately not for PPTR_REMOVE.  So
replaying a parent pointer insert from the log runs with a correct total
while performing the same insert at runtime runs with zero.

That field is not a constant.  xfs_da_grow_inode_int() treats it as a
running remainder:

	args->total -= dp->i_nblocks - nblks;		/* xfs_da_btree.c:2388 */

xfs_da_args.total is an xfs_extlen_t, i.e. uint32_t (xfs_types.h:14), so
subtracting the first block the attr fork gains wraps it to 0xffffffff.
It is passed down as xfs_bmapi_write()'s total argument
(xfs_da_btree.c:2348), stored as xfs_bmalloca.total, copied to
xfs_alloc_arg.total (xfs_bmap.c:3214, 3379) and finally reaches

	if (available < (int)max(args->total, alloc_len))

in xfs_alloc_space_available() (xfs_alloc.c:2525), where the cast turns
~0U back into -1 and the minimum-free-space test can no longer fail.
Parent pointer allocations therefore skip a check that every other xattr
allocation observes.

Growing the fork twice in one operation is ordinary, not a corner case:
XFS_DAS_LEAF_ADD calls xfs_attr3_leaf_to_node(), which grows the fork
(xfs_attr_leaf.c:1319), then sets XFS_DAS_NODE_ADD and returns -EAGAIN;
the next cycle can reach xfs_attr3_leaf_split() (xfs_attr_leaf.c:1462),
and a node split reaches xfs_da_grow_inode() again by way of
xfs_da3_split() (xfs_da_btree.c:748, 866).  The xfs_da_args lives across
that roll, so the later allocations are the ones that see the wrapped
value.

Set the field from xfs_attr_calc_size(), matching both the recovery path
above and xfs_attr_set() (xfs_attr.c:1150), rather than clamping the
subtraction, which would leave total meaningless for parent pointers and
hide the omission.

The initialiser is shared with five other callers and the value is inert
on all of them.  Every reader of args->total in the attr code needs
xfs_da_grow_inode(), whose only attr-fork callers are the three growth
functions in xfs_attr_leaf.c and the two split functions in
xfs_da_btree.c, and the state machine cannot reach any of them from a
remove: each remove state completes with
xfs_attr_complete_op(attr, xfs_attr_init_add_state(args)), and
xfs_attr_complete_op() replaces that add state with XFS_DAS_DONE unless
XFS_DA_OP_REPLACE is set (xfs_attr.c:497), which only the two replace
helpers ever set.  xfs_parent_lookup() never allocates at all, and on
xfs_parent_set() the assignment is immediately overwritten by
xfs_attr.c:1150, so it is dead there rather than merely unused.  Setting
it unconditionally is simpler than mirroring
xfs_attri_recover_work()'s switch.

This makes the allocator stricter for parent pointers rather than only
more correct: where total was 0 the test reduced to
available < alloc_len, and it now asks for the whole remaining
reservation, 25 blocks on a 4k-block filesystem.  That changes which AG
is chosen and can cost an extra allocator pass, but it does not
introduce a new failure.  xfs_bmap_btalloc_low_space() retries with
args->minlen and sweeps every AG before declaring ENOSPC
(xfs_bmap.c:3511-3532), and a parent-pointer link never runs
reservationless in the first place - xfs_link() refuses the resblks == 0
fallback while pptrs are enabled, precisely because it cannot back out if
the xattrs must grow (xfs_inode.c:948-954).

Fixes: b7c62d90c12c ("xfs: parent pointer attribute creation")
Signed-off-by: Javier Tia <floss@jetm.me>
---
 fs/xfs/libxfs/xfs_parent.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index 3509cc4b2175..d6588d0a9286 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -156,6 +156,8 @@ xfs_parent_da_args_init(
 	xfs_ino_t		owner,
 	const struct xfs_name	*parent_name)
 {
+	int			local;
+
 	args->geo = child->i_mount->m_attr_geo;
 	args->whichfork = XFS_ATTR_FORK;
 	args->attr_filter = XFS_ATTR_PARENT;
@@ -168,6 +170,17 @@ xfs_parent_da_args_init(
 	args->value = rec;
 	args->valuelen = sizeof(struct xfs_parent_rec);
 	xfs_attr_sethash(args);
+
+	/*
+	 * xfs_da_grow_inode_int() subtracts every block it allocates from
+	 * args->total, which is unsigned, so the zero left here by
+	 * kmem_cache_zalloc() wraps to ~0U as soon as the attr fork grows once.
+	 * Derive it the way xfs_attr_set() does instead.  A parent pointer's
+	 * value is a struct xfs_parent_rec, so the entry is always local, which
+	 * is what the ASSERT records.
+	 */
+	args->total = xfs_attr_calc_size(args, &local);
+	ASSERT(local);
 }
 
 /* Make sure the incore state is ready for a parent pointer query/update. */
-- 
Javier Tia

Re: [PATCH 1/5] xfs: initialise error in xfs_defer_finish_one()

[Darrick J. Wong]

On Sat, Aug 08, 2026 at 05:40:18PM -0600, Javier Tia wrote:
> 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.

You could have stopped here with the commit mess.age

> An item-less pending item is not hypothetical.  Of the three
> xfs_defer_alloc() callers, xfs_defer_add() always follows with
> xfs_defer_add_item(), but the other two do not.
> xfs_defer_start_recovery() is harmless because it adds to a
> caller-supplied r_dfops list rather than to tp->t_dfops, so its items
> never enter this path at all, and they are driven by
> xfs_defer_finish_recovery() and ops->recover_work() rather than by
> xfs_defer_finish_one().  xfs_defer_add_barrier() is neither:
> xfs_defer_create_intents() walks tp->t_dfops without filtering
> item-less entries, so a barrier is spliced onto the pending list and is
> eligible to be picked by xfs_defer_finish_noroll().
> xfs_reap_ag_blocks() adds one every other extent, so online repair
> reaches this on any filesystem built with CONFIG_XFS_ONLINE_REPAIR.
> 
> The consequence is a filesystem shutdown that depends on stack
> contents.  xfs_defer_finish_noroll() treats any non--EAGAIN return as
> fatal and calls xfs_force_shutdown(SHUTDOWN_CORRUPT_INCORE), so
> whenever the uninitialised value happens to be non-zero a successful
> barrier is reported as in-core corruption and the filesystem is taken
> down in the middle of a repair.  ops->finish_cleanup() also receives
> the same value where an op type provides one, though no op type that
> can reach the empty-list path defines one.
> 
> Returning zero is the correct result rather than a papered-over error,
> and not only because the barrier type deliberately has no work items:
> reaching the free path at all means the item loop drained without a
> non-zero error, so zero is the truthful value for any op type.
> 
> The uninitialised declaration is older than the Fixes: commit below,
> but that commit is where the bug became reachable - it added
> xfs_defer_add_barrier(), the barrier op type and the only caller of it
> in one go, and before it no item-less pending item could exist.
> 
> Fixes: 3f3cec031099 ("xfs: force small EFIs for reaping btree extents")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Javier Tia <floss@jetm.me>
> ---
>  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;

We should really just turn on automatic zeroing of automatic variable.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  
>  	trace_xfs_defer_pending_finish(tp->t_mountp, dfp);
>  
> -- 
> Javier Tia
> 

Re: [PATCH 2/5] xfs: give the deferred barrier op type a name

[Darrick J. Wong]

On Sat, Aug 08, 2026 at 05:40:19PM -0600, Javier Tia wrote:
> 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>

Looks ok,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  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
> 

Re: [PATCH 4/5] xfs: correct the parent pointer space reservation comment

[Darrick J. Wong]

On Sat, Aug 08, 2026 at 05:40:21PM -0600, Javier Tia wrote:
> 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 an inode with N hardlinks carries N of them,
> and `xfs_io -c "parent -p"` on a 31-link file lists 31.  By the Nth link
> the attr fork is in leaf or node format and the insert is not into a
> fresh tree.
> 
> The reservation itself is fine, which is what makes the comment worth
> fixing rather than the code.  XFS_DAENTER_SPACE_RES() reserves
> XFS_DA_NODE_MAXDEPTH blocks plus a bmap allowance for each, i.e. enough
> to split every level of a maximum-depth attr dabtree.  That depth is a
> format ceiling, not a runtime property, so the result cannot depend on
> the format the fork happens to be in.  Anyone auditing a reservation
> shortfall here reads the comment, concludes the sizing rests on an
> assumption that demonstrably does not hold, and goes looking for a bug
> that is not there.
> 
> Record why no double split allowance is needed either, since that is one
> of two visible differences from xfs_attr_calc_size() and is not obvious
> from the expression: a parent pointer's name is a dirent name and its
> value is a struct xfs_parent_rec, so the leaf entry is local and at most
> round_up(3 + 255 + 12, 4) = 272 bytes.  Parent pointers require V5 and
> therefore XFS_MIN_CRC_BLOCKSIZE, so the smallest half-block this can be
> compared against is 512 and the double split branch is unreachable on
> every mountable geometry.  Locality is decided against a different
> threshold, xfs_attr_leaf_entsize_local_max() at three quarters of a
> block, which the 272 bytes also clears.
> 
> Record the other difference too.  The second term hands a byte count to
> XFS_NEXTENTADD_SPACE_RES(), whose parameter counts mappings, so it asks
> for more extent-add allowance than the one mapping a parent pointer
> adds.  The factor depends on the block size, because the macro divides
> by XFS_MAX_CONTIG_EXTENTS_PER_BLOCK(), so the comment says only that it
> over-reserves - a patch whose whole point is that the old comment stated
> a geometry-dependent thing as invariant should not do the same.  That it
> over-reserves is why it is not a bug and why this patch leaves it alone.
> 
> Signed-off-by: Javier Tia <floss@jetm.me>
> ---
>  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.

We should probably reduce this some day (new feature bit), but in the
meantime this checks out.  The commit message could be a lot shorter
since you don't need to reiterate the diff there....

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  	 */
>  	return XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK) +
>  	       XFS_NEXTENTADD_SPACE_RES(mp, namelen, XFS_ATTR_FORK);
> -- 
> Javier Tia
>