Re: [PATCH 06/10] btrfs: add helper to kill background process running _btrfs_stress_remount_compress

Anand Jain
2024-03-28T09:25:09Z

On 3/28/24 01:11, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Killing a background process running _btrfs_stress_remount_compress() is
> not as simple as sending a signal to the process and waiting for it to
> die. Therefore we have the following logic to terminate such process:
>
> kill $pid
> wait $pid
> while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> sleep 1
> done
>
> Since this is repeated in several test cases, move this logic to a common
> helper and use it in all affected test cases. This will help to avoid
> repeating the same code again several times in upcoming changes.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

> ---
> common/btrfs | 15 +++++++++++++++
> tests/btrfs/063 | 7 +------
> tests/btrfs/068 | 8 ++------
> tests/btrfs/071 | 12 +++++-------
> tests/btrfs/073 | 8 +-------
> tests/btrfs/074 | 8 +-------
> 6 files changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/common/btrfs b/common/btrfs
> index 46056d4a..66a3a347 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -411,6 +411,21 @@ _btrfs_stress_remount_compress()
> done
> }
>
> +# Kill a background process running _btrfs_stress_remount_compress()
> +_btrfs_kill_stress_remount_compress_pid()
> +{
> + local remount_pid=$1
> + local btrfs_mnt=$2
> +
> + # Ignore if process already died.
> + kill $remount_pid &> /dev/null
> + wait $remount_pid &> /dev/null
> + # Wait for the remount loop to finish.
> + while ps aux | grep "mount.*${btrfs_mnt}" | grep -qv grep; do
> + sleep 1
> + done
> +}
> +
> # stress btrfs by replacing devices in a loop
> # Note that at least 3 devices are needed in SCRATCH_DEV_POOL and the last
> # device should be free(not used by btrfs)
> diff --git a/tests/btrfs/063 b/tests/btrfs/063
> index baf0c356..5ee2837f 100755
> --- a/tests/btrfs/063
> +++ b/tests/btrfs/063
> @@ -52,12 +52,7 @@ run_test()
> echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> wait $fsstress_pid
> _btrfs_kill_stress_balance_pid $balance_pid
> - kill $remount_pid
> - wait $remount_pid
> - # wait for the remount loop to finish
> - while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> - sleep 1
> - done
> + _btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
>
> echo "Scrub the filesystem" >>$seqres.full
> $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/068 b/tests/btrfs/068
> index 15fd41db..db53254a 100755
> --- a/tests/btrfs/068
> +++ b/tests/btrfs/068
> @@ -58,12 +58,8 @@ run_test()
> wait $fsstress_pid
>
> touch $stop_file
> - kill $remount_pid
> - wait
> - # wait for the remount loop process to finish
> - while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> - sleep 1
> - done
> + wait $subvol_pid
> + _btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
>
> echo "Scrub the filesystem" >>$seqres.full
> $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/071 b/tests/btrfs/071
> index 6ebbd8cc..7ba15390 100755
> --- a/tests/btrfs/071
> +++ b/tests/btrfs/071
> @@ -58,17 +58,15 @@ run_test()
> echo "$remount_pid" >>$seqres.full
>
> echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> - wait $fsstress_pid
> - kill $replace_pid $remount_pid
> - wait
> + kill $replace_pid
> + wait $fsstress_pid $replace_pid
>
> - # wait for the remount and replace operations to finish
> + # wait for the replace operationss to finish
> while ps aux | grep "replace start" | grep -qv grep; do
> sleep 1
> done
> - while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> - sleep 1
> - done
> +
> + _btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
>
> echo "Scrub the filesystem" >>$seqres.full
> $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/073 b/tests/btrfs/073
> index 49a4abd1..50358286 100755
> --- a/tests/btrfs/073
> +++ b/tests/btrfs/073
> @@ -51,13 +51,7 @@ run_test()
>
> echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> wait $fsstress_pid
> - kill $remount_pid
> - wait $remount_pid
> - # wait for the remount operation to finish
> - while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> - sleep 1
> - done
> -
> + _btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
> _btrfs_kill_stress_scrub_pid $scrub_pid
>
> echo "Scrub the filesystem" >>$seqres.full
> diff --git a/tests/btrfs/074 b/tests/btrfs/074
> index d51922d0..6e93b36a 100755
> --- a/tests/btrfs/074
> +++ b/tests/btrfs/074
> @@ -52,13 +52,7 @@ run_test()
>
> echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> wait $fsstress_pid
> - kill $remount_pid
> - wait $remount_pid
> - # wait for the remount operation to finish
> - while ps aux | grep "mount.*$SCRATCH_MNT" | grep -qv grep; do
> - sleep 1
> - done
> -
> + _btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
> _btrfs_kill_stress_defrag_pid $defrag_pid
>
> echo "Scrub the filesystem" >>$seqres.full