Re: [PATCH 04/10] btrfs: add helper to kill background process running _btrfs_stress_scrub

[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_scrub() 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 "scrub 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>
> ---
>   common/btrfs    | 14 ++++++++++++++
>   tests/btrfs/061 |  7 +------
>   tests/btrfs/066 |  8 ++------
>   tests/btrfs/069 | 11 +++++------
>   tests/btrfs/072 | 11 +++++------
>   tests/btrfs/073 | 11 +++++------
>   6 files changed, 32 insertions(+), 30 deletions(-)
> 
> diff --git a/common/btrfs b/common/btrfs
> index e95cff7f..d0adeea1 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -349,6 +349,20 @@ _btrfs_stress_scrub()
>   	done
>   }
>   
> +# Kill a background process running _btrfs_stress_scrub()
> +_btrfs_kill_stress_scrub_pid()
> +{
> +       local scrub_pid=$1
> +
> +       # Ignore if process already died.
> +       kill $scrub_pid &> /dev/null
> +       wait $scrub_pid &> /dev/null
> +       # Wait for the scrub operation to finish.


> +       while ps aux | grep "scrub start" | grep -qv grep; do

This can be replaced by pgrep. I'll make the change if there are
no objections.

       while pgrep -f "btrfs scrub start" > /dev/null; do

Thanks, Anand

> +               sleep 1
> +       done
> +}
> +
>   # stress btrfs by defragmenting every file/dir in a loop and compress file
>   # contents while defragmenting if second argument is not "nocompress"
>   _btrfs_stress_defrag()
> diff --git a/tests/btrfs/061 b/tests/btrfs/061
> index d0b55e48..b8b2706c 100755
> --- a/tests/btrfs/061
> +++ b/tests/btrfs/061
> @@ -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 $scrub_pid
> -	wait $scrub_pid
> -	# wait for the crub operation to finish
> -	while ps aux | grep "scrub start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	_btrfs_kill_stress_scrub_pid $scrub_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/066 b/tests/btrfs/066
> index a29034bb..29821fdd 100755
> --- a/tests/btrfs/066
> +++ b/tests/btrfs/066
> @@ -57,12 +57,8 @@ run_test()
>   	wait $fsstress_pid
>   
>   	touch $stop_file
> -	kill $scrub_pid
> -	wait
> -	# wait for the scrub operation to finish
> -	while ps aux | grep "scrub start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	wait $subvol_pid
> +	_btrfs_kill_stress_scrub_pid $scrub_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 139dde48..20f44b39 100755
> --- a/tests/btrfs/069
> +++ b/tests/btrfs/069
> @@ -59,17 +59,16 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $replace_pid $scrub_pid
> -	wait
> +	kill $replace_pid
> +	wait $replace_pid
>   
> -	# wait for the scrub and replace operations to finish
> -	while ps aux | grep "scrub start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	# 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
> +
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
>   	if [ $? -ne 0 ]; then
> diff --git a/tests/btrfs/072 b/tests/btrfs/072
> index 4b6b6fb5..6c15b51f 100755
> --- a/tests/btrfs/072
> +++ b/tests/btrfs/072
> @@ -52,16 +52,15 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $scrub_pid $defrag_pid
> -	wait
> -	# wait for the scrub and defrag operations to finish
> -	while ps aux | grep "scrub start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	kill $defrag_pid
> +	wait $defrag_pid
> +	# wait for the defrag operation to finish
>   	while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
>   		sleep 1
>   	done
>   
> +	_btrfs_kill_stress_scrub_pid $scrub_pid
> +
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
>   	if [ $? -ne 0 ]; then
> diff --git a/tests/btrfs/073 b/tests/btrfs/073
> index b1604f94..49a4abd1 100755
> --- a/tests/btrfs/073
> +++ b/tests/btrfs/073
> @@ -51,16 +51,15 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $scrub_pid $remount_pid
> -	wait
> -	# wait for the scrub and remount operations to finish
> -	while ps aux | grep "scrub start" | grep -qv grep; do
> -		sleep 1
> -	done
> +	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_scrub_pid $scrub_pid
> +
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
>   	if [ $? -ne 0 ]; then