> On Dec 19, 2024, at 2:48 AM, John Garry <john.g.garry@xxxxxxxxxx> wrote:
>
> On 17/12/2024 02:08, Catherine Hoang wrote:
>> Add a test to validate the new atomic writes feature.
>
> Generally this look ok, just a couple of comments/questions, below.
>
> Thanks,
> John
>
>> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
>> ---
>> common/rc | 14 ++++++++
>> tests/xfs/611 | 81 +++++++++++++++++++++++++++++++++++++++++++++++
>> tests/xfs/611.out | 2 ++
>> 3 files changed, 97 insertions(+)
>> create mode 100755 tests/xfs/611
>> create mode 100644 tests/xfs/611.out
>> diff --git a/common/rc b/common/rc
>> index 2ee46e51..b9da749e 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -5148,6 +5148,20 @@ _require_scratch_btime()
>> _scratch_unmount
>> }
>> +_require_scratch_write_atomic()
>> +{
>> + _require_scratch
>> + _scratch_mkfs > /dev/null 2>&1
>> + _scratch_mount
>> +
>> + export STATX_WRITE_ATOMIC=0x10000
>> + $XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_MNT \
>> + | grep atomic >>$seqres.full 2>&1 || \
>> + _notrun "write atomic not supported by this filesystem"
>> +
>> + _scratch_unmount
>> +}
>> +
>> _require_inode_limits()
>> {
>> if [ $(_get_free_inode $TEST_DIR) -eq 0 ]; then
>> diff --git a/tests/xfs/611 b/tests/xfs/611
>> new file mode 100755
>> index 00000000..a26ec143
>> --- /dev/null
>> +++ b/tests/xfs/611
>> @@ -0,0 +1,81 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Oracle. All Rights Reserved.
>> +#
>> +# FS QA Test 611
>> +#
>> +# Validate atomic write support
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick rw
>> +
>> +_supported_fs xfs
>> +_require_scratch
>> +_require_scratch_write_atomic
>> +
>> +test_atomic_writes()
>> +{
>> + local bsize=$1
>> +
>> + _scratch_mkfs_xfs -b size=$bsize >> $seqres.full
>
> bsize (bdev max) can be upto 0.5M - are we really possibly testing FS blocksize == 0.5M?
>
> Apart from that, it would be nice if we fixed FS blocksize at 4K or 64K, and fed bdev min/max and ensured that we can only support atomic writes for bdev min <= fs blocksize <= bdev max. But maybe what you are doing is ok.
Currently I’m testing every valid block size between the bdev min and bdev
max. But I can also add a test to make sure we aren’t doing atomic writes if
block size < bdev min or block size > bdev max.
>
>> + _scratch_mount
>> + _xfs_force_bdev data $SCRATCH_MNT
>> +
>> + testfile=$SCRATCH_MNT/testfile
>> + touch $testfile
>> +
>> + file_min_write=$($XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile | \
>> + grep atomic_write_unit_min | cut -d ' ' -f 3)
>> + file_max_write=$($XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile | \
>> + grep atomic_write_unit_max | cut -d ' ' -f 3)
>> + file_max_segments=$($XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile | \
>> + grep atomic_write_segments_max | cut -d ' ' -f 3)
>> +
>> + # Check that atomic min/max = FS block size
>
> Hopefully we can have max > FS block size soon, but I am not sure how .... so it's hard to consider now how the test could be expanded to cover that.
>
>> + test $file_min_write -eq $bsize || \
>> + echo "atomic write min $file_min_write, should be fs block size $bsize"
>> + test $file_min_write -eq $bsize || \
>> + echo "atomic write max $file_max_write, should be fs block size $bsize"
>> + test $file_max_segments -eq 1 || \
>> + echo "atomic write max segments $file_max_segments, should be 1"
>> +
>> + # Check that we can perform an atomic write of len = FS block size
>> + bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D 0 $bsize" $testfile | \
>> + grep wrote | awk -F'[/ ]' '{print $2}')
>> + test $bytes_written -eq $bsize || echo "atomic write len=$bsize failed"
>> +
>> + # Check that we can perform an atomic write on an unwritten block
>> + $XFS_IO_PROG -c "falloc $bsize $bsize" $testfile
>> + bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D $bsize $bsize" $testfile | \
>> + grep wrote | awk -F'[/ ]' '{print $2}')
>> + test $bytes_written -eq $bsize || echo "atomic write to unwritten block failed"
>> +
>> + # Check that we can perform an atomic write on a sparse hole
>> + $XFS_IO_PROG -c "fpunch 0 $bsize" $testfile
>> + bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D 0 $bsize" $testfile | \
>> + grep wrote | awk -F'[/ ]' '{print $2}')
>> + test $bytes_written -eq $bsize || echo "atomic write to sparse hole failed"
>> +
>> + # Reject atomic write if len is out of bounds
>> + $XFS_IO_PROG -dc "pwrite -A -D 0 $((bsize - 1))" $testfile 2>> $seqres.full && \
>> + echo "atomic write len=$((bsize - 1)) should fail"
>> + $XFS_IO_PROG -dc "pwrite -A -D 0 $((bsize + 1))" $testfile 2>> $seqres.full && \
>> + echo "atomic write len=$((bsize + 1)) should fail"
>> +
>> + _scratch_unmount
>> +}
>> +
>> +bdev_min_write=$($XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_DEV | \
>> + grep atomic_write_unit_min | cut -d ' ' -f 3)
>> +bdev_max_write=$($XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_DEV | \
>> + grep atomic_write_unit_max | cut -d ' ' -f 3)
>> +
>> +for ((bsize=$bdev_min_write; bsize<=bdev_max_write; bsize*=2)); do
>> + _scratch_mkfs_xfs_supported -b size=$bsize >> $seqres.full 2>&1 && \
>> + test_atomic_writes $bsize
>> +done;
>> +
>> +# success, all done
>> +echo Silence is golden
>> +status=0
>> +exit
>> diff --git a/tests/xfs/611.out b/tests/xfs/611.out
>> new file mode 100644
>> index 00000000..b8a44164
>> --- /dev/null
>> +++ b/tests/xfs/611.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 611
>> +Silence is golden
