On Tue, May 20, 2025 at 05:35:30PM +0530, Ritesh Harjani wrote:
> Catherine Hoang <catherine.hoang@xxxxxxxxxx> writes:
>
> > From: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> >
> > Simple tests of various atomic write requests and a (simulated) hardware
> > device.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
> > ---
> > common/atomicwrites | 10 +++
> > tests/generic/1222 | 86 +++++++++++++++++++++++++
> > tests/generic/1222.out | 10 +++
> > tests/generic/1223 | 66 +++++++++++++++++++
> > tests/generic/1223.out | 9 +++
> > tests/generic/1224 | 140 +++++++++++++++++++++++++++++++++++++++++
> > tests/generic/1224.out | 17 +++++
> > tests/xfs/1216 | 67 ++++++++++++++++++++
> > tests/xfs/1216.out | 9 +++
> > tests/xfs/1217 | 70 +++++++++++++++++++++
> > tests/xfs/1217.out | 3 +
> > tests/xfs/1218 | 59 +++++++++++++++++
> > tests/xfs/1218.out | 15 +++++
> > 13 files changed, 561 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 100644 tests/generic/1224
> > create mode 100644 tests/generic/1224.out
> > create mode 100755 tests/xfs/1216
> > create mode 100644 tests/xfs/1216.out
> > create mode 100755 tests/xfs/1217
> > create mode 100644 tests/xfs/1217.out
> > create mode 100644 tests/xfs/1218
> > create mode 100644 tests/xfs/1218.out
> >
> > diff --git a/common/atomicwrites b/common/atomicwrites
> > index 391bb6f6..c75c3d39 100644
> > --- a/common/atomicwrites
> > +++ b/common/atomicwrites
> > @@ -115,3 +115,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..9d02bd70
> > --- /dev/null
> > +++ b/tests/generic/1222
> > @@ -0,0 +1,86 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 1222
> > +#
> > +# Validate multi-fsblock atomic write support with simulated hardware support
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick rw atomicwrites
> > +
> > +. ./common/scsi_debug
> > +. ./common/atomicwrites
> > +
> > +_cleanup()
> > +{
> > + _scratch_unmount &>/dev/null
> > + _put_scsi_debug_dev &>/dev/null
> > + cd /
> > + rm -r -f $tmp.*
> > +}
> > +
> > +_require_scsi_debug
> > +_require_scratch_nocheck
> > +# Format something so that ./check doesn't freak out
> > +_scratch_mkfs >> $seqres.full
> > +
> > +# 512b logical/physical sectors, 512M size, atomic writes enabled
> > +dev=$(_get_scsi_debug_dev 512 512 0 512 "atomic_wr=1")
> > +test -b "$dev" || _notrun "could not create atomic writes scsi_debug device"
> > +
> > +export SCRATCH_DEV=$dev
> > +unset USE_EXTERNAL
> > +
> > +_require_scratch_write_atomic
> > +_require_atomic_write_test_commands
>
> Is it possible to allow pwrite -A to be tested on $SCRATCH_MNT rather
> than on TEST_MNT? For e.g.
>
> What happens when TEST_DEV is not atomic write capable? Then this test
> won't run even though we are passing scsi_debug which supports atomic writes.
Hrmmmm. Maybe we need an open-coded version of the "make sure the
xfs_io commands are present" checks without actually doing live testing
of the $TEST_DIR since we're creating a scsi-debug with atomic write
capability anyway.
> > +
> > +echo "scsi_debug 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
> > +
> > +# 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
> > +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
> > +
> > +_scratch_unmount
> > +_put_scsi_debug_dev
> > +
> > +# success, all done
> > +echo Silence is golden
> > +status=0
> > +exit
> > diff --git a/tests/generic/1222.out b/tests/generic/1222.out
> > new file mode 100644
> > index 00000000..158b52fa
> > --- /dev/null
> > +++ b/tests/generic/1222.out
> > @@ -0,0 +1,10 @@
> > +QA output created by 1222
> > +two EINVAL for unsupported sizes
> > +pwrite: Invalid argument
> > +pwrite: Invalid argument
> > +all should work
> > +one EOPNOTSUPP for buffered atomic
> > +pwrite: Operation not supported
> > +one EINVAL for unaligned directio
> > +pwrite: Invalid argument
> > +Silence is golden
> > 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
> > +
> > +# 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
> > +for ((i = min_awu; i <= max_awu; i *= 2)); do
>
> If the kernel we are testing on doesn't have SW XFS atomic write patches
> and if the scratch device does not support HW atomic write, then this
> could cause an infinite loop, right? Since both min_awu and max_awu can
> come out to be 0?
<nod> This should _notrun if max_awu is zero.
> > + $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"
> > +if [ $sector_size -lt $min_awu ]; then
> > + _simple_atomic_write $sector_size $min_awu $testfile -d
> > +else
> > + # not supported, so fake the output
> > + echo "pwrite: Invalid argument"
> > +fi
> > +
> > +# success, all done
> > +echo Silence is golden
> > +status=0
> > +exit
> > diff --git a/tests/generic/1223.out b/tests/generic/1223.out
> > new file mode 100644
> > index 00000000..edf5bd71
> > --- /dev/null
> > +++ b/tests/generic/1223.out
> > @@ -0,0 +1,9 @@
> > +QA output created by 1223
> > +two EINVAL for unsupported sizes
> > +pwrite: Invalid argument
> > +pwrite: Invalid argument
> > +one EOPNOTSUPP for buffered atomic
> > +pwrite: Operation not supported
> > +one EINVAL for unaligned directio
> > +pwrite: Invalid argument
> > +Silence is golden
>
>
> Will continue reviewing from g/1224 and will let you know if I
> have any comments.
Ok. Thanks for reviewing! :)
--D
