[PATCH v3] generic/563: tolerate small reads in "write -> read/write" sub-test

[PATCH v3] generic/563: tolerate small reads in "write -> read/write" sub-test

[bxue]

From: Boyang Xue <bxue@redhat.com>

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@redhat.com>
---
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

Re: [PATCH v3] generic/563: tolerate small reads in "write -> read/write" sub-test

[Brian Foster]

On Mon, Apr 26, 2021 at 10:56:20PM +0800, bxue@redhat.com wrote:
> From: Boyang Xue <bxue@redhat.com>
> 
> 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@redhat.com>
> ---

LGTM, thanks for the fixups:

Reviewed-by: Brian Foster <bfoster@redhat.com>

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