On Wed, Jun 11, 2025 at 06:57:07PM -0700, Catherine Hoang wrote:
> Simple tests of various atomic write requests and a (simulated) hardware
> device.
>
> The first test performs basic multi-block atomic writes on a scsi_debug device
> with atomic writes enabled. We test all advertised sizes between the atomic
> write unit min and max. We also ensure that the write fails when expected, such
> as when attempting buffered io or unaligned directio.
>
> The second test is similar to the one above, except that it verifies multi-block
> atomic writes on actual hardware instead of simulated hardware. The device used
> in this test is not required to support atomic writes.
>
> The final two tests ensure multi-block atomic writes can be performed on various
> interweaved mappings, including written, mapped, hole, and unwritten. We also
> test large atomic writes on a heavily fragmented filesystem. These tests are
> separated into reflink (shared) and non-reflink tests.
>
> Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
Hi Catherine, Darrick,
The tests looks mostly okay. Just a few minor comments I've added below:
> ---
> common/atomicwrites | 10 ++++
> tests/generic/1222 | 89 ++++++++++++++++++++++++++++
> tests/generic/1222.out | 10 ++++
> tests/generic/1223 | 67 +++++++++++++++++++++
> tests/generic/1223.out | 9 +++
> tests/generic/1224 | 86 +++++++++++++++++++++++++++
> tests/generic/1224.out | 16 ++++++
> tests/generic/1225 | 128 +++++++++++++++++++++++++++++++++++++++++
> tests/generic/1225.out | 21 +++++++
> 9 files changed, 436 insertions(+)
> create mode 100755 tests/generic/1222
> create mode 100644 tests/generic/1222.out
> create mode 100755 tests/generic/1223
> create mode 100644 tests/generic/1223.out
> create mode 100755 tests/generic/1224
> create mode 100644 tests/generic/1224.out
> create mode 100755 tests/generic/1225
> create mode 100644 tests/generic/1225.out
>
> diff --git a/common/atomicwrites b/common/atomicwrites
> index 88f49a1a..4ba945ec 100644
> --- a/common/atomicwrites
> +++ b/common/atomicwrites
> @@ -136,3 +136,13 @@ _test_atomic_file_writes()
> $XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 1 $bsize" $testfile 2>> $seqres.full && \
> echo "atomic write requires offset to be aligned to bsize"
> }
> +
> +_simple_atomic_write() {
> + local pos=$1
> + local count=$2
> + local file=$3
> + local directio=$4
> +
> + echo "testing pos=$pos count=$count file=$file directio=$directio" >> $seqres.full
> + $XFS_IO_PROG $directio -c "pwrite -b $count -V 1 -A -D $pos $count" $file >> $seqres.full
> +}
> diff --git a/tests/generic/1222 b/tests/generic/1222
> new file mode 100755
> index 00000000..d3665d0b
> --- /dev/null
> +++ b/tests/generic/1222
> @@ -0,0 +1,89 @@
<snip>
> +
> +$XFS_IO_PROG -f -c "falloc 0 $((max_awu * 2))" -c fsync $testfile
> +
> +# try outside the advertised sizes
> +echo "two EINVAL for unsupported sizes"
> +min_i=$((min_awu / 2))
> +_simple_atomic_write $min_i $min_i $testfile -d
> +max_i=$((max_awu * 2))
> +_simple_atomic_write $max_i $max_i $testfile -d
> +
> +# try all of the advertised sizes
> +echo "all should work"
> +for ((i = min_awu; i <= max_awu; i *= 2)); do
> + $XFS_IO_PROG -f -c "falloc 0 $((max_awu * 2))" -c fsync $testfile
> + _test_atomic_file_writes $i $testfile
> + _simple_atomic_write $i $i $testfile -d
I'm still not sure what extra thing this _simple_atomic_write is testing
here that is not already tested via _test_atomic_file_writes? (same
question for g/1223 as well)
> +done
> +
> +# does not support buffered io
> +echo "one EOPNOTSUPP for buffered atomic"
> +_simple_atomic_write 0 $min_awu $testfile
> +
> +# does not support unaligned directio
> +echo "one EINVAL for unaligned directio"
> +_simple_atomic_write $sector_size $min_awu $testfile -d
> +
<snip>
> new file mode 100755
> index 00000000..f2dea804
> --- /dev/null
> +++ b/tests/generic/1225
> @@ -0,0 +1,128 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 1225
> +#
> +# basic tests for large atomic writes with mixed mappings
> +#
> +. ./common/preamble
> +_begin_fstest auto quick rw atomicwrites
> +
> +. ./common/atomicwrites
> +. ./common/filter
> +. ./common/reflink
I think we are not using reflink based helpers here so this can be
dropped.
<snip>
> +
> +echo "atomic write unwritten+mapped+unwritten"
> +dd if=/dev/zero of=$file1 bs=1M count=10 conv=fsync >>$seqres.full 2>&1
> +$XFS_IO_PROG -fc "falloc 0 4096000" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -D -V1 4096 4096" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -A -D -V1 0 65536" $file1 >>$seqres.full 2>&1
> +md5sum $file1 | _filter_scratch
> +
> +echo "atomic write adjacent mapped+unwritten and unwritten+mapped"
> +dd if=/dev/zero of=$file1 bs=1M count=10 conv=fsync >>$seqres.full 2>&1
> +$XFS_IO_PROG -fc "falloc 0 4096000" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -D -V1 0 4096" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -D -V1 61440 4096" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -A -D -V1 0 32768" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -A -D -V1 32768 32768" $file1 >>$seqres.full 2>&1
> +md5sum $file1 | _filter_scratch
> +
> +echo "atomic write mapped+unwritten+mapped"
> +dd if=/dev/zero of=$file1 bs=1M count=10 conv=fsync >>$seqres.full 2>&1
> +$XFS_IO_PROG -fc "falloc 0 4096000" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -D -V1 0 4096" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -D -V1 61440 4096" $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -A -D -V1 0 65536" $file1 >>$seqres.full 2>&1
> +md5sum $file1 | _filter_scratch
> +
> +echo "atomic write interweaved hole+unwritten+written"
> +dd if=/dev/zero of=$file1 bs=1M count=10 conv=fsync >>$seqres.full 2>&1
> +blksz=4096
> +nr=32
> +_weave_file_rainbow $blksz $nr $file1 >>$seqres.full 2>&1
> +$XFS_IO_PROG -dc "pwrite -A -D -V1 0 65536" $file1 >>$seqres.full 2>&1
> +md5sum $file1 | _filter_scratch
Thanks for adding more unwritten based combinations :)
Rest everything looks okay to me.
Regards,
ojaswin
<snip>
