Re: [PATCH v3] xfs: add a test for atomic writes

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 2/28/25 21:13, Darrick J. Wong wrote:
> On Fri, Feb 28, 2025 at 07:01:50PM +0530, Nirjhar Roy (IBM) wrote:
>> On Thu, 2025-02-27 at 18:11 -0800, Darrick J. Wong wrote:
>>> On Thu, Feb 27, 2025 at 04:20:59PM -0800, Catherine Hoang wrote:
>>>> Add a test to validate the new atomic writes feature.
>>>>
>>>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>>>> Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>> Er.... what git tree is this based upon?  generic/762 is a project
>>> quota
>>> test.
>> On which branch do you have tests/generic/762? I checked the latest
>> master(commit - 8467552f09e1672a02712653b532a84bd46ea10e) and the for-
>> next(commit - 5b56a2d888191bfc7131b096e611eab1881d8422) and it doesn't
>> seem to exist there. However, tests/xfs/762 does exist.
> Zorro's patches-in-queue, aka whatever gets pushed to for-next on
> Sunday.  My confusion stems from this patch modifying what looks like an
> existing atomic writes test, but generic/762 isn't that test so now I
> can't see everything that this test is examining.
>
> (I suggest everyone please post urls to public git repos so reviewers
> can get around these sorts of issues in the future.)

Noted. Thank you.

--NR

>
> --D
>
>> --NR
>>> --D
>>>
>>>> ---
>>>>   common/rc             |  51 ++++++++++++++
>>>>   tests/generic/762     | 160
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>   tests/generic/762.out |   2 +
>>>>   3 files changed, 213 insertions(+)
>>>>   create mode 100755 tests/generic/762
>>>>   create mode 100644 tests/generic/762.out
>>>>
>>>> diff --git a/common/rc b/common/rc
>>>> index 6592c835..08a9d9b8 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -2837,6 +2837,10 @@ _require_xfs_io_command()
>>>>   			opts+=" -d"
>>>>   			pwrite_opts+="-V 1 -b 4k"
>>>>   		fi
>>>> +		if [ "$param" == "-A" ]; then
>>>> +			opts+=" -d"
>>>> +			pwrite_opts+="-D -V 1 -b 4k"
>>>> +		fi
>>>>   		testio=`$XFS_IO_PROG -f $opts -c \
>>>>   		        "pwrite $pwrite_opts $param 0 4k" $testfile
>>>> 2>&1`
>>>>   		param_checked="$pwrite_opts $param"
>>>> @@ -5175,6 +5179,53 @@ _require_scratch_btime()
>>>>   	_scratch_unmount
>>>>   }
>>>>   
>>>> +_get_atomic_write_unit_min()
>>>> +{
>>>> +	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
>>>> +        grep atomic_write_unit_min | grep -o '[0-9]\+'
>>>> +}
>>>> +
>>>> +_get_atomic_write_unit_max()
>>>> +{
>>>> +	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
>>>> +        grep atomic_write_unit_max | grep -o '[0-9]\+'
>>>> +}
>>>> +
>>>> +_get_atomic_write_segments_max()
>>>> +{
>>>> +	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
>>>> +        grep atomic_write_segments_max | grep -o '[0-9]\+'
>>>> +}
>>>> +
>>>> +_require_scratch_write_atomic()
>>>> +{
>>>> +	_require_scratch
>>>> +
>>>> +	export STATX_WRITE_ATOMIC=0x10000
>>>> +
>>>> +	awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>>>> +	awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
>>>> +
>>>> +	if [ $awu_min_bdev -eq 0 ] && [ $awu_max_bdev -eq 0 ]; then
>>>> +		_notrun "write atomic not supported by this block
>>>> device"
>>>> +	fi
>>>> +
>>>> +	_scratch_mkfs > /dev/null 2>&1
>>>> +	_scratch_mount
>>>> +
>>>> +	testfile=$SCRATCH_MNT/testfile
>>>> +	touch $testfile
>>>> +
>>>> +	awu_min_fs=$(_get_atomic_write_unit_min $testfile)
>>>> +	awu_max_fs=$(_get_atomic_write_unit_max $testfile)
>>>> +
>>>> +	_scratch_unmount
>>>> +
>>>> +	if [ $awu_min_fs -eq 0 ] && [ $awu_max_fs -eq 0 ]; then
>>>> +		_notrun "write atomic not supported by this filesystem"
>>>> +	fi
>>>> +}
>>>> +
>>>>   _require_inode_limits()
>>>>   {
>>>>   	if [ $(_get_free_inode $TEST_DIR) -eq 0 ]; then
>>>> diff --git a/tests/generic/762 b/tests/generic/762
>>>> new file mode 100755
>>>> index 00000000..d0a80219
>>>> --- /dev/null
>>>> +++ b/tests/generic/762
>>>> @@ -0,0 +1,160 @@
>>>> +#! /bin/bash
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +# Copyright (c) 2025 Oracle.  All Rights Reserved.
>>>> +#
>>>> +# FS QA Test 762
>>>> +#
>>>> +# Validate atomic write support
>>>> +#
>>>> +. ./common/preamble
>>>> +_begin_fstest auto quick rw
>>>> +
>>>> +_require_scratch_write_atomic
>>>> +_require_xfs_io_command pwrite -A
>>>> +
>>>> +test_atomic_writes()
>>>> +{
>>>> +    local bsize=$1
>>>> +
>>>> +    case "$FSTYP" in
>>>> +    "xfs")
>>>> +        mkfs_opts="-b size=$bsize"
>>>> +        ;;
>>>> +    "ext4")
>>>> +        mkfs_opts="-b $bsize"
>>>> +        ;;
>>>> +    *)
>>>> +        ;;
>>>> +    esac
>>>> +
>>>> +    # If block size is not supported, skip this test
>>>> +    _scratch_mkfs $mkfs_opts >>$seqres.full 2>&1 || return
>>>> +    _try_scratch_mount >>$seqres.full 2>&1 || return
>>>> +
>>>> +    test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
>>>> +
>>>> +    testfile=$SCRATCH_MNT/testfile
>>>> +    touch $testfile
>>>> +
>>>> +    file_min_write=$(_get_atomic_write_unit_min $testfile)
>>>> +    file_max_write=$(_get_atomic_write_unit_max $testfile)
>>>> +    file_max_segments=$(_get_atomic_write_segments_max $testfile)
>>>> +
>>>> +    # Check that atomic min/max = FS block size
>>>> +    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 -V1 -b $bsize 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 single-block cow write
>>>> +    if [ "$FSTYP" == "xfs" ]; then
>>>> +        testfile_cp=$SCRATCH_MNT/testfile_copy
>>>> +        if _xfs_has_feature $SCRATCH_MNT reflink; then
>>>> +            cp --reflink $testfile $testfile_cp
>>>> +        fi
>>>> +        bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D -V1 -b
>>>> $bsize 0 $bsize" $testfile_cp | \
>>>> +            grep wrote | awk -F'[/ ]' '{print $2}')
>>>> +        test $bytes_written -eq $bsize || echo "atomic write on
>>>> reflinked file failed"
>>>> +    fi
>>>> +
>>>> +    # 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 -V1 -b $bsize
>>>> $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 -V1 -b $bsize 0
>>>> $bsize" $testfile | \
>>>> +        grep wrote | awk -F'[/ ]' '{print $2}')
>>>> +    test $bytes_written -eq $bsize || echo "atomic write to sparse
>>>> hole failed"
>>>> +
>>>> +    # Check that we can perform an atomic write on a fully mapped
>>>> block
>>>> +    bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 0
>>>> $bsize" $testfile | \
>>>> +        grep wrote | awk -F'[/ ]' '{print $2}')
>>>> +    test $bytes_written -eq $bsize || echo "atomic write to mapped
>>>> block failed"
>>>> +
>>>> +    # Reject atomic write if len is out of bounds
>>>> +    $XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 0 $((bsize - 1))"
>>>> $testfile 2>> $seqres.full && \
>>>> +        echo "atomic write len=$((bsize - 1)) should fail"
>>>> +    $XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 0 $((bsize + 1))"
>>>> $testfile 2>> $seqres.full && \
>>>> +        echo "atomic write len=$((bsize + 1)) should fail"
>>>> +
>>>> +    # Reject atomic write when iovecs > 1
>>>> +    $XFS_IO_PROG -dc "pwrite -A -D -V2 -b $bsize 0 $bsize"
>>>> $testfile 2>> $seqres.full && \
>>>> +        echo "atomic write only supports iovec count of 1"
>>>> +
>>>> +    # Reject atomic write when not using direct I/O
>>>> +    $XFS_IO_PROG -c "pwrite -A -V1 -b $bsize 0 $bsize" $testfile
>>>> 2>> $seqres.full && \
>>>> +        echo "atomic write requires direct I/O"
>>>> +
>>>> +    # Reject atomic write when offset % bsize != 0
>>>> +    $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"
>>>> +
>>>> +    _scratch_unmount
>>>> +}
>>>> +
>>>> +test_atomic_write_bounds()
>>>> +{
>>>> +    local bsize=$1
>>>> +
>>>> +    case "$FSTYP" in
>>>> +    "xfs")
>>>> +        mkfs_opts="-b size=$bsize"
>>>> +        ;;
>>>> +    "ext4")
>>>> +        mkfs_opts="-b $bsize"
>>>> +        ;;
>>>> +    *)
>>>> +        ;;
>>>> +    esac
>>>> +
>>>> +    # If block size is not supported, skip this test
>>>> +    _scratch_mkfs $mkfs_opts >>$seqres.full 2>&1 || return
>>>> +    _try_scratch_mount >>$seqres.full 2>&1 || return
>>>> +
>>>> +    test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
>>>> +
>>>> +    testfile=$SCRATCH_MNT/testfile
>>>> +    touch $testfile
>>>> +
>>>> +    $XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 0 $bsize"
>>>> $testfile 2>> $seqres.full && \
>>>> +        echo "atomic write should fail when bsize is out of
>>>> bounds"
>>>> +
>>>> +    _scratch_unmount
>>>> +}
>>>> +
>>>> +sys_min_write=$(cat "/sys/block/$(_short_dev
>>>> $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
>>>> +sys_max_write=$(cat "/sys/block/$(_short_dev
>>>> $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
>>>> +
>>>> +bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>>>> +bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
>>>> +
>>>> +if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
>>>> +    echo "bdev min write != sys min write"
>>>> +fi
>>>> +if [ "$sys_max_write" -ne "$bdev_max_write" ]; then
>>>> +    echo "bdev max write != sys max write"
>>>> +fi
>>>> +
>>>> +# Test all supported block sizes between bdev min and max
>>>> +for ((bsize=$bdev_min_write; bsize<=bdev_max_write; bsize*=2)); do
>>>> +        test_atomic_writes $bsize
>>>> +done;
>>>> +
>>>> +# Check that atomic write fails if bsize < bdev min or bsize >
>>>> bdev max
>>>> +test_atomic_write_bounds $((bdev_min_write / 2))
>>>> +test_atomic_write_bounds $((bdev_max_write * 2))
>>>> +
>>>> +# success, all done
>>>> +echo Silence is golden
>>>> +status=0
>>>> +exit
>>>> diff --git a/tests/generic/762.out b/tests/generic/762.out
>>>> new file mode 100644
>>>> index 00000000..fbaeb297
>>>> --- /dev/null
>>>> +++ b/tests/generic/762.out
>>>> @@ -0,0 +1,2 @@
>>>> +QA output created by 762
>>>> +Silence is golden
>>>> -- 
>>>> 2.34.1
>>>>
>>>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore