On Thu, Mar 28, 2024 at 8:17 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote:
>
> On 3/28/24 01:11, fdmanana@xxxxxxxxxx wrote:
> > From: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Killing a background process running _btrfs_stress_balance() 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
> > # Wait for the balance operation to finish.
> > while ps aux | grep "balance 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@xxxxxxxx>
>
> Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>
>
> A few minor things below.
>
> > ---
> > common/btrfs | 14 ++++++++++++++
> > tests/btrfs/060 | 8 ++------
> > tests/btrfs/061 | 10 ++++------
> > tests/btrfs/062 | 10 ++++------
> > tests/btrfs/063 | 10 ++++------
> > tests/btrfs/064 | 10 ++++------
> > tests/btrfs/255 | 8 ++------
> > 7 files changed, 34 insertions(+), 36 deletions(-)
> >
> > diff --git a/common/btrfs b/common/btrfs
> > index aa344706..e95cff7f 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -308,6 +308,20 @@ _btrfs_stress_balance()
> > done
> > }
> >
> > +# Kill a background process running _btrfs_stress_balance()
> > +_btrfs_kill_stress_balance_pid()
> > +{
> > + local balance_pid=$1
> > +
> > + # Ignore if process already died.
> > + kill $balance_pid &> /dev/null
> > + wait $balance_pid &> /dev/null
> > + # Wait for the balance operation to finish.
> > + while ps aux | grep "balance start" | grep -qv grep; do
>
>
> We can use pgrep instead. I will make the following changes before
> merging if you are okay with it.
Not doing such a change was intentional, as the goal is just to move
repeated code into a helper.
I would prefer for such change to be done separately in its own patch.
>
> - while ps aux | grep "balance start" | grep -qv grep; do
> + while pgrep -f "btrfs balance start" > /dev/null; do
>
>
>
> > + sleep 1
> > + done
> > +}
> > +
> > # stress btrfs by creating/mounting/umounting/deleting subvolume in a loop
> > _btrfs_stress_subvolume()
> > {
> > diff --git a/tests/btrfs/060 b/tests/btrfs/060
> > index a0184891..58167cc6 100755
> > --- a/tests/btrfs/060
> > +++ b/tests/btrfs/060
> > @@ -57,12 +57,8 @@ run_test()
> > wait $fsstress_pid
> >
> > touch $stop_file
> > - kill $balance_pid
> > - wait
> > - # wait for the balance operation to finish
> > - while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > - done
> > + wait $subvol_pid
> > + _btrfs_kill_stress_balance_pid $balance_pid
> >
> > echo "Scrub the filesystem" >>$seqres.full
> > $BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
> > diff --git a/tests/btrfs/061 b/tests/btrfs/061
> > index c1010413..d0b55e48 100755
> > --- a/tests/btrfs/061
> > +++ b/tests/btrfs/061
> > @@ -51,12 +51,10 @@ run_test()
> >
> > echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> > wait $fsstress_pid
> > - kill $balance_pid $scrub_pid
> > - wait
> > - # wait for the balance and scrub operations to finish
> > - while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > - done
> > + _btrfs_kill_stress_balance_pid $balance_pid
> > + kill $scrub_pid
> > + wait $scrub_pid
> > + # wait for the crub operation to finish
>
> s/crub/scrub/
Yeah, but the comment is going away in the patch that adds the helper
to kill scrub.
Thanks.
>
> Thanks, Anand
>
> > while ps aux | grep "scrub start" | grep -qv grep; do
> > sleep 1
> > done
> > diff --git a/tests/btrfs/062 b/tests/btrfs/062
> > index 818a0156..a2639d6c 100755
> > --- a/tests/btrfs/062
> > +++ b/tests/btrfs/062
> > @@ -52,12 +52,10 @@ run_test()
> >
> > echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> > wait $fsstress_pid
> > - kill $balance_pid $defrag_pid
> > - wait
> > - # wait for the balance and defrag operations to finish
> > - while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > - done
> > + _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
> > diff --git a/tests/btrfs/063 b/tests/btrfs/063
> > index 2f771baf..baf0c356 100755
> > --- a/tests/btrfs/063
> > +++ b/tests/btrfs/063
> > @@ -51,12 +51,10 @@ run_test()
> >
> > echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> > wait $fsstress_pid
> > - kill $balance_pid $remount_pid
> > - wait
> > - # wait for the balance and remount loop to finish
> > - while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > - done
> > + _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
> > diff --git a/tests/btrfs/064 b/tests/btrfs/064
> > index e9b46ce6..58b53afe 100755
> > --- a/tests/btrfs/064
> > +++ b/tests/btrfs/064
> > @@ -63,12 +63,10 @@ run_test()
> >
> > echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
> > wait $fsstress_pid
> > - kill $balance_pid $replace_pid
> > - wait
> > - # wait for the balance and replace operations to finish
> > - while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > - done
> > + _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
> > diff --git a/tests/btrfs/255 b/tests/btrfs/255
> > index 7e70944a..aa250467 100755
> > --- a/tests/btrfs/255
> > +++ b/tests/btrfs/255
> > @@ -41,12 +41,8 @@ for ((i = 0; i < 20; i++)); do
> > $BTRFS_UTIL_PROG quota enable $SCRATCH_MNT
> > $BTRFS_UTIL_PROG quota disable $SCRATCH_MNT
> > done
> > -kill $balance_pid &> /dev/null
> > -wait
> > -# wait for the balance operation to finish
> > -while ps aux | grep "balance start" | grep -qv grep; do
> > - sleep 1
> > -done
> > +
> > +_btrfs_kill_stress_balance_pid $balance_pid
> >
> > echo "Silence is golden"
> > status=0
>
