| Message ID | e5f1141cbe307e6057554e1c8fb8cff188ab68f0.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 |
On 3/28/24 01:11, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > 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@suse.com> Reviewed-by: Anand Jain <anand.jain@oracle.com> 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. - 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/ 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
On Thu, Mar 28, 2024 at 8:17 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_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@suse.com> > > Reviewed-by: Anand Jain <anand.jain@oracle.com> > > 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 >
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 + 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 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