On Mon, Apr 26, 2021 at 10:56:20PM +0800, bxue@xxxxxxxxxx wrote:
> From: Boyang Xue <bxue@xxxxxxxxxx>
>
> On ext2/ext3, it's expected that several single block metadata reads can occur
> when writing to file in the same cgroup (the stack is like below[1]). The
> purpose of the "write -> read/write" subtest is to make sure the larger pwrite
> is accounted to the correct cgroup, not necessarily enforce that zero bytes are
> read in service of the write. This patch fixes the sub-test in order to tolerate
> small reads in 1st cgroup.
>
> [1] Callchain of the read:
>
> @ext3_read_bio[
> submit_bio+1
> submit_bh_wbc+365
> ext4_read_bh+72
> ext4_get_branch+201
> ext4_ind_map_blocks+382
> ext4_map_blocks+295
> _ext4_get_block+170
> __block_write_begin_int+328
> ext4_write_begin+541
> generic_perform_write+213
> ext4_buffered_write_iter+167
> new_sync_write+345
> vfs_write+438
> __x64_sys_pwrite64+140
> do_syscall_64+51
> entry_SYSCALL_64_after_hwframe+68
> , 5793, 12]: 3
>
> Signed-off-by: Boyang Xue <bxue@xxxxxxxxxx>
> ---
LGTM, thanks for the fixups:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> Hi,
>
> This patch fix the "write -> read/write" sub-test in order to tolerate
> small reads in service of the write (like read metadata).
>
> Change from v2:
> Restore write tolerance to 5%, to match the historical behavior of the
> test.
>
> Thanks,
> Boyang
>
> tests/generic/563 | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/tests/generic/563 b/tests/generic/563
> index b113eacf..e73acebd 100755
> --- a/tests/generic/563
> +++ b/tests/generic/563
> @@ -60,6 +60,8 @@ check_cg()
> cgname=$(basename $cgroot)
> expectedread=$2
> expectedwrite=$3
> + readtol=$4
> + writetol=$5
> rbytes=0
> wbytes=0
>
> @@ -71,8 +73,8 @@ check_cg()
> awk -F = '{ print $2 }'`
> fi
>
> - _within_tolerance "read" $rbytes $expectedread 5% -v
> - _within_tolerance "write" $wbytes $expectedwrite 5% -v
> + _within_tolerance "read" $rbytes $expectedread $readtol -v
> + _within_tolerance "write" $wbytes $expectedwrite $writetol -v
> }
>
> # Move current process to another cgroup.
> @@ -113,7 +115,7 @@ $XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" -c fsync \
> $SCRATCH_MNT/file >> $seqres.full 2>&1
> switch_cg $cgdir
> $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> -check_cg $cgdir/$seq-cg $iosize $iosize
> +check_cg $cgdir/$seq-cg $iosize $iosize 5% 5%
>
> # Write from one cgroup then read and write from a second. Writes are charged to
> # the first group and nothing to the second.
> @@ -126,8 +128,12 @@ $XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" $SCRATCH_MNT/file \
> >> $seqres.full 2>&1
> switch_cg $cgdir
> $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> -check_cg $cgdir/$seq-cg 0 $iosize
> -check_cg $cgdir/$seq-cg-2 0 0
> +# Use a fixed value tolerance for the expected value of zero here
> +# because filesystems might perform a small number of metadata reads to
> +# complete the write. On ext2/3 with 1k block size, the read bytes is
> +# as large as 33792.
> +check_cg $cgdir/$seq-cg 0 $iosize 33792 5%
> +check_cg $cgdir/$seq-cg-2 0 0 0 0
>
> # Read from one cgroup, read & write from a second. Both reads and writes are
> # charged to the first group and nothing to the second.
> @@ -140,8 +146,8 @@ $XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" $SCRATCH_MNT/file \
> >> $seqres.full 2>&1
> switch_cg $cgdir
> $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> -check_cg $cgdir/$seq-cg $iosize $iosize
> -check_cg $cgdir/$seq-cg-2 0 0
> +check_cg $cgdir/$seq-cg $iosize $iosize 5% 5%
> +check_cg $cgdir/$seq-cg-2 0 0 0 0
>
> echo "-io" > $cgdir/cgroup.subtree_control || _fail "subtree control"
>
> --
> 2.27.0
>
