| Message ID | 297da2b53ce9b697d82d89afd322b2cc0d0f392d.1716492850.git.osandov@osandov.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [fstests] btrfs: add regression test for fsync vs. size-extending direct I/O into prealloc crash | expand |
On Thu, May 23, 2024 at 9:43 PM Omar Sandoval <osandov@osandov.com> wrote: > > From: Omar Sandoval <osandov@fb.com> > > Since this is 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@fb.com> > --- > tests/btrfs/312 | 66 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/312.out | 2 ++ > 2 files changed, 68 insertions(+) > create mode 100755 tests/btrfs/312 > create mode 100644 tests/btrfs/312.out > > diff --git a/tests/btrfs/312 b/tests/btrfs/312 > new file mode 100755 > index 00000000..aaca0e3e > --- /dev/null > +++ b/tests/btrfs/312 > @@ -0,0 +1,66 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) Meta Platforms, Inc. and affiliates. > +# > +# FS QA Test 312 > +# > +# Repeatedly fsync after size-extending direct I/O into a preallocated extent. > +# > +. ./common/preamble > +_begin_fstest dangerous log prealloc Can also add in the "quick group". A since this is writing into a prealloc extent, the correct group is "preallocrw". > + > +_supported_fs btrfs > +_require_scratch > +_require_btrfs_command inspect-internal dump-tree > +_require_btrfs_command inspect-internal inode-resolve Missing a _require_attrs because $SETFATTR_PROG is needed/used. Also missing a: _require_xfs_io_command falloc -k > +_fixed_by_kernel_commit XXXXXXXXXXXX \ > + "btrfs: fix crash on racing fsync and size-extending direct I/O into prealloc" > + > +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" > +_scratch_mount > + > +sectorsize=$(_scratch_btrfs_sectorsize) > + > +# Create a bunch of files so that we hopefully get one whose items are at the > +# end of a leaf. > +for ((i = 0; i < 1000; i++)); do > + $XFS_IO_PROG -c "open -f -d $SCRATCH_MNT/$i" -c "falloc -k 0 $((sectorsize * 3))" -c "pwrite -q 0 $sectorsize" You can pass -d to $XFS_IO_PROG and make this a bit shorter: $XFS_IO_PROG -f -d -c "falloc -k 0 $((sectorsize * 3))" -c "pwrite -q 0 $sectorsize" "$SCRATCH_MNT/$i" > + $SETFATTR_PROG -n user.a -v a "$SCRATCH_MNT/$i" > +done > +touch "$SCRATCH_MNT/$i" Why is this touch needed here? I can trigger the bug without it - it's confusing, if it's really needed then add a comment explaining why please. So now this works with the default leaf size of 16K on x86. But what about in case we run with MKFS_OPTIONS="-n 64K", or on a PPC machine where default leaf/node size is 64K? The 1000 iterations aren't enough, so I would suggest making the test always create a fs with a 64K node size and adjusting the iterations to be enough to trigger the bug. > + > +_scratch_unmount > + > +ino=$($BTRFS_UTIL_PROG inspect-internal dump-tree "$SCRATCH_DEV" -t 5 | > + $AWK_PROG -v sectorsize="$sectorsize" ' > +match($0, /^leaf [0-9]+ items ([0-9]+)/, arr) { > + nritems = arr[1] > +} > +match($0, /item ([0-9]+) key \(([0-9]+) EXTENT_DATA ([0-9]+)\)/, arr) { > + if (arr[1] == nritems - 1 && arr[3] == sectorsize) { > + print arr[2] > + exit > + } > +} > +') > + > +if [ -z "$ino" ]; then > + _fail "Extent at end of leaf not found" > +fi > + > +_scratch_mount > +path=$($BTRFS_UTIL_PROG inspect-internal inode-resolve "$ino" "$SCRATCH_MNT") > + > +# Try repeatedly to reproduce the race of an ordered extent finishing while > +# we're logging prealloc extents beyond i_size. > +for ((i = 0; i < 1000; i++)); do > + $XFS_IO_PROG -c "open -t -d $path" -c "falloc -k 0 $((sectorsize * 3))" -c "pwrite -q -w 0 $sectorsize" > + $SETFATTR_PROG -n user.a -v a "$path" > + $XFS_IO_PROG -c "open -d $path" -c "pwrite -q -w $sectorsize $sectorsize" || exit 1 the || exit 1 is odd here. Normally we do || _fail "some reason" or just don't do anything at all, as a golden output mismatch due to an error will make the test fail. Also don't forget to cc the fstests mailing list. Thanks. > +done > + > +# If it didn't crash, we're good. > + > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/btrfs/312.out b/tests/btrfs/312.out > new file mode 100644 > index 00000000..6e72aa94 > --- /dev/null > +++ b/tests/btrfs/312.out > @@ -0,0 +1,2 @@ > +QA output created by 312 > +Silence is golden > -- > 2.45.1 > >
diff --git a/tests/btrfs/312 b/tests/btrfs/312 new file mode 100755 index 00000000..aaca0e3e --- /dev/null +++ b/tests/btrfs/312 @@ -0,0 +1,66 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# FS QA Test 312 +# +# Repeatedly fsync after size-extending direct I/O into a preallocated extent. +# +. ./common/preamble +_begin_fstest dangerous log prealloc + +_supported_fs btrfs +_require_scratch +_require_btrfs_command inspect-internal dump-tree +_require_btrfs_command inspect-internal inode-resolve +_fixed_by_kernel_commit XXXXXXXXXXXX \ + "btrfs: fix crash on racing fsync and size-extending direct I/O into prealloc" + +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" +_scratch_mount + +sectorsize=$(_scratch_btrfs_sectorsize) + +# Create a bunch of files so that we hopefully get one whose items are at the +# end of a leaf. +for ((i = 0; i < 1000; i++)); do + $XFS_IO_PROG -c "open -f -d $SCRATCH_MNT/$i" -c "falloc -k 0 $((sectorsize * 3))" -c "pwrite -q 0 $sectorsize" + $SETFATTR_PROG -n user.a -v a "$SCRATCH_MNT/$i" +done +touch "$SCRATCH_MNT/$i" + +_scratch_unmount + +ino=$($BTRFS_UTIL_PROG inspect-internal dump-tree "$SCRATCH_DEV" -t 5 | + $AWK_PROG -v sectorsize="$sectorsize" ' +match($0, /^leaf [0-9]+ items ([0-9]+)/, arr) { + nritems = arr[1] +} +match($0, /item ([0-9]+) key \(([0-9]+) EXTENT_DATA ([0-9]+)\)/, arr) { + if (arr[1] == nritems - 1 && arr[3] == sectorsize) { + print arr[2] + exit + } +} +') + +if [ -z "$ino" ]; then + _fail "Extent at end of leaf not found" +fi + +_scratch_mount +path=$($BTRFS_UTIL_PROG inspect-internal inode-resolve "$ino" "$SCRATCH_MNT") + +# Try repeatedly to reproduce the race of an ordered extent finishing while +# we're logging prealloc extents beyond i_size. +for ((i = 0; i < 1000; i++)); do + $XFS_IO_PROG -c "open -t -d $path" -c "falloc -k 0 $((sectorsize * 3))" -c "pwrite -q -w 0 $sectorsize" + $SETFATTR_PROG -n user.a -v a "$path" + $XFS_IO_PROG -c "open -d $path" -c "pwrite -q -w $sectorsize $sectorsize" || exit 1 +done + +# If it didn't crash, we're good. + +echo "Silence is golden" +status=0 +exit diff --git a/tests/btrfs/312.out b/tests/btrfs/312.out new file mode 100644 index 00000000..6e72aa94 --- /dev/null +++ b/tests/btrfs/312.out @@ -0,0 +1,2 @@ +QA output created by 312 +Silence is golden