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

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



> On Mar 4, 2025, at 3:47 PM, Catherine Hoang <catherine.hoang@xxxxxxxxxx> wrote:
> 
>> 
>> On Mar 4, 2025, at 12:47 AM, Zorro Lang <zlang@xxxxxxxxxx> wrote:
>> 
>> On Mon, Mar 03, 2025 at 10:42:44PM +0000, Catherine Hoang wrote:
>>> 
>>> 
>>>> On Feb 28, 2025, at 7:43 AM, Darrick J. Wong <djwong@xxxxxxxxxx> 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@xxxxxxxxxx>
>>>>>>> Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@xxxxxxxxx>
>>>>>> 
>>>>>> 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.  
>>> 
>>> This test was based on for-next, I wasn’t aware there was another
>>> branch. I will rebase this onto patches-in-queue.
>> 
>> I can help to deal with the case number conflict too. It's good to me if
>> your patch is base on for-next, if you don't need to deal with some
>> conflicts with other "in-queue" patches, or pre-test un-pushed patches :)
> 
> Sounds good, thanks for letting me know! I will leave this patch as-is
> then, unless there are any other comments on it.

Hi Zorro,

I was wondering if there’s a chance this test would be picked up
soon? Or if there’s any changes I need to make to get this ready
to be merged?

Thanks,
Catherine
>> 
>> 
>> 
>> Thanks,
>> Zorro
>> 
>>>> 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 don’t think the atomic writes test was ever merged, unless I missed it?
>>>> 
>>>> (I suggest everyone please post urls to public git repos so reviewers
>>>> can get around these sorts of issues in the future.)
>>>> 
>>>> --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






[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux