Re: [PATCH 3/6] generic/765: move common atomic write code to a library file

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Catherine Hoang <catherine.hoang@oracle.com> writes:

> From: "Darrick J. Wong" <djwong@kernel.org>
>
> Move the common atomic writes code to common/atomic so we can share
> them.
>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  common/atomicwrites | 111 ++++++++++++++++++++++++++++++++++++++++++++
>  common/rc           |  47 -------------------
>  tests/generic/765   |  53 ++-------------------
>  3 files changed, 114 insertions(+), 97 deletions(-)
>  create mode 100644 common/atomicwrites
>
> diff --git a/common/atomicwrites b/common/atomicwrites
> new file mode 100644
> index 00000000..fd3a9b71
> --- /dev/null
> +++ b/common/atomicwrites
> @@ -0,0 +1,111 @@
> +##/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2025 Oracle.  All Rights Reserved.
> +#
> +# Routines for testing atomic writes.
> +
> +_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 -w 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 -w atomic_write_segments_max | grep -o '[0-9]\+'
> +}
> +
> +_require_scratch_write_atomic()

NIT: It would be more convenient if we had this as _require_scratch_atomic_write()


> +{
> +	_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
> +}
> +
> +_test_atomic_file_writes()
> +{
> +    local bsize="$1"
> +    local testfile="$2"
> +    local bytes_written
> +    local testfile_cp="$testfile.copy"
> +
> +    # 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"
> +}
> diff --git a/common/rc b/common/rc
> index 3a70c707..781fc9ba 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5433,53 +5433,6 @@ _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 -w 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 -w 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/765 b/tests/generic/765
> index 84381730..09e9fa38 100755
> --- a/tests/generic/765
> +++ b/tests/generic/765
> @@ -9,6 +9,8 @@
>  . ./common/preamble
>  _begin_fstest auto quick rw atomicwrites
>  
> +. ./common/atomicwrites
> +
>  _require_scratch_write_atomic
>  _require_xfs_io_command pwrite -A
>  
> @@ -87,56 +89,7 @@ test_atomic_writes()
>      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"
> +    _test_atomic_file_writes "$bsize" "$testfile"

I see this must be done since in later patches we are adding an atomic
write test using scsi debug.

Sure make sense in that case. Please feel free to add:

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

-ritesh

>  
>      _scratch_unmount
>  }
> -- 
> 2.34.1