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

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Dec 19, 2024 at 10:48:28AM +0000, John Garry 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@oracle.com>
> > ---
> >   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?

No, but I suppose the for loop at the bottom should stop at 64k.

> 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.
> 
> > +    _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.

Yeah, this test will need revision whenever we manage to finishing
running this marathon and get the next phase merged.  Until then,
let's at least test the existing functionality. :/

--D

> > +    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
> 
>