On Wed, May 14, 2025 at 02:41:40PM +0100, John Garry wrote:
>
> > +++ b/tests/generic/1222
> > @@ -0,0 +1,86 @@
<snip>
> > +# 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
> > +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
>
> I figure that $sector_size is default at 512, which would never be equal to
> fsblocksize (so the test looks ok)
For now, yes -- the only filesystems supporting atomic writes (ext4 and
XFS v5) don't support 512b fsblocks.
<snip>
> > diff --git a/tests/generic/1223 b/tests/generic/1223
> > new file mode 100755
> > index 00000000..8a77386e
> > --- /dev/null
> > +++ b/tests/generic/1223
> > @@ -0,0 +1,66 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 1223
> > +#
> > +# Validate multi-fsblock atomic write support with or without hw support
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick rw atomicwrites
> > +
> > +. ./common/atomicwrites
> > +
> > +_require_scratch
> > +_require_atomic_write_test_commands
> > +
> > +echo "scratch device atomic write properties" >> $seqres.full
> > +$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_DEV >> $seqres.full
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
> > +
> > +testfile=$SCRATCH_MNT/testfile
> > +touch $testfile
> > +
> > +echo "filesystem atomic write properties" >> $seqres.full
> > +$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile >> $seqres.full
> > +
> > +sector_size=$(blockdev --getss $SCRATCH_DEV)
> > +min_awu=$(_get_atomic_write_unit_min $testfile)
> > +max_awu=$(_get_atomic_write_unit_max $testfile)
> > +
> > +$XFS_IO_PROG -f -c "falloc 0 $((max_awu * 2))" -c fsync $testfile
> > +
>
> It seems many sub-tests are same as 1222
>
> It is difficult to factor them out?
Yes. g/1222 will _notrun itself if the scsi_debug module isn't present
or the fake device cannot be created. Apparently many of the people who
run fstests also have test infrastructure that cannot handle modules, so
they don't run anything involving scsi_debug.
That's why g/1223 only requires that the scratch fs advertises some sort
of atomic write capability, it doesn't care how it provides that.
<snip>
> > diff --git a/tests/generic/1224 b/tests/generic/1224
> > new file mode 100644
> > index 00000000..fb178be4
> > --- /dev/null
> > +++ b/tests/generic/1224
<snip>
> > +# atomic write max size
> > +dd if=/dev/zero of=$file1 bs=1M count=10 conv=fsync >>$seqres.full 2>&1
> > +aw_max=$(_get_atomic_write_unit_max $file1)
> > +cp $file1 $file1.chk
> > +$XFS_IO_PROG -dc "pwrite -D -V1 0 $aw_max" $file1 >>$seqres.full 2>&1
> > +$XFS_IO_PROG -c "pwrite 0 $aw_max" $file1.chk >>$seqres.full 2>&1
> > +cmp -s $file1 $file1.chk || echo "file1 doesnt match file1.chk"
> > +#md5sum $file1 | _filter_scratch
> > +
> > +# atomic write max size on fragmented fs
> > +avail=`_get_available_space $SCRATCH_MNT`
> > +filesizemb=$((avail / 1024 / 1024 - 1))
> > +fragmentedfile=$SCRATCH_MNT/fragmentedfile
> > +$XFS_IO_PROG -fc "falloc 0 ${filesizemb}m" $fragmentedfile
> > +$here/src/punch-alternating $fragmentedfile
> > +touch $file3
> > +$XFS_IO_PROG -dc "pwrite -A -D -V1 0 65536" $file3 >>$seqres.full 2>&1
> > +md5sum $file3 | _filter_scratch
>
> nice :)
>
> But we also test RWF_NOWAIT at some stage?
>
> RWF_NOWAIT should fail always for filesystem-based atomic write
It's hard to test NOWAIT because the selected io path might not actually
encounter contention, and there are various things that NOWAIT will wait
on anyway (like memory allocation and metadata reads).
<snip>
> > diff --git a/tests/generic/1225 b/tests/generic/1225
> > new file mode 100644
> > index 00000000..600ada56
> > --- /dev/null
> > +++ b/tests/generic/1225
>
> I think that we should now omit this test. We don't guarantee serialization
> of atomic writes, so no point in testing it.
>
> I should have confirmed this earlier, sorry
Ok.
<snip>
> > diff --git a/tests/xfs/1216 b/tests/xfs/1216
> > new file mode 100755
> > index 00000000..d9a10ed9
> > --- /dev/null
> > +++ b/tests/xfs/1216
> > @@ -0,0 +1,68 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 1216
> > +#
> > +# Validate multi-fsblock realtime file atomic write support with or without hw
> > +# support
>
> nice to see rtvol being tested.
Thanks. :)
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick rw atomicwrites
> > +
> > +. ./common/atomicwrites
> > +
> > +_require_realtime
> > +_require_scratch
> > +_require_atomic_write_test_commands
> > +
> > +echo "scratch device atomic write properties" >> $seqres.full
> > +$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_RTDEV >> $seqres.full
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +test "$FSTYP" = "xfs" && _xfs_force_bdev realtime $SCRATCH_MNT
Don't need this FSTYP test here, FSTYP is always xfs.
<snip>
> > diff --git a/tests/xfs/1217 b/tests/xfs/1217
> > new file mode 100755
> > index 00000000..012a1f46
> > --- /dev/null
> > +++ b/tests/xfs/1217
> > @@ -0,0 +1,70 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 1217
> > +#
> > +# Check that software atomic writes can complete an operation after a crash.
> > +#
>
> Could we prove that we get a torn write for a regular non-atomic write also?
Perhaps? But I don't see the point -- non-atomic write completions
could be done atomically.
--D
> > +. ./common/preamble
> > +_begin_fstest auto quick rw atomicwrites
> > +
>
> Thanks,
> John
>
