Re: [PATCH fstests v3] generic: test Btrfs fsync vs. size-extending prealloc write crash

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Tue, May 28, 2024 at 10:13:14AM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@xxxxxx>
> 
> This is a regression test for a Btrfs bug, but there's nothing
> Btrfs-specific about it. Since it's a race, we just try to make the race
> happen in a loop and pass if it doesn't crash after all of our attempts.
> 
> Signed-off-by: Omar Sandoval <osandov@xxxxxx>
> ---
> Changes from v2 [1]:
> 
> - Rebased on for-next.
> - Added _require_odirect.
> - Added FSTYP check to _fixed_by_kernel_commit.
> - Added Filipe's Reviewed-by.
> 
> Changes from v1 [2]:
> 
> - Added missing groups and requires.
> - Simplified $XFS_IO_PROG calls.
> - Removed -i flag from $XFS_IO_PROG to make race reproduce more
>   reliably.
> - Removed all of the file creation and dump-tree parsing since the only
>   file on a fresh filesystem is guaranteed to be at the end of a leaf
>   anyways.
> - Rewrote to be a generic test.
> 
> 1: https://lore.kernel.org/linux-btrfs/d032e0b964f163229b684c0ac72b656ec9bf7b48.1716584019.git.osandov@xxxxxxxxxxx/
> 2: https://lore.kernel.org/linux-btrfs/297da2b53ce9b697d82d89afd322b2cc0d0f392d.1716492850.git.osandov@xxxxxxxxxxx/
> 
>  tests/generic/748     | 45 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/748.out |  2 ++
>  2 files changed, 47 insertions(+)
>  create mode 100755 tests/generic/748
>  create mode 100644 tests/generic/748.out
> 
> diff --git a/tests/generic/748 b/tests/generic/748
> new file mode 100755
> index 00000000..2ec33694
> --- /dev/null
> +++ b/tests/generic/748
> @@ -0,0 +1,45 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) Meta Platforms, Inc. and affiliates.
> +#
> +# FS QA Test 748
> +#
> +# Repeatedly prealloc beyond i_size, set an xattr, direct write into the
> +# prealloc while extending i_size, then fdatasync. This is a regression test
> +# for a Btrfs crash.
> +#
> +. ./common/preamble
> +. ./common/attr
> +_begin_fstest auto quick log preallocrw dangerous
> +
> +_supported_fs generic
> +_require_scratch
> +_require_attrs
> +_require_odirect
> +_require_xfs_io_command falloc -k
> +[ "$FSTYP" = btrfs ] && _fixed_by_kernel_commit XXXXXXXXXXXX \
> +	"btrfs: fix crash on racing fsync and size-extending write into prealloc"
> +
> +# -i slows down xfs_io startup and makes the race much less reliable.
> +export XFS_IO_PROG="$(echo "$XFS_IO_PROG" | sed 's/ -i\b//')"

I think the "export" is not necessary, although it doesn't affect much :) I'll
remove it when I merge it. Is that good to you too?

> +
> +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mount
> +
> +blksz=$(_get_block_size "$SCRATCH_MNT")

Maybe you'd like to use _get_file_block_size, which is more accurate
to get the minimum block size of a file as a generic test case.

I can help to do this change if you agree that too.

Thanks,
Zorro

> +
> +# On Btrfs, since this is the only file on the filesystem, its metadata is at
> +# the end of a B-tree leaf. We want an ordered extent completion to add an
> +# extent item at the end of the leaf while we're logging prealloc extents
> +# beyond i_size after an xattr was set.
> +for ((i = 0; i < 5000; i++)); do
> +	$XFS_IO_PROG -ftd -c "falloc -k 0 $((blksz * 3))" -c "pwrite -q -w 0 $blksz" "$SCRATCH_MNT/file"
> +	$SETFATTR_PROG -n user.a -v a "$SCRATCH_MNT/file"
> +	$XFS_IO_PROG -d -c "pwrite -q -w $blksz $blksz" "$SCRATCH_MNT/file"
> +done
> +
> +# If it didn't crash, we're good.
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/748.out b/tests/generic/748.out
> new file mode 100644
> index 00000000..dc050a96
> --- /dev/null
> +++ b/tests/generic/748.out
> @@ -0,0 +1,2 @@
> +QA output created by 748
> +Silence is golden
> -- 
> 2.45.1
> 
> 





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux