* [PATCH v2 1/3] fstests: add missing _require_freeze() to tests
2022-06-21 17:37 [PATCH v2 0/3] aborted fstests may leave frozen fs behind Amir Goldstein
@ 2022-06-21 17:37 ` Amir Goldstein
2022-06-24 15:26 ` Zorro Lang
2022-06-21 17:37 ` [PATCH v2 2/3] fstests: unfreeze fs on cleanup routines Amir Goldstein
2022-06-21 17:37 ` [PATCH v2 3/3] xfs/{422,517}: kill background jobs on test termination Amir Goldstein
2 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2022-06-21 17:37 UTC (permalink / raw)
To: Zorro Lang; +Cc: Darrick J . Wong, Dave Chinner, fstests
And add a few tests that use freeze to the freeze group
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
tests/xfs/119 | 1 +
tests/xfs/318 | 3 ++-
tests/xfs/325 | 3 ++-
tests/xfs/422 | 3 ++-
tests/xfs/438 | 2 +-
tests/xfs/517 | 1 +
6 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/xfs/119 b/tests/xfs/119
index a1180371..b6f96601 100755
--- a/tests/xfs/119
+++ b/tests/xfs/119
@@ -20,6 +20,7 @@ _begin_fstest log v2log auto freeze
_supported_fs xfs
_require_scratch
+_require_freeze
# this may hang
sync
diff --git a/tests/xfs/318 b/tests/xfs/318
index 38c7aa60..be93f9ab 100755
--- a/tests/xfs/318
+++ b/tests/xfs/318
@@ -7,7 +7,7 @@
# Simulate free extent errors with a file write and a file remove.
#
. ./common/preamble
-_begin_fstest auto quick rw
+_begin_fstest auto quick rw freeze
# Override the default cleanup function.
_cleanup()
@@ -26,6 +26,7 @@ _supported_fs xfs
_require_scratch
_require_error_injection
_require_xfs_io_error_injection "rmap_finish_one"
+_require_freeze
blksz=65536
blks=64
diff --git a/tests/xfs/325 b/tests/xfs/325
index 5b26b2b3..c6861fbc 100755
--- a/tests/xfs/325
+++ b/tests/xfs/325
@@ -8,7 +8,7 @@
# Inject an error during extent freeing to test log recovery.
#
. ./common/preamble
-_begin_fstest auto quick clone
+_begin_fstest auto quick clone freeze
# Override the default cleanup function.
_cleanup()
@@ -29,6 +29,7 @@ _require_cp_reflink
_require_scratch_reflink
_require_error_injection
_require_xfs_io_error_injection "free_extent"
+_require_freeze
blksz=65536
blks=30
diff --git a/tests/xfs/422 b/tests/xfs/422
index 175253aa..a83a66df 100755
--- a/tests/xfs/422
+++ b/tests/xfs/422
@@ -9,7 +9,7 @@
# activity, so we can't have userspace wandering in and thawing it.
#
. ./common/preamble
-_begin_fstest dangerous_scrub dangerous_online_repair
+_begin_fstest dangerous_scrub dangerous_online_repair freeze
_register_cleanup "_cleanup" BUS
@@ -24,6 +24,7 @@ _require_xfs_scratch_rmapbt
_require_xfs_io_command "scrub"
_require_xfs_io_error_injection "force_repair"
_require_command "$KILLALL_PROG" killall
+_require_freeze
echo "Format and populate"
_scratch_mkfs > "$seqres.full" 2>&1
diff --git a/tests/xfs/438 b/tests/xfs/438
index c3008b1c..cfe75bd8 100755
--- a/tests/xfs/438
+++ b/tests/xfs/438
@@ -21,7 +21,7 @@
# Fixed by upstream commit 373b058 ("xfs: Properly retry failed dquot
# items in case of error during buffer writeback")
. ./common/preamble
-_begin_fstest auto quick quota
+_begin_fstest auto quick quota freeze
# Override the default cleanup function.
_cleanup()
diff --git a/tests/xfs/517 b/tests/xfs/517
index 88c4f43b..f7f9a8a2 100755
--- a/tests/xfs/517
+++ b/tests/xfs/517
@@ -29,6 +29,7 @@ _supported_fs xfs
_require_xfs_scratch_rmapbt
_require_xfs_io_command "fsmap"
_require_command "$KILLALL_PROG" killall
+_require_freeze
echo "Format and populate"
_scratch_mkfs > "$seqres.full" 2>&1
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] fstests: add missing _require_freeze() to tests
2022-06-21 17:37 ` [PATCH v2 1/3] fstests: add missing _require_freeze() to tests Amir Goldstein
@ 2022-06-24 15:26 ` Zorro Lang
0 siblings, 0 replies; 6+ messages in thread
From: Zorro Lang @ 2022-06-24 15:26 UTC (permalink / raw)
To: fstests
On Tue, Jun 21, 2022 at 08:37:27PM +0300, Amir Goldstein wrote:
> And add a few tests that use freeze to the freeze group
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
Although there were some different points about if "318, 325, 422, 438"
should be in freeze group, I don't mind having them in freeze group personally.
So ...
Reviewed-by: Zorro Lang <zlang@redhat.com>
> tests/xfs/119 | 1 +
> tests/xfs/318 | 3 ++-
> tests/xfs/325 | 3 ++-
> tests/xfs/422 | 3 ++-
> tests/xfs/438 | 2 +-
> tests/xfs/517 | 1 +
> 6 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tests/xfs/119 b/tests/xfs/119
> index a1180371..b6f96601 100755
> --- a/tests/xfs/119
> +++ b/tests/xfs/119
> @@ -20,6 +20,7 @@ _begin_fstest log v2log auto freeze
> _supported_fs xfs
>
> _require_scratch
> +_require_freeze
>
> # this may hang
> sync
> diff --git a/tests/xfs/318 b/tests/xfs/318
> index 38c7aa60..be93f9ab 100755
> --- a/tests/xfs/318
> +++ b/tests/xfs/318
> @@ -7,7 +7,7 @@
> # Simulate free extent errors with a file write and a file remove.
> #
> . ./common/preamble
> -_begin_fstest auto quick rw
> +_begin_fstest auto quick rw freeze
>
> # Override the default cleanup function.
> _cleanup()
> @@ -26,6 +26,7 @@ _supported_fs xfs
> _require_scratch
> _require_error_injection
> _require_xfs_io_error_injection "rmap_finish_one"
> +_require_freeze
>
> blksz=65536
> blks=64
> diff --git a/tests/xfs/325 b/tests/xfs/325
> index 5b26b2b3..c6861fbc 100755
> --- a/tests/xfs/325
> +++ b/tests/xfs/325
> @@ -8,7 +8,7 @@
> # Inject an error during extent freeing to test log recovery.
> #
> . ./common/preamble
> -_begin_fstest auto quick clone
> +_begin_fstest auto quick clone freeze
>
> # Override the default cleanup function.
> _cleanup()
> @@ -29,6 +29,7 @@ _require_cp_reflink
> _require_scratch_reflink
> _require_error_injection
> _require_xfs_io_error_injection "free_extent"
> +_require_freeze
>
> blksz=65536
> blks=30
> diff --git a/tests/xfs/422 b/tests/xfs/422
> index 175253aa..a83a66df 100755
> --- a/tests/xfs/422
> +++ b/tests/xfs/422
> @@ -9,7 +9,7 @@
> # activity, so we can't have userspace wandering in and thawing it.
> #
> . ./common/preamble
> -_begin_fstest dangerous_scrub dangerous_online_repair
> +_begin_fstest dangerous_scrub dangerous_online_repair freeze
>
> _register_cleanup "_cleanup" BUS
>
> @@ -24,6 +24,7 @@ _require_xfs_scratch_rmapbt
> _require_xfs_io_command "scrub"
> _require_xfs_io_error_injection "force_repair"
> _require_command "$KILLALL_PROG" killall
> +_require_freeze
>
> echo "Format and populate"
> _scratch_mkfs > "$seqres.full" 2>&1
> diff --git a/tests/xfs/438 b/tests/xfs/438
> index c3008b1c..cfe75bd8 100755
> --- a/tests/xfs/438
> +++ b/tests/xfs/438
> @@ -21,7 +21,7 @@
> # Fixed by upstream commit 373b058 ("xfs: Properly retry failed dquot
> # items in case of error during buffer writeback")
> . ./common/preamble
> -_begin_fstest auto quick quota
> +_begin_fstest auto quick quota freeze
>
> # Override the default cleanup function.
> _cleanup()
> diff --git a/tests/xfs/517 b/tests/xfs/517
> index 88c4f43b..f7f9a8a2 100755
> --- a/tests/xfs/517
> +++ b/tests/xfs/517
> @@ -29,6 +29,7 @@ _supported_fs xfs
> _require_xfs_scratch_rmapbt
> _require_xfs_io_command "fsmap"
> _require_command "$KILLALL_PROG" killall
> +_require_freeze
>
> echo "Format and populate"
> _scratch_mkfs > "$seqres.full" 2>&1
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] fstests: unfreeze fs on cleanup routines
2022-06-21 17:37 [PATCH v2 0/3] aborted fstests may leave frozen fs behind Amir Goldstein
2022-06-21 17:37 ` [PATCH v2 1/3] fstests: add missing _require_freeze() to tests Amir Goldstein
@ 2022-06-21 17:37 ` Amir Goldstein
2022-06-24 16:06 ` Zorro Lang
2022-06-21 17:37 ` [PATCH v2 3/3] xfs/{422,517}: kill background jobs on test termination Amir Goldstein
2 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2022-06-21 17:37 UTC (permalink / raw)
To: Zorro Lang; +Cc: Darrick J . Wong, Dave Chinner, fstests
Many of tests that freeze fs do not make sure that fs is unfrozen on
test termination.
Some tests also need to kill and wait for writer processes after
unfreezing the fs.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
tests/generic/068 | 14 ++++++++++----
tests/generic/085 | 3 +++
tests/generic/280 | 13 ++++++++++++-
tests/generic/390 | 8 +++++++-
tests/generic/459 | 2 ++
tests/generic/491 | 9 +++++++++
tests/xfs/011 | 3 ++-
tests/xfs/119 | 9 +++++++++
tests/xfs/297 | 11 +++++++++++
tests/xfs/318 | 3 ++-
tests/xfs/325 | 3 ++-
tests/xfs/438 | 2 ++
12 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/tests/generic/068 b/tests/generic/068
index 56262cd7..eeddf6d1 100755
--- a/tests/generic/068
+++ b/tests/generic/068
@@ -17,9 +17,12 @@ ITERATIONS=10
# Override the default cleanup function.
_cleanup()
{
- cd /
-
- trap 0 1 2 3 15
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ [ -n "$pid" ] && kill -9 $pid 2>/dev/null
+ wait $pid
+ cd /
+ rm -f $tmp.*
}
# Import common functions.
@@ -60,6 +63,7 @@ touch $tmp.running
rm -r $STRESS_DIR/*
rmdir $STRESS_DIR
} &
+pid=$!
# start fstest -m loop in a background block; this gets us mmap coverage
{
@@ -75,6 +79,7 @@ touch $tmp.running
rm -rf $FSTEST_DIR/*
rmdir $FSTEST_DIR
} &
+pid="$pid $!"
i=0
let ITERATIONS=$ITERATIONS-1
@@ -103,6 +108,7 @@ done
rm $tmp.running
# wait for fsstresses to finish
-wait
+wait $pid
+unset pid
exit 1
diff --git a/tests/generic/085 b/tests/generic/085
index 20cf875a..786d8e6f 100755
--- a/tests/generic/085
+++ b/tests/generic/085
@@ -25,6 +25,8 @@ cleanup_dmdev()
{
# in case it's still suspended and/or mounted
$DMSETUP_PROG resume $lvdev >/dev/null 2>&1
+ [ -n "$pid" ] && kill -9 $pid 2>/dev/null
+ wait $pid
$UMOUNT_PROG $lvdev >/dev/null 2>&1
_dmsetup_remove $node
}
@@ -75,6 +77,7 @@ done &
pid="$pid $!"
wait $pid
+unset pid
status=0
exit
diff --git a/tests/generic/280 b/tests/generic/280
index 07144555..8e1ae4d2 100755
--- a/tests/generic/280
+++ b/tests/generic/280
@@ -10,6 +10,17 @@
. ./common/preamble
_begin_fstest auto quota freeze
+# Override the default cleanup function.
+_cleanup()
+{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ [ -n "$pid" ] && kill -9 $pid 2>/dev/null
+ wait $pid
+ cd /
+ rm -f $tmp.*
+}
+
# Import common functions.
. ./common/filter
. ./common/quota
@@ -34,7 +45,7 @@ pid=$!
sleep 1
xfs_freeze -u $SCRATCH_MNT
wait $pid
-_scratch_unmount
+unset pid
# Failure comes in the form of a deadlock.
diff --git a/tests/generic/390 b/tests/generic/390
index 20c66e22..e8f6a5dc 100755
--- a/tests/generic/390
+++ b/tests/generic/390
@@ -14,8 +14,12 @@ _begin_fstest auto freeze stress
_cleanup()
{
cd /
- # Make sure $SCRATCH_MNT is unfreezed
+ # Kill freeze loops and make sure $SCRATCH_MNT is unfreezed
+ [ -n "$freeze_pids" ] && kill -9 $freeze_pids 2>/dev/null
+ wait $freeze_pids
xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ [ -n "$fsstress_pid" ] && kill -9 $fsstress_pid 2>/dev/null
+ wait $fsstress_pid
rm -f $tmp.*
}
@@ -62,7 +66,9 @@ done
wait $fsstress_pid
result=$?
+unset fsstress_pid
wait $freeze_pids
+unset freeze_pids
# Exit with fsstress return value
status=$result
diff --git a/tests/generic/459 b/tests/generic/459
index 57d58e55..7be39089 100755
--- a/tests/generic/459
+++ b/tests/generic/459
@@ -24,6 +24,8 @@ _begin_fstest auto freeze thin
# Override the default cleanup function.
_cleanup()
{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
cd /
rm -f $tmp.*
$UMOUNT_PROG $SCRATCH_MNT >>$seqres.full 2>&1
diff --git a/tests/generic/491 b/tests/generic/491
index e6e57dcd..797b08d5 100755
--- a/tests/generic/491
+++ b/tests/generic/491
@@ -12,6 +12,15 @@
. ./common/preamble
_begin_fstest auto quick freeze mount
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ rm -f $tmp.*
+}
+
# Import common functions.
. ./common/filter
diff --git a/tests/xfs/011 b/tests/xfs/011
index d6e9099e..ed44d074 100755
--- a/tests/xfs/011
+++ b/tests/xfs/011
@@ -16,10 +16,11 @@ _begin_fstest auto freeze log metadata quick
# Override the default cleanup function.
_cleanup()
{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
$KILLALL_PROG -9 fsstress 2>/dev/null
wait
cd /
- _scratch_unmount 2>/dev/null
rm -f $tmp.*
}
diff --git a/tests/xfs/119 b/tests/xfs/119
index b6f96601..5ffbce25 100755
--- a/tests/xfs/119
+++ b/tests/xfs/119
@@ -11,6 +11,15 @@
. ./common/preamble
_begin_fstest log v2log auto freeze
+# Override the default cleanup function.
+_cleanup()
+{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ cd /
+ rm -f $tmp.*
+}
+
# Import common functions.
. ./common/filter
diff --git a/tests/xfs/297 b/tests/xfs/297
index ca482e06..07f84c25 100755
--- a/tests/xfs/297
+++ b/tests/xfs/297
@@ -11,6 +11,17 @@
. ./common/preamble
_begin_fstest auto freeze
+# Override the default cleanup function.
+_cleanup()
+{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
+ $KILLALL_PROG -q -9 $FSSTRESS_PROG
+ wait
+ cd /
+ rm -f $tmp.*
+}
+
# Import common functions.
. ./common/filter
diff --git a/tests/xfs/318 b/tests/xfs/318
index be93f9ab..5798f9a3 100755
--- a/tests/xfs/318
+++ b/tests/xfs/318
@@ -12,8 +12,9 @@ _begin_fstest auto quick rw freeze
# Override the default cleanup function.
_cleanup()
{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
cd /
- _scratch_unmount > /dev/null 2>&1
rm -rf $tmp.*
}
diff --git a/tests/xfs/325 b/tests/xfs/325
index c6861fbc..43fb09a6 100755
--- a/tests/xfs/325
+++ b/tests/xfs/325
@@ -13,8 +13,9 @@ _begin_fstest auto quick clone freeze
# Override the default cleanup function.
_cleanup()
{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
cd /
- _scratch_unmount > /dev/null 2>&1
rm -rf $tmp.*
}
diff --git a/tests/xfs/438 b/tests/xfs/438
index cfe75bd8..0425c5b1 100755
--- a/tests/xfs/438
+++ b/tests/xfs/438
@@ -26,6 +26,8 @@ _begin_fstest auto quick quota freeze
# Override the default cleanup function.
_cleanup()
{
+ # Make sure $SCRATCH_MNT is unfreezed
+ xfs_freeze -u $SCRATCH_MNT 2>/dev/null
[ -z "${interval}" ] || \
sysctl -w fs.xfs.xfssyncd_centisecs=${interval} >/dev/null 2>&1
cd /
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] fstests: unfreeze fs on cleanup routines
2022-06-21 17:37 ` [PATCH v2 2/3] fstests: unfreeze fs on cleanup routines Amir Goldstein
@ 2022-06-24 16:06 ` Zorro Lang
0 siblings, 0 replies; 6+ messages in thread
From: Zorro Lang @ 2022-06-24 16:06 UTC (permalink / raw)
To: fstests
On Tue, Jun 21, 2022 at 08:37:28PM +0300, Amir Goldstein wrote:
> Many of tests that freeze fs do not make sure that fs is unfrozen on
> test termination.
>
> Some tests also need to kill and wait for writer processes after
> unfreezing the fs.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
The changes look good, and I didn't find regression issue from it for now.
Please tell me your cencern if anyone has, or I'll merge it.
Reviewed-by: Zorro Lang <zlang@redhat.com>
> tests/generic/068 | 14 ++++++++++----
> tests/generic/085 | 3 +++
> tests/generic/280 | 13 ++++++++++++-
> tests/generic/390 | 8 +++++++-
> tests/generic/459 | 2 ++
> tests/generic/491 | 9 +++++++++
> tests/xfs/011 | 3 ++-
> tests/xfs/119 | 9 +++++++++
> tests/xfs/297 | 11 +++++++++++
> tests/xfs/318 | 3 ++-
> tests/xfs/325 | 3 ++-
> tests/xfs/438 | 2 ++
> 12 files changed, 71 insertions(+), 9 deletions(-)
>
> diff --git a/tests/generic/068 b/tests/generic/068
> index 56262cd7..eeddf6d1 100755
> --- a/tests/generic/068
> +++ b/tests/generic/068
> @@ -17,9 +17,12 @@ ITERATIONS=10
> # Override the default cleanup function.
> _cleanup()
> {
> - cd /
> -
> - trap 0 1 2 3 15
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + [ -n "$pid" ] && kill -9 $pid 2>/dev/null
> + wait $pid
> + cd /
> + rm -f $tmp.*
> }
>
> # Import common functions.
> @@ -60,6 +63,7 @@ touch $tmp.running
> rm -r $STRESS_DIR/*
> rmdir $STRESS_DIR
> } &
> +pid=$!
>
> # start fstest -m loop in a background block; this gets us mmap coverage
> {
> @@ -75,6 +79,7 @@ touch $tmp.running
> rm -rf $FSTEST_DIR/*
> rmdir $FSTEST_DIR
> } &
> +pid="$pid $!"
>
> i=0
> let ITERATIONS=$ITERATIONS-1
> @@ -103,6 +108,7 @@ done
> rm $tmp.running
>
> # wait for fsstresses to finish
> -wait
> +wait $pid
> +unset pid
>
> exit 1
> diff --git a/tests/generic/085 b/tests/generic/085
> index 20cf875a..786d8e6f 100755
> --- a/tests/generic/085
> +++ b/tests/generic/085
> @@ -25,6 +25,8 @@ cleanup_dmdev()
> {
> # in case it's still suspended and/or mounted
> $DMSETUP_PROG resume $lvdev >/dev/null 2>&1
> + [ -n "$pid" ] && kill -9 $pid 2>/dev/null
> + wait $pid
> $UMOUNT_PROG $lvdev >/dev/null 2>&1
> _dmsetup_remove $node
> }
> @@ -75,6 +77,7 @@ done &
> pid="$pid $!"
>
> wait $pid
> +unset pid
>
> status=0
> exit
> diff --git a/tests/generic/280 b/tests/generic/280
> index 07144555..8e1ae4d2 100755
> --- a/tests/generic/280
> +++ b/tests/generic/280
> @@ -10,6 +10,17 @@
> . ./common/preamble
> _begin_fstest auto quota freeze
>
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + [ -n "$pid" ] && kill -9 $pid 2>/dev/null
> + wait $pid
> + cd /
> + rm -f $tmp.*
> +}
> +
> # Import common functions.
> . ./common/filter
> . ./common/quota
> @@ -34,7 +45,7 @@ pid=$!
> sleep 1
> xfs_freeze -u $SCRATCH_MNT
> wait $pid
> -_scratch_unmount
> +unset pid
>
> # Failure comes in the form of a deadlock.
>
> diff --git a/tests/generic/390 b/tests/generic/390
> index 20c66e22..e8f6a5dc 100755
> --- a/tests/generic/390
> +++ b/tests/generic/390
> @@ -14,8 +14,12 @@ _begin_fstest auto freeze stress
> _cleanup()
> {
> cd /
> - # Make sure $SCRATCH_MNT is unfreezed
> + # Kill freeze loops and make sure $SCRATCH_MNT is unfreezed
> + [ -n "$freeze_pids" ] && kill -9 $freeze_pids 2>/dev/null
> + wait $freeze_pids
> xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + [ -n "$fsstress_pid" ] && kill -9 $fsstress_pid 2>/dev/null
> + wait $fsstress_pid
> rm -f $tmp.*
> }
>
> @@ -62,7 +66,9 @@ done
>
> wait $fsstress_pid
> result=$?
> +unset fsstress_pid
> wait $freeze_pids
> +unset freeze_pids
>
> # Exit with fsstress return value
> status=$result
> diff --git a/tests/generic/459 b/tests/generic/459
> index 57d58e55..7be39089 100755
> --- a/tests/generic/459
> +++ b/tests/generic/459
> @@ -24,6 +24,8 @@ _begin_fstest auto freeze thin
> # Override the default cleanup function.
> _cleanup()
> {
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> cd /
> rm -f $tmp.*
> $UMOUNT_PROG $SCRATCH_MNT >>$seqres.full 2>&1
> diff --git a/tests/generic/491 b/tests/generic/491
> index e6e57dcd..797b08d5 100755
> --- a/tests/generic/491
> +++ b/tests/generic/491
> @@ -12,6 +12,15 @@
> . ./common/preamble
> _begin_fstest auto quick freeze mount
>
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + cd /
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + rm -f $tmp.*
> +}
> +
> # Import common functions.
> . ./common/filter
>
> diff --git a/tests/xfs/011 b/tests/xfs/011
> index d6e9099e..ed44d074 100755
> --- a/tests/xfs/011
> +++ b/tests/xfs/011
> @@ -16,10 +16,11 @@ _begin_fstest auto freeze log metadata quick
> # Override the default cleanup function.
> _cleanup()
> {
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> $KILLALL_PROG -9 fsstress 2>/dev/null
> wait
> cd /
> - _scratch_unmount 2>/dev/null
> rm -f $tmp.*
> }
>
> diff --git a/tests/xfs/119 b/tests/xfs/119
> index b6f96601..5ffbce25 100755
> --- a/tests/xfs/119
> +++ b/tests/xfs/119
> @@ -11,6 +11,15 @@
> . ./common/preamble
> _begin_fstest log v2log auto freeze
>
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + cd /
> + rm -f $tmp.*
> +}
> +
> # Import common functions.
> . ./common/filter
>
> diff --git a/tests/xfs/297 b/tests/xfs/297
> index ca482e06..07f84c25 100755
> --- a/tests/xfs/297
> +++ b/tests/xfs/297
> @@ -11,6 +11,17 @@
> . ./common/preamble
> _begin_fstest auto freeze
>
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> + $KILLALL_PROG -q -9 $FSSTRESS_PROG
> + wait
> + cd /
> + rm -f $tmp.*
> +}
> +
> # Import common functions.
> . ./common/filter
>
> diff --git a/tests/xfs/318 b/tests/xfs/318
> index be93f9ab..5798f9a3 100755
> --- a/tests/xfs/318
> +++ b/tests/xfs/318
> @@ -12,8 +12,9 @@ _begin_fstest auto quick rw freeze
> # Override the default cleanup function.
> _cleanup()
> {
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> cd /
> - _scratch_unmount > /dev/null 2>&1
> rm -rf $tmp.*
> }
>
> diff --git a/tests/xfs/325 b/tests/xfs/325
> index c6861fbc..43fb09a6 100755
> --- a/tests/xfs/325
> +++ b/tests/xfs/325
> @@ -13,8 +13,9 @@ _begin_fstest auto quick clone freeze
> # Override the default cleanup function.
> _cleanup()
> {
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> cd /
> - _scratch_unmount > /dev/null 2>&1
> rm -rf $tmp.*
> }
>
> diff --git a/tests/xfs/438 b/tests/xfs/438
> index cfe75bd8..0425c5b1 100755
> --- a/tests/xfs/438
> +++ b/tests/xfs/438
> @@ -26,6 +26,8 @@ _begin_fstest auto quick quota freeze
> # Override the default cleanup function.
> _cleanup()
> {
> + # Make sure $SCRATCH_MNT is unfreezed
> + xfs_freeze -u $SCRATCH_MNT 2>/dev/null
> [ -z "${interval}" ] || \
> sysctl -w fs.xfs.xfssyncd_centisecs=${interval} >/dev/null 2>&1
> cd /
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] xfs/{422,517}: kill background jobs on test termination
2022-06-21 17:37 [PATCH v2 0/3] aborted fstests may leave frozen fs behind Amir Goldstein
2022-06-21 17:37 ` [PATCH v2 1/3] fstests: add missing _require_freeze() to tests Amir Goldstein
2022-06-21 17:37 ` [PATCH v2 2/3] fstests: unfreeze fs on cleanup routines Amir Goldstein
@ 2022-06-21 17:37 ` Amir Goldstein
2 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2022-06-21 17:37 UTC (permalink / raw)
To: Zorro Lang; +Cc: Darrick J . Wong, Dave Chinner, fstests
Those tests failed to cleanup background jobs properly after test
is interrupted and even sometimes when it completed succefully.
xfs/517 would sometime fails randomally with this false positive error:
QA output created by 517
Format and populate
Concurrent fsmap and freeze
+Terminated
Test done
The tests have several background sub-shells that spawn short lived
programs in a loop. By killing the spawned programs using killall,
killall could find no process to kill and the sub-shell loop could still
spawn another process that is not going to be killed and in the worst
case, the freeze_loop() could spawn the xfs_io "freeze" command after
test has thawn the fs before exit, which leaves the fs frozen after the
test.
The "Terminated" output is emitted by the sub-shell when killing the
programs that it has spawned when the loop did not finish before test
timeout. By killing the sub-shell and not the spawned programs, we
avoid the false positive "Terminated" error.
Use a helper to perform this cleanup dance:
First kill and wait the freeze_loop so it won't try to freeze fs again
Then make sure fs is not frozen.
Then kill and wait for the rest of the sub-shells, because
if fs is frozen a killed writer process will never exit.
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
tests/xfs/422 | 32 ++++++++++++++++++++++++++++++--
tests/xfs/517 | 26 +++++++++++++++++++++++---
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/tests/xfs/422 b/tests/xfs/422
index a83a66df..fdbb8bf1 100755
--- a/tests/xfs/422
+++ b/tests/xfs/422
@@ -13,6 +13,32 @@ _begin_fstest dangerous_scrub dangerous_online_repair freeze
_register_cleanup "_cleanup" BUS
+# First kill and wait the freeze loop so it won't try to freeze fs again
+# Then make sure fs is not frozen
+# Then kill and wait for the rest of the workers
+# Because if fs is frozen a killed writer will never exit
+kill_loops() {
+ local sig=$1
+
+ [ -n "$freeze_pid" ] && kill $sig $freeze_pid
+ wait $freeze_pid
+ unset freeze_pid
+ $XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT
+ [ -n "$stress_pid" ] && kill $sig $stress_pid
+ [ -n "$repair_pid" ] && kill $sig $repair_pid
+ wait
+ unset stress_pid
+ unset repair_pid
+}
+
+# Override the default cleanup function.
+_cleanup()
+{
+ kill_loops -9 > /dev/null 2>&1
+ cd /
+ rm -rf $tmp.*
+}
+
# Import common functions.
. ./common/filter
. ./common/fuzzy
@@ -78,8 +104,11 @@ end=$((start + (30 * TIME_FACTOR) ))
echo "Loop started at $(date --date="@${start}"), ending at $(date --date="@${end}")" >> $seqres.full
stress_loop $end &
+stress_pid=$!
freeze_loop $end &
+freeze_pid=$!
repair_loop $end &
+repair_pid=$!
# Wait until 2 seconds after the loops should have finished...
while [ "$(date +%s)" -lt $((end + 2)) ]; do
@@ -87,8 +116,7 @@ while [ "$(date +%s)" -lt $((end + 2)) ]; do
done
# ...and clean up after the loops in case they didn't do it themselves.
-$KILLALL_PROG -TERM xfs_io fsstress >> $seqres.full 2>&1
-$XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT >> $seqres.full 2>&1
+kill_loops >> $seqres.full 2>&1
echo "Loop finished at $(date)" >> $seqres.full
echo "Test done"
diff --git a/tests/xfs/517 b/tests/xfs/517
index f7f9a8a2..6877af13 100755
--- a/tests/xfs/517
+++ b/tests/xfs/517
@@ -11,11 +11,29 @@ _begin_fstest auto quick fsmap freeze
_register_cleanup "_cleanup" BUS
+# First kill and wait the freeze loop so it won't try to freeze fs again
+# Then make sure fs is not frozen
+# Then kill and wait for the rest of the workers
+# Because if fs is frozen a killed writer will never exit
+kill_loops() {
+ local sig=$1
+
+ [ -n "$freeze_pid" ] && kill $sig $freeze_pid
+ wait $freeze_pid
+ unset freeze_pid
+ $XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT
+ [ -n "$stress_pid" ] && kill $sig $stress_pid
+ [ -n "$fsmap_pid" ] && kill $sig $fsmap_pid
+ wait
+ unset stress_pid
+ unset fsmap_pid
+}
+
# Override the default cleanup function.
_cleanup()
{
+ kill_loops -9 > /dev/null 2>&1
cd /
- $XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT > /dev/null 2>&1
rm -rf $tmp.*
}
@@ -83,8 +101,11 @@ end=$((start + (30 * TIME_FACTOR) ))
echo "Loop started at $(date --date="@${start}"), ending at $(date --date="@${end}")" >> $seqres.full
stress_loop $end &
+stress_pid=$!
freeze_loop $end &
+freeze_pid=$!
fsmap_loop $end &
+fsmap_pid=$!
# Wait until 2 seconds after the loops should have finished...
while [ "$(date +%s)" -lt $((end + 2)) ]; do
@@ -92,8 +113,7 @@ while [ "$(date +%s)" -lt $((end + 2)) ]; do
done
# ...and clean up after the loops in case they didn't do it themselves.
-$KILLALL_PROG -TERM xfs_io fsstress >> $seqres.full 2>&1
-$XFS_IO_PROG -x -c 'thaw' $SCRATCH_MNT >> $seqres.full 2>&1
+kill_loops >> $seqres.full 2>&1
echo "Loop finished at $(date)" >> $seqres.full
echo "Test done"
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread