On 16/10/24 12:09, Filipe Manana wrote: > > > On Tue, Oct 15, 2024 at 4:42 PM Mark Harmstone <maharmstone@xxxxxx> wrote: >> >> Adds a test for a bug we encountered on Linux 6.4 on aarch64, where a >> race could mean that csums weren't getting written to the log tree, >> leading to corruption when it was replayed. >> >> The patches to detect log this tree corruption are in btrfs-progs 6.11. > > This shouldn't be needed right? > Because after log replay the csums are missing and 'btrfs check' > detects (IIRC) missing csums for extents referred by file extent items > in a subvolume tree - if it doesn't then it should be improved. Yes, but we're not mounting it in the tests between the log_writes calls, so the log isn't getting replayed. The patches to btrfs check make it so that it identifies filesystems that would get corrupted as soon as they're next mounted. > >> >> Signed-off-by: Mark Harmstone <maharmstone@xxxxxx> >> --- >> This is a genericized version of the test I originally proposed as >> btrfs/333. >> >> tests/generic/757 | 71 +++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/757.out | 2 ++ >> 2 files changed, 73 insertions(+) >> create mode 100755 tests/generic/757 >> create mode 100644 tests/generic/757.out >> >> diff --git a/tests/generic/757 b/tests/generic/757 >> new file mode 100755 >> index 00000000..6ad3d01e >> --- /dev/null >> +++ b/tests/generic/757 >> @@ -0,0 +1,71 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# >> +# FS QA Test 757 >> +# >> +# Test async dio with fsync to test a btrfs bug where a race meant that csums >> +# weren't getting written to the log tree, causing corruptions on remount. >> +# This can be seen on subpage FSes on Linux 6.4. >> +# >> +. ./common/preamble >> +_begin_fstest auto quick metadata log recoveryloop >> + >> +_fixed_by_kernel_commit e917ff56c8e7 \ >> + "btrfs: determine synchronous writers from bio or writeback control" > > For generic tests what we do is: > > [ $FSTYP == "btrfs" ] && _fixed_by_kernel_commit ..... > > As long as the failure has not been observed and fixed on other filesystems. > In case one day a regression happens in another fs, we just add a > corresponding line using the same logic. > > Otherwise if the test one days fails on another fs and fstests > suggests that that commit is missing, it would be odd. > > Everything else looks good, so with that fixed (maybe Zorro can change > that when picking the patch): > > Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx> > > Thanks. > Thanks Filipe. Zorro, let me know if you're happy making this change, or otherwise I'll resubmit. Mark
