Re: [PATCH 1/5] common: move mread() to generic helper _mread()

Darrick J. Wong
2024-06-11T14:28:30Z

On Mon, Jun 10, 2024 at 08:01:58PM -0700, Luis Chamberlain wrote:
> We want a shared way to use mmap in a way that we can test
> for the SIGBUS, provide a shared routine which other tests can
> leverage.
>
> Suggested-by: "Darrick J. Wong" <djwong@kernel.org>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> common/rc | 28 ++++++++++++++++++++++++++++
> tests/generic/574 | 36 ++++--------------------------------
> 2 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 163041fea5b9..fa7942809d6c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -52,6 +52,34 @@ _pwrite_byte() {
> $XFS_IO_PROG $xfs_io_args -f -c "pwrite -S $pattern $offset $len" "$file"
> }
>
> +_round_up_to_page_boundary()
> +{
> + local n=$1
> + local page_size=$(_get_page_size)
> +
> + echo $(( (n + page_size - 1) & ~(page_size - 1) ))
> +}
> +
> +_mread()
> +{
> + local file=$1
> + local offset=$2
> + local length=$3
> + local map_len=$(_round_up_to_page_boundary $(_get_filesize $file))
> +
> + # Some callers expect xfs_io to crash with SIGBUS due to the mread,
> + # causing the shell to print "Bus error" to stderr. To allow this
> + # message to be redirected, execute xfs_io in a new shell instance.
> + # However, for this to work reliably, we also need to prevent the new
> + # shell instance from optimizing out the fork and directly exec'ing
> + # xfs_io. The easiest way to do that is to append 'true' to the
> + # commands, so that xfs_io is no longer the last command the shell sees.
> + # Don't let it write core files to the filesystem.
> + bash -c "trap '' SIGBUS; ulimit -c 0; $XFS_IO_PROG -r $file \
> + -c 'mmap -r 0 $map_len' \
> + -c 'mread -v $offset $length'; true"
> +}
> +
> # mmap-write a byte into a range of a file
> _mwrite_byte() {
> local pattern="$1"
> diff --git a/tests/generic/574 b/tests/generic/574
> index cb42baaa67aa..d44c23e5abc2 100755
> --- a/tests/generic/574
> +++ b/tests/generic/574
> @@ -52,34 +52,6 @@ setup_zeroed_file()
> cmp $fsv_orig_file $fsv_file
> }
>
> -round_up_to_page_boundary()
> -{
> - local n=$1
> - local page_size=$(_get_page_size)
> -
> - echo $(( (n + page_size - 1) & ~(page_size - 1) ))
> -}
> -
> -mread()
> -{
> - local file=$1
> - local offset=$2
> - local length=$3
> - local map_len=$(round_up_to_page_boundary $(_get_filesize $file))
> -
> - # Some callers expect xfs_io to crash with SIGBUS due to the mread,
> - # causing the shell to print "Bus error" to stderr. To allow this
> - # message to be redirected, execute xfs_io in a new shell instance.
> - # However, for this to work reliably, we also need to prevent the new
> - # shell instance from optimizing out the fork and directly exec'ing
> - # xfs_io. The easiest way to do that is to append 'true' to the
> - # commands, so that xfs_io is no longer the last command the shell sees.
> - # Don't let it write core files to the filesystem.
> - bash -c "trap '' SIGBUS; ulimit -c 0; $XFS_IO_PROG -r $file \
> - -c 'mmap -r 0 $map_len' \
> - -c 'mread -v $offset $length'; true"
> -}
> -
> corruption_test()
> {
> local block_size=$1
> @@ -142,7 +114,7 @@ corruption_test()
> fi
>
> # Reading the full file via mmap should fail.
> - mread $fsv_file 0 $file_len >/dev/null 2>$tmp.err
> + _mread $fsv_file 0 $file_len >/dev/null 2>$tmp.err
> if ! grep -q 'Bus error' $tmp.err; then
> echo "Didn't see SIGBUS when reading file via mmap"
> cat $tmp.err
> @@ -150,7 +122,7 @@ corruption_test()
>
> # Reading just the corrupted part via mmap should fail.
> if ! $is_merkle_tree; then
> - mread $fsv_file $zap_offset $zap_len >/dev/null 2>$tmp.err
> + _mread $fsv_file $zap_offset $zap_len >/dev/null 2>$tmp.err
> if ! grep -q 'Bus error' $tmp.err; then
> echo "Didn't see SIGBUS when reading corrupted part via mmap"
> cat $tmp.err
> @@ -174,10 +146,10 @@ corrupt_eof_block_test()
> head -c $zap_len /dev/zero | tr '\0' X \
> | _fsv_scratch_corrupt_bytes $fsv_file $file_len
>
> - mread $fsv_file $file_len $zap_len >$tmp.out 2>$tmp.err
> + _mread $fsv_file $file_len $zap_len >$tmp.out 2>$tmp.err
>
> head -c $file_len /dev/zero >$tmp.zeroes
> - mread $tmp.zeroes $file_len $zap_len >$tmp.zeroes_out
> + _mread $tmp.zeroes $file_len $zap_len >$tmp.zeroes_out
>
> grep -q 'Bus error' $tmp.err || diff $tmp.out $tmp.zeroes_out
> }
> --
> 2.43.0
>
>