Re: [PATCH 07/10] btrfs: add helper to kill background process running _btrfs_stress_replace

[Anand Jain <anand.jain@oracle.com>]

On 3/28/24 01:11, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Killing a background process running _btrfs_stress_replace() 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 "replace start" | 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    | 14 ++++++++++++++
>   tests/btrfs/064 |  7 +------
>   tests/btrfs/065 |  8 ++------
>   tests/btrfs/069 |  9 +--------
>   tests/btrfs/070 |  9 +--------
>   tests/btrfs/071 | 10 ++--------
>   6 files changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/common/btrfs b/common/btrfs
> index 66a3a347..6d7e7a68 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -475,6 +475,20 @@ _btrfs_stress_replace()
>   	done
>   }
>   
> +# Kill a background process running _btrfs_stress_replace()
> +_btrfs_kill_stress_replace_pid()
> +{
> +       local replace_pid=$1
> +
> +       # Ignore if process already died.
> +       kill $replace_pid &> /dev/null
> +       wait $replace_pid &> /dev/null
> +       # Wait for the replace operation to finish.
> +       while ps aux | grep "replace start" | grep -qv grep; do
> +               sleep 1
> +       done
> +}
> +
>   # find the right option to force output in bytes, older versions of btrfs-progs
>   # print that by default, newer print human readable numbers with unit suffix
>   _btrfs_qgroup_units()
> diff --git a/tests/btrfs/064 b/tests/btrfs/064
> index 58b53afe..9e0b3b30 100755
> --- a/tests/btrfs/064
> +++ b/tests/btrfs/064
> @@ -64,12 +64,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 $replace_pid
> -	wait $replace_pid
> -	# wait for the replace operation to finish
> -	while ps aux | grep "replace start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	_btrfs_kill_stress_replace_pid $replace_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/065 b/tests/btrfs/065
> index c4b6aafe..d2b04178 100755
> --- a/tests/btrfs/065
> +++ b/tests/btrfs/065
> @@ -65,12 +65,8 @@ run_test()
>   	wait $fsstress_pid
>   
>   	touch $stop_file
> -	kill $replace_pid
> -	wait
> -	# wait for the replace operation to finish
> -	while ps aux | grep "replace start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	wait $subvol_pid
> +	_btrfs_kill_stress_replace_pid $replace_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/069 b/tests/btrfs/069
> index 20f44b39..ad1609d4 100755
> --- a/tests/btrfs/069
> +++ b/tests/btrfs/069
> @@ -59,15 +59,8 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $replace_pid
> -	wait $replace_pid
> -
> -	# wait for the replace operation to finish
> -	while ps aux | grep "replace start" | grep -qv grep; do
> -		sleep 1
> -	done
> -
>   	_btrfs_kill_stress_scrub_pid $scrub_pid
> +	_btrfs_kill_stress_replace_pid $replace_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/070 b/tests/btrfs/070
> index cefa5723..3054c270 100755
> --- a/tests/btrfs/070
> +++ b/tests/btrfs/070
> @@ -60,14 +60,7 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $replace_pid
> -	wait $replace_pid
> -
> -	# wait for the replace operation to finish
> -	while ps aux | grep "replace start" | grep -qv grep; do
> -		sleep 1
> -	done
> -
> +	_btrfs_kill_stress_replace_pid $replace_pid
>   	_btrfs_kill_stress_defrag_pid $defrag_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
> diff --git a/tests/btrfs/071 b/tests/btrfs/071
> index 7ba15390..36b39341 100755
> --- a/tests/btrfs/071
> +++ b/tests/btrfs/071
> @@ -58,14 +58,8 @@ run_test()
>   	echo "$remount_pid" >>$seqres.full
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> -	kill $replace_pid
> -	wait $fsstress_pid $replace_pid
> -
> -	# wait for the replace operationss to finish
> -	while ps aux | grep "replace start" | grep -qv grep; do
> -		sleep 1
> -	done
> -
> +	wait $fsstress_pid
> +	_btrfs_kill_stress_replace_pid $replace_pid
>   	_btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
>   
>   	echo "Scrub the filesystem" >>$seqres.full