On Fri, Oct 18, 2024 at 05:36:26PM +0000, Mark Harmstone wrote: > 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. I can help to change that when I merge it. Thanks you and Filipe. Thanks, Zorro > > Mark > >
