* [PATCH v2] xfs: fix internal error from AGFL exhaustion
@ 2023-10-30 21:00 Omar Sandoval
2023-10-30 21:00 ` [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits Omar Sandoval
2023-10-31 21:24 ` [PATCH v2] xfs: fix internal error from AGFL exhaustion Darrick J. Wong
0 siblings, 2 replies; 9+ messages in thread
From: Omar Sandoval @ 2023-10-30 21:00 UTC (permalink / raw)
To: linux-xfs; +Cc: kernel-team, Darrick J . Wong, Dave Chinner
From: Omar Sandoval <osandov@fb.com>
We've been seeing XFS errors like the following:
XFS: Internal error i != 1 at line 3526 of file fs/xfs/libxfs/xfs_btree.c. Caller xfs_btree_insert+0x1ec/0x280
...
Call Trace:
xfs_corruption_error+0x94/0xa0
xfs_btree_insert+0x221/0x280
xfs_alloc_fixup_trees+0x104/0x3e0
xfs_alloc_ag_vextent_size+0x667/0x820
xfs_alloc_fix_freelist+0x5d9/0x750
xfs_free_extent_fix_freelist+0x65/0xa0
__xfs_free_extent+0x57/0x180
...
This is the XFS_IS_CORRUPT() check in xfs_btree_insert() when
xfs_btree_insrec() fails.
After converting this into a panic and dissecting the core dump, I found
that xfs_btree_insrec() is failing because it's trying to split a leaf
node in the cntbt when the AG free list is empty. In particular, it's
failing to get a block from the AGFL _while trying to refill the AGFL_.
If a single operation splits every level of the bnobt and the cntbt (and
the rmapbt if it is enabled) at once, the free list will be empty. Then,
when the next operation tries to refill the free list, it allocates
space. If the allocation does not use a full extent, it will need to
insert records for the remaining space in the bnobt and cntbt. And if
those new records go in full leaves, the leaves (and potentially more
nodes up to the old root) need to be split.
Fix it by accounting for the additional splits that may be required to
refill the free list in the calculation for the minimum free list size.
P.S. As far as I can tell, this bug has existed for a long time -- maybe
back to xfs-history commit afdf80ae7405 ("Add XFS_AG_MAXLEVELS macros
...") in April 1994! It requires a very unlucky sequence of events, and
in fact we didn't hit it until a particular sparse mmap workload updated
from 5.12 to 5.19. But this bug existed in 5.12, so it must've been
exposed by some other change in allocation or writeback patterns. It's
also much less likely to be hit with the rmapbt enabled, since that
increases the minimum free list size and is unlikely to split at the
same time as the bnobt and cntbt.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Changes since v1 [1]:
- Updated calculation to account for internal nodes that may also need
to be split.
- Updated comments and commit message accordingly.
1: https://lore.kernel.org/linux-xfs/ZTrSiwF7OAq0hJHn@dread.disaster.area/T/
fs/xfs/libxfs/xfs_alloc.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 3069194527dd..3949c6ad0cce 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2275,16 +2275,35 @@ xfs_alloc_min_freelist(
ASSERT(mp->m_alloc_maxlevels > 0);
+ /*
+ * For a btree shorter than the maximum height, the worst case is that
+ * every level gets split and a new level is added, then while inserting
+ * another entry to refill the AGFL, every level under the old root gets
+ * split again. This is:
+ *
+ * (current height + 1) + (current height - 1)
+ * = (new height) + (new height - 2)
+ * = 2 * new height - 2
+ *
+ * For a btree of maximum height, the worst case is that every level
+ * under the root gets split, then while inserting another entry to
+ * refill the AGFL, every level under the root gets split again. This is
+ * also:
+ *
+ * 2 * (new_height - 1)
+ * = 2 * new height - 2
+ */
+
/* space needed by-bno freespace btree */
min_free = min_t(unsigned int, levels[XFS_BTNUM_BNOi] + 1,
- mp->m_alloc_maxlevels);
+ mp->m_alloc_maxlevels) * 2 - 2;
/* space needed by-size freespace btree */
min_free += min_t(unsigned int, levels[XFS_BTNUM_CNTi] + 1,
- mp->m_alloc_maxlevels);
+ mp->m_alloc_maxlevels) * 2 - 2;
/* space needed reverse mapping used space btree */
if (xfs_has_rmapbt(mp))
min_free += min_t(unsigned int, levels[XFS_BTNUM_RMAPi] + 1,
- mp->m_rmap_maxlevels);
+ mp->m_rmap_maxlevels) * 2 - 2;
return min_free;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-10-30 21:00 [PATCH v2] xfs: fix internal error from AGFL exhaustion Omar Sandoval
@ 2023-10-30 21:00 ` Omar Sandoval
2023-10-31 21:24 ` Darrick J. Wong
2023-11-01 6:45 ` Zorro Lang
2023-10-31 21:24 ` [PATCH v2] xfs: fix internal error from AGFL exhaustion Darrick J. Wong
1 sibling, 2 replies; 9+ messages in thread
From: Omar Sandoval @ 2023-10-30 21:00 UTC (permalink / raw)
To: fstests, linux-xfs; +Cc: kernel-team, Darrick J . Wong, Dave Chinner
This is a regression test for patch "xfs: fix internal error from AGFL
exhaustion"), which is not yet merged. Without the fix, it will fail
with a "Structure needs cleaning" error.
Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
Changes since v1 [1]:
- Fixed to check whether mkfs.xfs supports -m rmapbt.
- Changed bare $XFS_DB calls to _scratch_xfs_db.
- Expanded comment about what happens without the fix.
I didn't add a check for whether everything ended up in AG 0, because it
wasn't clear to me what to do in that case. We could skip the test, but
it also doesn't hurt to run it anyways.
1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
tests/xfs/601.out | 2 ++
2 files changed, 70 insertions(+)
create mode 100755 tests/xfs/601
create mode 100644 tests/xfs/601.out
diff --git a/tests/xfs/601 b/tests/xfs/601
new file mode 100755
index 00000000..68df6ac0
--- /dev/null
+++ b/tests/xfs/601
@@ -0,0 +1,68 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) Meta Platforms, Inc. and affiliates.
+#
+# FS QA Test 601
+#
+# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
+#
+. ./common/preamble
+_begin_fstest auto prealloc punch
+
+. ./common/filter
+
+_supported_fs xfs
+_require_scratch
+_require_test_program punch-alternating
+_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
+
+# Disable the rmapbt so we only need to worry about splitting the bnobt and
+# cntbt at the same time.
+opts=
+if $MKFS_XFS_PROG |& grep -q rmapbt; then
+ opts="-m rmapbt=0"
+fi
+_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
+. "$tmp.mkfs"
+_scratch_mount
+
+alloc_block_len=$((_fs_has_crcs ? 56 : 16))
+allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
+allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
+
+# Create a big file with a size such that the punches below create the exact
+# free extents we want.
+num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
+$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
+
+# Fill in any small free extents in AG 0. After this, there should be only one,
+# large free extent.
+_scratch_unmount
+mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
+ $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
+ tac | tail -n +2)
+_scratch_mount
+for gap_i in "${!gaps[@]}"; do
+ gap=${gaps[$gap_i]}
+ $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
+done
+
+# Create enough free space records to make the bnobt and cntbt both full,
+# 2-level trees, plus one more record to make them split all the way to the
+# root and become 3-level trees. After this, there is a 7-block free extent in
+# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
+# than the rightmost two are full. Without the fix, the free list is also
+# empty.
+$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
+"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
+
+# Do an arbitrary operation that refills the free list. Without the fix, this
+# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
+# the cntbt, then try to insert the remaining 1 block free extent in the
+# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
+# leaf and fails because the free list is empty, returning EFSCORRUPTED.
+$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/xfs/601.out b/tests/xfs/601.out
new file mode 100644
index 00000000..0d70c3e5
--- /dev/null
+++ b/tests/xfs/601.out
@@ -0,0 +1,2 @@
+QA output created by 601
+Silence is golden
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] xfs: fix internal error from AGFL exhaustion
2023-10-30 21:00 [PATCH v2] xfs: fix internal error from AGFL exhaustion Omar Sandoval
2023-10-30 21:00 ` [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits Omar Sandoval
@ 2023-10-31 21:24 ` Darrick J. Wong
2023-10-31 21:59 ` Dave Chinner
1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2023-10-31 21:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-xfs, kernel-team, Dave Chinner
On Mon, Oct 30, 2023 at 02:00:02PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> We've been seeing XFS errors like the following:
>
> XFS: Internal error i != 1 at line 3526 of file fs/xfs/libxfs/xfs_btree.c. Caller xfs_btree_insert+0x1ec/0x280
> ...
> Call Trace:
> xfs_corruption_error+0x94/0xa0
> xfs_btree_insert+0x221/0x280
> xfs_alloc_fixup_trees+0x104/0x3e0
> xfs_alloc_ag_vextent_size+0x667/0x820
> xfs_alloc_fix_freelist+0x5d9/0x750
> xfs_free_extent_fix_freelist+0x65/0xa0
> __xfs_free_extent+0x57/0x180
> ...
>
> This is the XFS_IS_CORRUPT() check in xfs_btree_insert() when
> xfs_btree_insrec() fails.
>
> After converting this into a panic and dissecting the core dump, I found
> that xfs_btree_insrec() is failing because it's trying to split a leaf
> node in the cntbt when the AG free list is empty. In particular, it's
> failing to get a block from the AGFL _while trying to refill the AGFL_.
>
> If a single operation splits every level of the bnobt and the cntbt (and
> the rmapbt if it is enabled) at once, the free list will be empty. Then,
> when the next operation tries to refill the free list, it allocates
> space. If the allocation does not use a full extent, it will need to
> insert records for the remaining space in the bnobt and cntbt. And if
> those new records go in full leaves, the leaves (and potentially more
> nodes up to the old root) need to be split.
>
> Fix it by accounting for the additional splits that may be required to
> refill the free list in the calculation for the minimum free list size.
>
> P.S. As far as I can tell, this bug has existed for a long time -- maybe
> back to xfs-history commit afdf80ae7405 ("Add XFS_AG_MAXLEVELS macros
> ...") in April 1994! It requires a very unlucky sequence of events, and
> in fact we didn't hit it until a particular sparse mmap workload updated
> from 5.12 to 5.19. But this bug existed in 5.12, so it must've been
> exposed by some other change in allocation or writeback patterns. It's
> also much less likely to be hit with the rmapbt enabled, since that
> increases the minimum free list size and is unlikely to split at the
> same time as the bnobt and cntbt.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> Changes since v1 [1]:
>
> - Updated calculation to account for internal nodes that may also need
> to be split.
> - Updated comments and commit message accordingly.
>
> 1: https://lore.kernel.org/linux-xfs/ZTrSiwF7OAq0hJHn@dread.disaster.area/T/
>
> fs/xfs/libxfs/xfs_alloc.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 3069194527dd..3949c6ad0cce 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2275,16 +2275,35 @@ xfs_alloc_min_freelist(
>
> ASSERT(mp->m_alloc_maxlevels > 0);
>
> + /*
> + * For a btree shorter than the maximum height, the worst case is that
> + * every level gets split and a new level is added, then while inserting
> + * another entry to refill the AGFL, every level under the old root gets
> + * split again. This is:
> + *
> + * (current height + 1) + (current height - 1)
Could you make this comment define this relationship more explicitly?
* (full height split reservation) + (AGFL refill split height)
* (current height + 1) + (current height - 1)
With that added,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> + * = (new height) + (new height - 2)
> + * = 2 * new height - 2
> + *
> + * For a btree of maximum height, the worst case is that every level
> + * under the root gets split, then while inserting another entry to
> + * refill the AGFL, every level under the root gets split again. This is
> + * also:
> + *
> + * 2 * (new_height - 1)
> + * = 2 * new height - 2
> + */
> +
> /* space needed by-bno freespace btree */
> min_free = min_t(unsigned int, levels[XFS_BTNUM_BNOi] + 1,
> - mp->m_alloc_maxlevels);
> + mp->m_alloc_maxlevels) * 2 - 2;
> /* space needed by-size freespace btree */
> min_free += min_t(unsigned int, levels[XFS_BTNUM_CNTi] + 1,
> - mp->m_alloc_maxlevels);
> + mp->m_alloc_maxlevels) * 2 - 2;
> /* space needed reverse mapping used space btree */
> if (xfs_has_rmapbt(mp))
> min_free += min_t(unsigned int, levels[XFS_BTNUM_RMAPi] + 1,
> - mp->m_rmap_maxlevels);
> + mp->m_rmap_maxlevels) * 2 - 2;
>
> return min_free;
> }
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-10-30 21:00 ` [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits Omar Sandoval
@ 2023-10-31 21:24 ` Darrick J. Wong
2023-11-01 6:45 ` Zorro Lang
1 sibling, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2023-10-31 21:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: fstests, linux-xfs, kernel-team, Dave Chinner
On Mon, Oct 30, 2023 at 02:00:15PM -0700, Omar Sandoval wrote:
> This is a regression test for patch "xfs: fix internal error from AGFL
> exhaustion"), which is not yet merged. Without the fix, it will fail
> with a "Structure needs cleaning" error.
>
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
Looks good now,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> Changes since v1 [1]:
>
> - Fixed to check whether mkfs.xfs supports -m rmapbt.
> - Changed bare $XFS_DB calls to _scratch_xfs_db.
> - Expanded comment about what happens without the fix.
>
> I didn't add a check for whether everything ended up in AG 0, because it
> wasn't clear to me what to do in that case. We could skip the test, but
> it also doesn't hurt to run it anyways.
>
> 1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
>
> tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/601.out | 2 ++
> 2 files changed, 70 insertions(+)
> create mode 100755 tests/xfs/601
> create mode 100644 tests/xfs/601.out
>
> diff --git a/tests/xfs/601 b/tests/xfs/601
> new file mode 100755
> index 00000000..68df6ac0
> --- /dev/null
> +++ b/tests/xfs/601
> @@ -0,0 +1,68 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) Meta Platforms, Inc. and affiliates.
> +#
> +# FS QA Test 601
> +#
> +# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
> +#
> +. ./common/preamble
> +_begin_fstest auto prealloc punch
> +
> +. ./common/filter
> +
> +_supported_fs xfs
> +_require_scratch
> +_require_test_program punch-alternating
> +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
> +
> +# Disable the rmapbt so we only need to worry about splitting the bnobt and
> +# cntbt at the same time.
> +opts=
> +if $MKFS_XFS_PROG |& grep -q rmapbt; then
> + opts="-m rmapbt=0"
> +fi
> +_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
> +. "$tmp.mkfs"
> +_scratch_mount
> +
> +alloc_block_len=$((_fs_has_crcs ? 56 : 16))
> +allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
> +allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
> +
> +# Create a big file with a size such that the punches below create the exact
> +# free extents we want.
> +num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
> +$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
> +
> +# Fill in any small free extents in AG 0. After this, there should be only one,
> +# large free extent.
> +_scratch_unmount
> +mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
> + $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
> + tac | tail -n +2)
> +_scratch_mount
> +for gap_i in "${!gaps[@]}"; do
> + gap=${gaps[$gap_i]}
> + $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
> +done
> +
> +# Create enough free space records to make the bnobt and cntbt both full,
> +# 2-level trees, plus one more record to make them split all the way to the
> +# root and become 3-level trees. After this, there is a 7-block free extent in
> +# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
> +# than the rightmost two are full. Without the fix, the free list is also
> +# empty.
> +$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
> +"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
> +
> +# Do an arbitrary operation that refills the free list. Without the fix, this
> +# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
> +# the cntbt, then try to insert the remaining 1 block free extent in the
> +# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
> +# leaf and fails because the free list is empty, returning EFSCORRUPTED.
> +$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/601.out b/tests/xfs/601.out
> new file mode 100644
> index 00000000..0d70c3e5
> --- /dev/null
> +++ b/tests/xfs/601.out
> @@ -0,0 +1,2 @@
> +QA output created by 601
> +Silence is golden
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] xfs: fix internal error from AGFL exhaustion
2023-10-31 21:24 ` [PATCH v2] xfs: fix internal error from AGFL exhaustion Darrick J. Wong
@ 2023-10-31 21:59 ` Dave Chinner
0 siblings, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2023-10-31 21:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Omar Sandoval, linux-xfs, kernel-team, Dave Chinner
On Tue, Oct 31, 2023 at 02:24:00PM -0700, Darrick J. Wong wrote:
> On Mon, Oct 30, 2023 at 02:00:02PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > We've been seeing XFS errors like the following:
> >
> > XFS: Internal error i != 1 at line 3526 of file fs/xfs/libxfs/xfs_btree.c. Caller xfs_btree_insert+0x1ec/0x280
> > ...
> > Call Trace:
> > xfs_corruption_error+0x94/0xa0
> > xfs_btree_insert+0x221/0x280
> > xfs_alloc_fixup_trees+0x104/0x3e0
> > xfs_alloc_ag_vextent_size+0x667/0x820
> > xfs_alloc_fix_freelist+0x5d9/0x750
> > xfs_free_extent_fix_freelist+0x65/0xa0
> > __xfs_free_extent+0x57/0x180
> > ...
> >
> > This is the XFS_IS_CORRUPT() check in xfs_btree_insert() when
> > xfs_btree_insrec() fails.
> >
> > After converting this into a panic and dissecting the core dump, I found
> > that xfs_btree_insrec() is failing because it's trying to split a leaf
> > node in the cntbt when the AG free list is empty. In particular, it's
> > failing to get a block from the AGFL _while trying to refill the AGFL_.
> >
> > If a single operation splits every level of the bnobt and the cntbt (and
> > the rmapbt if it is enabled) at once, the free list will be empty. Then,
> > when the next operation tries to refill the free list, it allocates
> > space. If the allocation does not use a full extent, it will need to
> > insert records for the remaining space in the bnobt and cntbt. And if
> > those new records go in full leaves, the leaves (and potentially more
> > nodes up to the old root) need to be split.
> >
> > Fix it by accounting for the additional splits that may be required to
> > refill the free list in the calculation for the minimum free list size.
> >
> > P.S. As far as I can tell, this bug has existed for a long time -- maybe
> > back to xfs-history commit afdf80ae7405 ("Add XFS_AG_MAXLEVELS macros
> > ...") in April 1994! It requires a very unlucky sequence of events, and
> > in fact we didn't hit it until a particular sparse mmap workload updated
> > from 5.12 to 5.19. But this bug existed in 5.12, so it must've been
> > exposed by some other change in allocation or writeback patterns. It's
> > also much less likely to be hit with the rmapbt enabled, since that
> > increases the minimum free list size and is unlikely to split at the
> > same time as the bnobt and cntbt.
> >
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> > ---
> > Changes since v1 [1]:
> >
> > - Updated calculation to account for internal nodes that may also need
> > to be split.
> > - Updated comments and commit message accordingly.
> >
> > 1: https://lore.kernel.org/linux-xfs/ZTrSiwF7OAq0hJHn@dread.disaster.area/T/
> >
> > fs/xfs/libxfs/xfs_alloc.c | 25 ++++++++++++++++++++++---
> > 1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > index 3069194527dd..3949c6ad0cce 100644
> > --- a/fs/xfs/libxfs/xfs_alloc.c
> > +++ b/fs/xfs/libxfs/xfs_alloc.c
> > @@ -2275,16 +2275,35 @@ xfs_alloc_min_freelist(
> >
> > ASSERT(mp->m_alloc_maxlevels > 0);
> >
> > + /*
> > + * For a btree shorter than the maximum height, the worst case is that
> > + * every level gets split and a new level is added, then while inserting
> > + * another entry to refill the AGFL, every level under the old root gets
> > + * split again. This is:
> > + *
> > + * (current height + 1) + (current height - 1)
>
> Could you make this comment define this relationship more explicitly?
>
> * (full height split reservation) + (AGFL refill split height)
> * (current height + 1) + (current height - 1)
>
> With that added,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Yup, with that comment change I'm happy with it, too.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-10-30 21:00 ` [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits Omar Sandoval
2023-10-31 21:24 ` Darrick J. Wong
@ 2023-11-01 6:45 ` Zorro Lang
2023-11-01 15:56 ` Darrick J. Wong
1 sibling, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2023-11-01 6:45 UTC (permalink / raw)
To: Omar Sandoval, Darrick J . Wong; +Cc: fstests, linux-xfs, kernel-team
On Mon, Oct 30, 2023 at 02:00:15PM -0700, Omar Sandoval wrote:
> This is a regression test for patch "xfs: fix internal error from AGFL
> exhaustion"), which is not yet merged. Without the fix, it will fail
> with a "Structure needs cleaning" error.
>
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> ---
> Changes since v1 [1]:
>
> - Fixed to check whether mkfs.xfs supports -m rmapbt.
> - Changed bare $XFS_DB calls to _scratch_xfs_db.
> - Expanded comment about what happens without the fix.
>
> I didn't add a check for whether everything ended up in AG 0, because it
> wasn't clear to me what to do in that case. We could skip the test, but
> it also doesn't hurt to run it anyways.
>
> 1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
>
> tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/601.out | 2 ++
The xfs/601 has been taken by:
39f88c55 ("generic: test FALLOC_FL_UNSHARE when pagecache is not loaded")
I'll change this case to another number.
...
Hi Darrick, I just noticed that commit has "generic", but that's a case in
tests/xfs, and there's not "_supported_fs xfs" in xfs/601. Do you want to
move it to be a generic case?
> 2 files changed, 70 insertions(+)
> create mode 100755 tests/xfs/601
> create mode 100644 tests/xfs/601.out
>
> diff --git a/tests/xfs/601 b/tests/xfs/601
> new file mode 100755
> index 00000000..68df6ac0
> --- /dev/null
> +++ b/tests/xfs/601
> @@ -0,0 +1,68 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) Meta Platforms, Inc. and affiliates.
> +#
> +# FS QA Test 601
> +#
> +# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
> +#
> +. ./common/preamble
> +_begin_fstest auto prealloc punch
> +
> +. ./common/filter
> +
> +_supported_fs xfs
> +_require_scratch
_require_xfs_io_command "fpunch" ?
I can help to add that, others look good to me.
Reviewed-by: Zorro Lang <zlang@redhat.com>
> +_require_test_program punch-alternating
> +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
> +
> +# Disable the rmapbt so we only need to worry about splitting the bnobt and
> +# cntbt at the same time.
> +opts=
> +if $MKFS_XFS_PROG |& grep -q rmapbt; then
> + opts="-m rmapbt=0"
> +fi
> +_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
> +. "$tmp.mkfs"
> +_scratch_mount
> +
> +alloc_block_len=$((_fs_has_crcs ? 56 : 16))
> +allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
> +allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
> +
> +# Create a big file with a size such that the punches below create the exact
> +# free extents we want.
> +num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
> +$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
> +
> +# Fill in any small free extents in AG 0. After this, there should be only one,
> +# large free extent.
> +_scratch_unmount
> +mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
> + $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
> + tac | tail -n +2)
> +_scratch_mount
> +for gap_i in "${!gaps[@]}"; do
> + gap=${gaps[$gap_i]}
> + $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
> +done
> +
> +# Create enough free space records to make the bnobt and cntbt both full,
> +# 2-level trees, plus one more record to make them split all the way to the
> +# root and become 3-level trees. After this, there is a 7-block free extent in
> +# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
> +# than the rightmost two are full. Without the fix, the free list is also
> +# empty.
> +$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
> +"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
> +
> +# Do an arbitrary operation that refills the free list. Without the fix, this
> +# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
> +# the cntbt, then try to insert the remaining 1 block free extent in the
> +# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
> +# leaf and fails because the free list is empty, returning EFSCORRUPTED.
> +$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/601.out b/tests/xfs/601.out
> new file mode 100644
> index 00000000..0d70c3e5
> --- /dev/null
> +++ b/tests/xfs/601.out
> @@ -0,0 +1,2 @@
> +QA output created by 601
> +Silence is golden
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-11-01 6:45 ` Zorro Lang
@ 2023-11-01 15:56 ` Darrick J. Wong
2023-11-17 14:32 ` Zorro Lang
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2023-11-01 15:56 UTC (permalink / raw)
To: Zorro Lang; +Cc: Omar Sandoval, fstests, linux-xfs, kernel-team
On Wed, Nov 01, 2023 at 02:45:43PM +0800, Zorro Lang wrote:
> On Mon, Oct 30, 2023 at 02:00:15PM -0700, Omar Sandoval wrote:
> > This is a regression test for patch "xfs: fix internal error from AGFL
> > exhaustion"), which is not yet merged. Without the fix, it will fail
> > with a "Structure needs cleaning" error.
> >
> > Signed-off-by: Omar Sandoval <osandov@osandov.com>
> > ---
> > Changes since v1 [1]:
> >
> > - Fixed to check whether mkfs.xfs supports -m rmapbt.
> > - Changed bare $XFS_DB calls to _scratch_xfs_db.
> > - Expanded comment about what happens without the fix.
> >
> > I didn't add a check for whether everything ended up in AG 0, because it
> > wasn't clear to me what to do in that case. We could skip the test, but
> > it also doesn't hurt to run it anyways.
> >
> > 1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
> >
> > tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/601.out | 2 ++
>
> The xfs/601 has been taken by:
> 39f88c55 ("generic: test FALLOC_FL_UNSHARE when pagecache is not loaded")
>
> I'll change this case to another number.
>
> ...
> Hi Darrick, I just noticed that commit has "generic", but that's a case in
> tests/xfs, and there's not "_supported_fs xfs" in xfs/601. Do you want to
> move it to be a generic case?
Yes, the existing xfs/601 regression test (kernel commit 35d30c9cf127)
can become a generic test; and then this one can take its place.
--D
> > 2 files changed, 70 insertions(+)
> > create mode 100755 tests/xfs/601
> > create mode 100644 tests/xfs/601.out
> >
> > diff --git a/tests/xfs/601 b/tests/xfs/601
> > new file mode 100755
> > index 00000000..68df6ac0
> > --- /dev/null
> > +++ b/tests/xfs/601
> > @@ -0,0 +1,68 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) Meta Platforms, Inc. and affiliates.
> > +#
> > +# FS QA Test 601
> > +#
> > +# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto prealloc punch
> > +
> > +. ./common/filter
> > +
> > +_supported_fs xfs
> > +_require_scratch
>
> _require_xfs_io_command "fpunch" ?
>
> I can help to add that, others look good to me.
>
> Reviewed-by: Zorro Lang <zlang@redhat.com>
>
> > +_require_test_program punch-alternating
> > +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
> > +
> > +# Disable the rmapbt so we only need to worry about splitting the bnobt and
> > +# cntbt at the same time.
> > +opts=
> > +if $MKFS_XFS_PROG |& grep -q rmapbt; then
> > + opts="-m rmapbt=0"
> > +fi
> > +_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
> > +. "$tmp.mkfs"
> > +_scratch_mount
> > +
> > +alloc_block_len=$((_fs_has_crcs ? 56 : 16))
> > +allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
> > +allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
> > +
> > +# Create a big file with a size such that the punches below create the exact
> > +# free extents we want.
> > +num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
> > +$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
> > +
> > +# Fill in any small free extents in AG 0. After this, there should be only one,
> > +# large free extent.
> > +_scratch_unmount
> > +mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
> > + $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
> > + tac | tail -n +2)
> > +_scratch_mount
> > +for gap_i in "${!gaps[@]}"; do
> > + gap=${gaps[$gap_i]}
> > + $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
> > +done
> > +
> > +# Create enough free space records to make the bnobt and cntbt both full,
> > +# 2-level trees, plus one more record to make them split all the way to the
> > +# root and become 3-level trees. After this, there is a 7-block free extent in
> > +# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
> > +# than the rightmost two are full. Without the fix, the free list is also
> > +# empty.
> > +$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
> > +"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
> > +
> > +# Do an arbitrary operation that refills the free list. Without the fix, this
> > +# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
> > +# the cntbt, then try to insert the remaining 1 block free extent in the
> > +# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
> > +# leaf and fails because the free list is empty, returning EFSCORRUPTED.
> > +$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
> > +
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/xfs/601.out b/tests/xfs/601.out
> > new file mode 100644
> > index 00000000..0d70c3e5
> > --- /dev/null
> > +++ b/tests/xfs/601.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 601
> > +Silence is golden
> > --
> > 2.41.0
> >
> >
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-11-01 15:56 ` Darrick J. Wong
@ 2023-11-17 14:32 ` Zorro Lang
2023-11-17 22:07 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2023-11-17 14:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Omar Sandoval, fstests, linux-xfs, kernel-team
On Wed, Nov 01, 2023 at 08:56:30AM -0700, Darrick J. Wong wrote:
> On Wed, Nov 01, 2023 at 02:45:43PM +0800, Zorro Lang wrote:
> > On Mon, Oct 30, 2023 at 02:00:15PM -0700, Omar Sandoval wrote:
> > > This is a regression test for patch "xfs: fix internal error from AGFL
> > > exhaustion"), which is not yet merged. Without the fix, it will fail
> > > with a "Structure needs cleaning" error.
> > >
> > > Signed-off-by: Omar Sandoval <osandov@osandov.com>
> > > ---
> > > Changes since v1 [1]:
> > >
> > > - Fixed to check whether mkfs.xfs supports -m rmapbt.
> > > - Changed bare $XFS_DB calls to _scratch_xfs_db.
> > > - Expanded comment about what happens without the fix.
> > >
> > > I didn't add a check for whether everything ended up in AG 0, because it
> > > wasn't clear to me what to do in that case. We could skip the test, but
> > > it also doesn't hurt to run it anyways.
> > >
> > > 1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
> > >
> > > tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
> > > tests/xfs/601.out | 2 ++
> >
> > The xfs/601 has been taken by:
> > 39f88c55 ("generic: test FALLOC_FL_UNSHARE when pagecache is not loaded")
> >
> > I'll change this case to another number.
> >
> > ...
> > Hi Darrick, I just noticed that commit has "generic", but that's a case in
> > tests/xfs, and there's not "_supported_fs xfs" in xfs/601. Do you want to
> > move it to be a generic case?
>
> Yes, the existing xfs/601 regression test (kernel commit 35d30c9cf127)
> can become a generic test; and then this one can take its place.
If you need to move current xfs/601 to generic/, better to have another patch.
I'll merge this patch in xfs/ at first. Then you can do that moving in your next
"random fix" patchset, or I can send a patch to do that.
Thanks,
Zorro
>
> --D
>
> > > 2 files changed, 70 insertions(+)
> > > create mode 100755 tests/xfs/601
> > > create mode 100644 tests/xfs/601.out
> > >
> > > diff --git a/tests/xfs/601 b/tests/xfs/601
> > > new file mode 100755
> > > index 00000000..68df6ac0
> > > --- /dev/null
> > > +++ b/tests/xfs/601
> > > @@ -0,0 +1,68 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) Meta Platforms, Inc. and affiliates.
> > > +#
> > > +# FS QA Test 601
> > > +#
> > > +# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto prealloc punch
> > > +
> > > +. ./common/filter
> > > +
> > > +_supported_fs xfs
> > > +_require_scratch
> >
> > _require_xfs_io_command "fpunch" ?
> >
> > I can help to add that, others look good to me.
> >
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> >
> > > +_require_test_program punch-alternating
> > > +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
> > > +
> > > +# Disable the rmapbt so we only need to worry about splitting the bnobt and
> > > +# cntbt at the same time.
> > > +opts=
> > > +if $MKFS_XFS_PROG |& grep -q rmapbt; then
> > > + opts="-m rmapbt=0"
> > > +fi
> > > +_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
> > > +. "$tmp.mkfs"
> > > +_scratch_mount
> > > +
> > > +alloc_block_len=$((_fs_has_crcs ? 56 : 16))
> > > +allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
> > > +allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
> > > +
> > > +# Create a big file with a size such that the punches below create the exact
> > > +# free extents we want.
> > > +num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
> > > +$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
> > > +
> > > +# Fill in any small free extents in AG 0. After this, there should be only one,
> > > +# large free extent.
> > > +_scratch_unmount
> > > +mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
> > > + $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
> > > + tac | tail -n +2)
> > > +_scratch_mount
> > > +for gap_i in "${!gaps[@]}"; do
> > > + gap=${gaps[$gap_i]}
> > > + $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
> > > +done
> > > +
> > > +# Create enough free space records to make the bnobt and cntbt both full,
> > > +# 2-level trees, plus one more record to make them split all the way to the
> > > +# root and become 3-level trees. After this, there is a 7-block free extent in
> > > +# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
> > > +# than the rightmost two are full. Without the fix, the free list is also
> > > +# empty.
> > > +$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
> > > +"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
> > > +
> > > +# Do an arbitrary operation that refills the free list. Without the fix, this
> > > +# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
> > > +# the cntbt, then try to insert the remaining 1 block free extent in the
> > > +# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
> > > +# leaf and fails because the free list is empty, returning EFSCORRUPTED.
> > > +$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
> > > +
> > > +echo "Silence is golden"
> > > +status=0
> > > +exit
> > > diff --git a/tests/xfs/601.out b/tests/xfs/601.out
> > > new file mode 100644
> > > index 00000000..0d70c3e5
> > > --- /dev/null
> > > +++ b/tests/xfs/601.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 601
> > > +Silence is golden
> > > --
> > > 2.41.0
> > >
> > >
> >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
2023-11-17 14:32 ` Zorro Lang
@ 2023-11-17 22:07 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2023-11-17 22:07 UTC (permalink / raw)
To: Zorro Lang; +Cc: Omar Sandoval, fstests, linux-xfs, kernel-team
On Fri, Nov 17, 2023 at 10:32:32PM +0800, Zorro Lang wrote:
> On Wed, Nov 01, 2023 at 08:56:30AM -0700, Darrick J. Wong wrote:
> > On Wed, Nov 01, 2023 at 02:45:43PM +0800, Zorro Lang wrote:
> > > On Mon, Oct 30, 2023 at 02:00:15PM -0700, Omar Sandoval wrote:
> > > > This is a regression test for patch "xfs: fix internal error from AGFL
> > > > exhaustion"), which is not yet merged. Without the fix, it will fail
> > > > with a "Structure needs cleaning" error.
> > > >
> > > > Signed-off-by: Omar Sandoval <osandov@osandov.com>
> > > > ---
> > > > Changes since v1 [1]:
> > > >
> > > > - Fixed to check whether mkfs.xfs supports -m rmapbt.
> > > > - Changed bare $XFS_DB calls to _scratch_xfs_db.
> > > > - Expanded comment about what happens without the fix.
> > > >
> > > > I didn't add a check for whether everything ended up in AG 0, because it
> > > > wasn't clear to me what to do in that case. We could skip the test, but
> > > > it also doesn't hurt to run it anyways.
> > > >
> > > > 1: https://lore.kernel.org/linux-xfs/c7be2fe66a297316b934ddd3a1368b14f39a9f22.1698190540.git.osandov@osandov.com/
> > > >
> > > > tests/xfs/601 | 68 +++++++++++++++++++++++++++++++++++++++++++++++
> > > > tests/xfs/601.out | 2 ++
> > >
> > > The xfs/601 has been taken by:
> > > 39f88c55 ("generic: test FALLOC_FL_UNSHARE when pagecache is not loaded")
> > >
> > > I'll change this case to another number.
> > >
> > > ...
> > > Hi Darrick, I just noticed that commit has "generic", but that's a case in
> > > tests/xfs, and there's not "_supported_fs xfs" in xfs/601. Do you want to
> > > move it to be a generic case?
> >
> > Yes, the existing xfs/601 regression test (kernel commit 35d30c9cf127)
> > can become a generic test; and then this one can take its place.
>
> If you need to move current xfs/601 to generic/, better to have another patch.
> I'll merge this patch in xfs/ at first. Then you can do that moving in your next
> "random fix" patchset, or I can send a patch to do that.
<shrug> I'll ./mvtest and send the resulting patch next week unless you
beat me to it.
--D
> Thanks,
> Zorro
>
> >
> > --D
> >
> > > > 2 files changed, 70 insertions(+)
> > > > create mode 100755 tests/xfs/601
> > > > create mode 100644 tests/xfs/601.out
> > > >
> > > > diff --git a/tests/xfs/601 b/tests/xfs/601
> > > > new file mode 100755
> > > > index 00000000..68df6ac0
> > > > --- /dev/null
> > > > +++ b/tests/xfs/601
> > > > @@ -0,0 +1,68 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) Meta Platforms, Inc. and affiliates.
> > > > +#
> > > > +# FS QA Test 601
> > > > +#
> > > > +# Regression test for patch "xfs: fix internal error from AGFL exhaustion".
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto prealloc punch
> > > > +
> > > > +. ./common/filter
> > > > +
> > > > +_supported_fs xfs
> > > > +_require_scratch
> > >
> > > _require_xfs_io_command "fpunch" ?
> > >
> > > I can help to add that, others look good to me.
> > >
> > > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > >
> > > > +_require_test_program punch-alternating
> > > > +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: fix internal error from AGFL exhaustion"
> > > > +
> > > > +# Disable the rmapbt so we only need to worry about splitting the bnobt and
> > > > +# cntbt at the same time.
> > > > +opts=
> > > > +if $MKFS_XFS_PROG |& grep -q rmapbt; then
> > > > + opts="-m rmapbt=0"
> > > > +fi
> > > > +_scratch_mkfs $opts | _filter_mkfs > /dev/null 2> "$tmp.mkfs"
> > > > +. "$tmp.mkfs"
> > > > +_scratch_mount
> > > > +
> > > > +alloc_block_len=$((_fs_has_crcs ? 56 : 16))
> > > > +allocbt_leaf_maxrecs=$(((dbsize - alloc_block_len) / 8))
> > > > +allocbt_node_maxrecs=$(((dbsize - alloc_block_len) / 12))
> > > > +
> > > > +# Create a big file with a size such that the punches below create the exact
> > > > +# free extents we want.
> > > > +num_holes=$((allocbt_leaf_maxrecs * allocbt_node_maxrecs - 1))
> > > > +$XFS_IO_PROG -c "falloc 0 $((9 * dbsize + num_holes * dbsize * 2))" -f "$SCRATCH_MNT/big"
> > > > +
> > > > +# Fill in any small free extents in AG 0. After this, there should be only one,
> > > > +# large free extent.
> > > > +_scratch_unmount
> > > > +mapfile -t gaps < <(_scratch_xfs_db -c 'agf 0' -c 'addr cntroot' -c btdump |
> > > > + $SED_PROG -rn 's/^[0-9]+:\[[0-9]+,([0-9]+)\].*/\1/p' |
> > > > + tac | tail -n +2)
> > > > +_scratch_mount
> > > > +for gap_i in "${!gaps[@]}"; do
> > > > + gap=${gaps[$gap_i]}
> > > > + $XFS_IO_PROG -c "falloc 0 $((gap * dbsize))" -f "$SCRATCH_MNT/gap$gap_i"
> > > > +done
> > > > +
> > > > +# Create enough free space records to make the bnobt and cntbt both full,
> > > > +# 2-level trees, plus one more record to make them split all the way to the
> > > > +# root and become 3-level trees. After this, there is a 7-block free extent in
> > > > +# the rightmost leaf of the cntbt, and all of the leaves of the cntbt other
> > > > +# than the rightmost two are full. Without the fix, the free list is also
> > > > +# empty.
> > > > +$XFS_IO_PROG -c "fpunch $dbsize $((7 * dbsize))" "$SCRATCH_MNT/big"
> > > > +"$here/src/punch-alternating" -o 9 "$SCRATCH_MNT/big"
> > > > +
> > > > +# Do an arbitrary operation that refills the free list. Without the fix, this
> > > > +# will allocate 6 blocks from the 7-block free extent in the rightmost leaf of
> > > > +# the cntbt, then try to insert the remaining 1 block free extent in the
> > > > +# leftmost leaf of the cntbt. But that leaf is full, so this tries to split the
> > > > +# leaf and fails because the free list is empty, returning EFSCORRUPTED.
> > > > +$XFS_IO_PROG -c "fpunch 0 $dbsize" "$SCRATCH_MNT/big"
> > > > +
> > > > +echo "Silence is golden"
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/xfs/601.out b/tests/xfs/601.out
> > > > new file mode 100644
> > > > index 00000000..0d70c3e5
> > > > --- /dev/null
> > > > +++ b/tests/xfs/601.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 601
> > > > +Silence is golden
> > > > --
> > > > 2.41.0
> > > >
> > > >
> > >
> > >
> >
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-11-17 22:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-30 21:00 [PATCH v2] xfs: fix internal error from AGFL exhaustion Omar Sandoval
2023-10-30 21:00 ` [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits Omar Sandoval
2023-10-31 21:24 ` Darrick J. Wong
2023-11-01 6:45 ` Zorro Lang
2023-11-01 15:56 ` Darrick J. Wong
2023-11-17 14:32 ` Zorro Lang
2023-11-17 22:07 ` Darrick J. Wong
2023-10-31 21:24 ` [PATCH v2] xfs: fix internal error from AGFL exhaustion Darrick J. Wong
2023-10-31 21:59 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox