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

Filipe Manana
2024-03-28T09:28:24Z

On Thu, Mar 28, 2024 at 8:41 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_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.

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.

>
> 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
>