public inbox for fstests@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
       [not found] <68cd85697855f686529829a2825b044913148caf.1698699188.git.osandov@fb.com>
@ 2023-10-30 21:00 ` Omar Sandoval
  2023-10-31 21:24   ` Darrick J. Wong
  2023-11-01  6:45   ` Zorro Lang
  0 siblings, 2 replies; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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
  2023-11-19 13:21           ` Zorro Lang
  0 siblings, 1 reply; 7+ 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] 7+ messages in thread

* Re: [PATCH fstests v2] xfs: test refilling AGFL after lots of btree splits
  2023-11-17 22:07         ` Darrick J. Wong
@ 2023-11-19 13:21           ` Zorro Lang
  0 siblings, 0 replies; 7+ messages in thread
From: Zorro Lang @ 2023-11-19 13:21 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests

On Fri, Nov 17, 2023 at 02:07:32PM -0800, Darrick J. Wong wrote:
> 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.

Don't worry, this's a "peace and love" mailing list ;-)

> 
> --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] 7+ messages in thread

end of thread, other threads:[~2023-11-19 13:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <68cd85697855f686529829a2825b044913148caf.1698699188.git.osandov@fb.com>
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-11-19 13:21           ` Zorro Lang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox