diff mbox series

[05/10] btrfs: add helper to kill background process running _btrfs_stress_defrag

Message ID 247bde0d4f7d943337e228dded8ad03753b0e3c9.1711558345.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series fstests/btrfs: fixes for tests btrfs/06[0-9] and btrfs/07[0-4] | expand

Commit Message

Filipe Manana March 27, 2024, 5:11 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

Killing a background process running _btrfs_stress_defrag() 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 "btrfs filesystem defrag" | 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/062 |  7 +------
 tests/btrfs/067 |  8 ++------
 tests/btrfs/070 | 11 +++++------
 tests/btrfs/072 |  7 +------
 tests/btrfs/074 | 11 +++++------
 6 files changed, 28 insertions(+), 30 deletions(-)

Comments

Anand Jain March 28, 2024, 9:18 a.m. UTC | #1 
On 3/28/24 01:11, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Killing a background process running _btrfs_stress_defrag() 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 "btrfs filesystem defrag" | 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/062 |  7 +------
>   tests/btrfs/067 |  8 ++------
>   tests/btrfs/070 | 11 +++++------
>   tests/btrfs/072 |  7 +------
>   tests/btrfs/074 | 11 +++++------
>   6 files changed, 28 insertions(+), 30 deletions(-)
> 
> diff --git a/common/btrfs b/common/btrfs
> index d0adeea1..46056d4a 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -383,6 +383,20 @@ _btrfs_stress_defrag()
>   	done
>   }
>   
> +# Kill a background process running _btrfs_stress_defrag()
> +_btrfs_kill_stress_defrag_pid()
> +{
> +       local defrag_pid=$1
> +
> +       # Ignore if process already died.
> +       kill $defrag_pid &> /dev/null
> +       wait $defrag_pid &> /dev/null
> +       # Wait for the defrag operation to finish.
> +       while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do

The same comments apply here regarding the use of pgrep.

Looks good.
Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

> +               sleep 1
> +       done
> +}
> +
>   # stress btrfs by remounting it with different compression algorithms in a loop
>   # run this with fsstress running at background could exercise the compression
>   # code path and ensure no race when switching compression algorithm with constant
> diff --git a/tests/btrfs/062 b/tests/btrfs/062
> index a2639d6c..59d581be 100755
> --- a/tests/btrfs/062
> +++ b/tests/btrfs/062
> @@ -53,12 +53,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 $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_defrag_pid $defrag_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/btrfs/067 b/tests/btrfs/067
> index 709db155..2bb00b87 100755
> --- a/tests/btrfs/067
> +++ b/tests/btrfs/067
> @@ -58,12 +58,8 @@ run_test()
>   	wait $fsstress_pid
>   
>   	touch $stop_file
> -	kill $defrag_pid
> -	wait
> -	# wait for btrfs defrag process to exit, otherwise it will block umount
> -	while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
> -		sleep 1
> -	done
> +	wait $subvol_pid
> +	_btrfs_kill_stress_defrag_pid $defrag_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 54aa275c..cefa5723 100755
> --- a/tests/btrfs/070
> +++ b/tests/btrfs/070
> @@ -60,17 +60,16 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $replace_pid $defrag_pid
> -	wait
> +	kill $replace_pid
> +	wait $replace_pid
>   
> -	# wait for the defrag and replace operations to finish
> -	while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_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 6c15b51f..505d0b57 100755
> --- a/tests/btrfs/072
> +++ b/tests/btrfs/072
> @@ -52,13 +52,8 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	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_defrag_pid $defrag_pid
>   	_btrfs_kill_stress_scrub_pid $scrub_pid
>   
>   	echo "Scrub the filesystem" >>$seqres.full
> diff --git a/tests/btrfs/074 b/tests/btrfs/074
> index 9b22c620..d51922d0 100755
> --- a/tests/btrfs/074
> +++ b/tests/btrfs/074
> @@ -52,16 +52,15 @@ run_test()
>   
>   	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
>   	wait $fsstress_pid
> -	kill $defrag_pid $remount_pid
> -	wait
> -	# wait for the defrag and remount operations to finish
> -	while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_pid
> +
>   	echo "Scrub the filesystem" >>$seqres.full
>   	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
>   	if [ $? -ne 0 ]; then
Filipe Manana March 28, 2024, 9:32 a.m. UTC | #2 
On Thu, Mar 28, 2024 at 9:19 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> On 3/28/24 01:11, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Killing a background process running _btrfs_stress_defrag() 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 "btrfs filesystem defrag" | 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/062 |  7 +------
> >   tests/btrfs/067 |  8 ++------
> >   tests/btrfs/070 | 11 +++++------
> >   tests/btrfs/072 |  7 +------
> >   tests/btrfs/074 | 11 +++++------
> >   6 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/common/btrfs b/common/btrfs
> > index d0adeea1..46056d4a 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -383,6 +383,20 @@ _btrfs_stress_defrag()
> >       done
> >   }
> >
> > +# Kill a background process running _btrfs_stress_defrag()
> > +_btrfs_kill_stress_defrag_pid()
> > +{
> > +       local defrag_pid=$1
> > +
> > +       # Ignore if process already died.
> > +       kill $defrag_pid &> /dev/null
> > +       wait $defrag_pid &> /dev/null
> > +       # Wait for the defrag operation to finish.
> > +       while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
>
> The same comments apply here regarding the use of pgrep.

Yes, but as previously pointed out, the goal of the patch is just to
move repeated code into a helper function.
I would prefer such a change to be done separately.

Thanks.

>
> Looks good.
> Reviewed-by: Anand Jain <anand.jain@oracle.com>
>
> Thanks, Anand
>
> > +               sleep 1
> > +       done
> > +}
> > +
> >   # stress btrfs by remounting it with different compression algorithms in a loop
> >   # run this with fsstress running at background could exercise the compression
> >   # code path and ensure no race when switching compression algorithm with constant
> > diff --git a/tests/btrfs/062 b/tests/btrfs/062
> > index a2639d6c..59d581be 100755
> > --- a/tests/btrfs/062
> > +++ b/tests/btrfs/062
> > @@ -53,12 +53,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 $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_defrag_pid $defrag_pid
> >
> >       echo "Scrub the filesystem" >>$seqres.full
> >       $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> > diff --git a/tests/btrfs/067 b/tests/btrfs/067
> > index 709db155..2bb00b87 100755
> > --- a/tests/btrfs/067
> > +++ b/tests/btrfs/067
> > @@ -58,12 +58,8 @@ run_test()
> >       wait $fsstress_pid
> >
> >       touch $stop_file
> > -     kill $defrag_pid
> > -     wait
> > -     # wait for btrfs defrag process to exit, otherwise it will block umount
> > -     while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
> > -             sleep 1
> > -     done
> > +     wait $subvol_pid
> > +     _btrfs_kill_stress_defrag_pid $defrag_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 54aa275c..cefa5723 100755
> > --- a/tests/btrfs/070
> > +++ b/tests/btrfs/070
> > @@ -60,17 +60,16 @@ run_test()
> >
> >       echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> >       wait $fsstress_pid
> > -     kill $replace_pid $defrag_pid
> > -     wait
> > +     kill $replace_pid
> > +     wait $replace_pid
> >
> > -     # wait for the defrag and replace operations to finish
> > -     while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_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 6c15b51f..505d0b57 100755
> > --- a/tests/btrfs/072
> > +++ b/tests/btrfs/072
> > @@ -52,13 +52,8 @@ run_test()
> >
> >       echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> >       wait $fsstress_pid
> > -     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_defrag_pid $defrag_pid
> >       _btrfs_kill_stress_scrub_pid $scrub_pid
> >
> >       echo "Scrub the filesystem" >>$seqres.full
> > diff --git a/tests/btrfs/074 b/tests/btrfs/074
> > index 9b22c620..d51922d0 100755
> > --- a/tests/btrfs/074
> > +++ b/tests/btrfs/074
> > @@ -52,16 +52,15 @@ run_test()
> >
> >       echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> >       wait $fsstress_pid
> > -     kill $defrag_pid $remount_pid
> > -     wait
> > -     # wait for the defrag and remount operations to finish
> > -     while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_pid
> > +
> >       echo "Scrub the filesystem" >>$seqres.full
> >       $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> >       if [ $? -ne 0 ]; then
>
diff mbox series

Patch

diff --git a/common/btrfs b/common/btrfs
index d0adeea1..46056d4a 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -383,6 +383,20 @@  _btrfs_stress_defrag()
 	done
 }
 
+# Kill a background process running _btrfs_stress_defrag()
+_btrfs_kill_stress_defrag_pid()
+{
+       local defrag_pid=$1
+
+       # Ignore if process already died.
+       kill $defrag_pid &> /dev/null
+       wait $defrag_pid &> /dev/null
+       # Wait for the defrag operation to finish.
+       while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
+               sleep 1
+       done
+}
+
 # stress btrfs by remounting it with different compression algorithms in a loop
 # run this with fsstress running at background could exercise the compression
 # code path and ensure no race when switching compression algorithm with constant
diff --git a/tests/btrfs/062 b/tests/btrfs/062
index a2639d6c..59d581be 100755
--- a/tests/btrfs/062
+++ b/tests/btrfs/062
@@ -53,12 +53,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 $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_defrag_pid $defrag_pid
 
 	echo "Scrub the filesystem" >>$seqres.full
 	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
diff --git a/tests/btrfs/067 b/tests/btrfs/067
index 709db155..2bb00b87 100755
--- a/tests/btrfs/067
+++ b/tests/btrfs/067
@@ -58,12 +58,8 @@  run_test()
 	wait $fsstress_pid
 
 	touch $stop_file
-	kill $defrag_pid
-	wait
-	# wait for btrfs defrag process to exit, otherwise it will block umount
-	while ps aux | grep "btrfs filesystem defrag" | grep -qv grep; do
-		sleep 1
-	done
+	wait $subvol_pid
+	_btrfs_kill_stress_defrag_pid $defrag_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 54aa275c..cefa5723 100755
--- a/tests/btrfs/070
+++ b/tests/btrfs/070
@@ -60,17 +60,16 @@  run_test()
 
 	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
 	wait $fsstress_pid
-	kill $replace_pid $defrag_pid
-	wait
+	kill $replace_pid
+	wait $replace_pid
 
-	# wait for the defrag and replace operations to finish
-	while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_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 6c15b51f..505d0b57 100755
--- a/tests/btrfs/072
+++ b/tests/btrfs/072
@@ -52,13 +52,8 @@  run_test()
 
 	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
 	wait $fsstress_pid
-	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_defrag_pid $defrag_pid
 	_btrfs_kill_stress_scrub_pid $scrub_pid
 
 	echo "Scrub the filesystem" >>$seqres.full
diff --git a/tests/btrfs/074 b/tests/btrfs/074
index 9b22c620..d51922d0 100755
--- a/tests/btrfs/074
+++ b/tests/btrfs/074
@@ -52,16 +52,15 @@  run_test()
 
 	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
 	wait $fsstress_pid
-	kill $defrag_pid $remount_pid
-	wait
-	# wait for the defrag and remount operations to finish
-	while ps aux | grep "btrfs filesystem defrag" | 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_defrag_pid $defrag_pid
+
 	echo "Scrub the filesystem" >>$seqres.full
 	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
 	if [ $? -ne 0 ]; then