Re: [PATCH 3/3] xfs/182: fix spurious direct write failure

[Xiao Yang <yangx.jy@fujitsu.com>]

Hi Darrick,

It looks good to me. It actually fixed the failure of xfs/182 with 
enabled fsdax and reflink.
Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com>

Best Regards,
Xiao Yang

On 2023/1/18 8:42, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test has some weird behavior that causes regressions when fsdax and
> reflink are enabled.  The goal of this test is to set a cow extent size
> hint, perform some random directio writes, perform a directio rewrite of
> the entire file, and make sure that the file content (and extent count)
> are sane afterwards.
> 
> Most of the time, the random directio writes will never touch the
> 8388609th byte, though if they do randomly select that EOF block, they'd
> end up extending the file by $real_blksz bytes and causing spurious test
> failures.
> 
> Then, the rewrite does this:
> 
> pwrite -S 0x63 -b $real_blksz 0 $((filesize + 1))
> 
> Note that we previously set filesize=8388608, which means that we're
> asking for a series of direct writes that fill the first 8388608 bytes
> with 'c'.  The last write in the series becomes a single byte direct
> write.  For regular file access mode, this last write will fail with
> EINVAL, since block devices do not support byte granularity writes and
> XFS does not fall back to the pagecache for unaligned direct wites.
> Hence we never wrote the 8388609th byte of the file.
> 
> However, fsdax *does* allow byte-granularity direct writes, which means
> that the single-byte write succeeds.  There is no EINVAL return code,
> and the 8388609th byte of the file is now 'c' instead of 'a'.  As a
> result, the md5 of file2 is different.
> 
> Since fsdax+reflink is the newcomer, amend the direct writes in this
> test so that they always end at the 8388608th byte, since we were never
> really testing that last byte anyway.  This makes the test behavior
> consistent across both access modes.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>   tests/xfs/182     |    4 ++--
>   tests/xfs/182.out |    1 -
>   2 files changed, 2 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/tests/xfs/182 b/tests/xfs/182
> index ec3f7dc026..696b933e60 100755
> --- a/tests/xfs/182
> +++ b/tests/xfs/182
> @@ -55,9 +55,9 @@ md5sum $testdir/file2 | _filter_scratch
>   
>   echo "CoW and unmount"
>   $XFS_IO_PROG -f -c "cowextsize" $testdir/file2 >> $seqres.full
> -$XFS_IO_PROG -d -f -c "pwrite -R -S 0x63 -b $real_blksz 0 $((filesize + 1))" \
> +$XFS_IO_PROG -d -f -c "pwrite -R -S 0x63 -b $real_blksz 0 $filesize" \
>   	$testdir/file2 2>&1 >> $seqres.full | _filter_xfs_io_error
> -$XFS_IO_PROG -d -f -c "pwrite -S 0x63 -b $real_blksz 0 $((filesize + 1))" \
> +$XFS_IO_PROG -d -f -c "pwrite -S 0x63 -b $real_blksz 0 $filesize" \
>   	$testdir/file2 2>&1 >> $seqres.full | _filter_xfs_io_error
>   _scratch_cycle_mount
>   
> diff --git a/tests/xfs/182.out b/tests/xfs/182.out
> index 41384437ad..8821bcd5bd 100644
> --- a/tests/xfs/182.out
> +++ b/tests/xfs/182.out
> @@ -5,7 +5,6 @@ Compare files
>   2909feb63a37b0e95fe5cfb7f274f7b1  SCRATCH_MNT/test-182/file1
>   2909feb63a37b0e95fe5cfb7f274f7b1  SCRATCH_MNT/test-182/file2
>   CoW and unmount
> -pwrite: Invalid argument
>   Compare files
>   2909feb63a37b0e95fe5cfb7f274f7b1  SCRATCH_MNT/test-182/file1
>   c6ba35da9f73ced20d7781a448cc11d4  SCRATCH_MNT/test-182/file2
>