On Mon, Feb 12, 2024 at 11:41:14AM -0500, Josef Bacik wrote:
> Btrfs had a deadlock that you could trigger by mmap'ing a large file and
> using that as the buffer for fiemap. This test adds a c program to do
> this, and the fstest creates a large enough file and then runs the
> reproducer on the file. Without the fix btrfs deadlocks, with the fix
> we pass fine.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> v1->v2:
> - Add the fiemap group to the test.
> - Actually include the reproducer helper program.
Looks like this patch is missed (except the author changed the patch subject:),
correct me if I'm wrong. The last review point hoped to add "src/fiemap-fault"
to .gitignore.
>
> src/Makefile | 2 +-
> src/fiemap-fault.c | 73 +++++++++++++++++++++++++++++++++++++++++++
> tests/generic/740 | 41 ++++++++++++++++++++++++
> tests/generic/740.out | 2 ++
> 4 files changed, 117 insertions(+), 1 deletion(-)
> create mode 100644 src/fiemap-fault.c
> create mode 100644 tests/generic/740
> create mode 100644 tests/generic/740.out
>
[snip]
> diff --git a/tests/generic/740 b/tests/generic/740
> new file mode 100644
> index 00000000..30ace1dd
> --- /dev/null
> +++ b/tests/generic/740
> @@ -0,0 +1,41 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved.
> +#
> +# FS QA Test 708
> +#
> +# Test fiemap into an mmaped buffer of the same file
> +#
> +# Create a reasonably large file, then run a program which mmaps it and uses
> +# that as a buffer for an fiemap call. This is a regression test for btrfs
> +# where we used to hold a lock for the duration of the fiemap call which would
> +# result in a deadlock if we page faulted.
> +#
> +. ./common/preamble
> +_begin_fstest quick auto fiemap
> +[ $FSTYP == "btrfs" ] && \
> + _fixed_by_kernel_commit xxxxxxxxxxxx \
This commit id is "b0ad381fa769" now.
> + "btrfs: fix deadlock with fiemap and extent locking"
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_test
> +_require_odirect
> +_require_test_program fiemap-fault
> +dst=$TEST_DIR/fiemap-fault-$seq
> +
> +echo "Silence is golden"
> +
> +for i in $(seq 0 2 1000)
> +do
> + $XFS_IO_PROG -d -f -c "pwrite -q $((i * 4096)) 4096" $dst
> +done
_require_sparse_files ?
> +
> +$here/src/fiemap-fault $dst > /dev/null || _fail "failed doing fiemap"
> +
> +rm -f $dst
If you'd like to remove this file, do it in _cleanup(), e.g.
_cleanup()
{
rm -f $dst
cd /
rm -r -f $tmp.*
}
Could you rebase this patch on latest fstests for-next branch with above
changes, then send a V2 please? I'd like to have this patch in next fstests
release.
Thanks,
Zorro
> +
> +# success, all done
> +status=$?
> +exit
> +
> diff --git a/tests/generic/740.out b/tests/generic/740.out
> new file mode 100644
> index 00000000..3f841e60
> --- /dev/null
> +++ b/tests/generic/740.out
> @@ -0,0 +1,2 @@
> +QA output created by 740
> +Silence is golden
> --
> 2.43.0
>
>
