diff mbox series

btrfs: properly shutdown subvolume stress worker to avoid umount failures

Message ID 4ee3a7f176ee871345a68aaa48b13e8ca89c2262.1721829411.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: properly shutdown subvolume stress worker to avoid umount failures | expand

Commit Message

Filipe Manana July 24, 2024, 1:58 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

When killing a test that is using the subvolume stress worker, we may end
up in a situation where we end up leaving a subvolume mounted which makes
the shutdown sequence fail. Example when killing a script that keeps
running fstests in a loop:

   FSTYP         -- btrfs
   PLATFORM      -- Linux/x86_64 debian0 6.10.0-rc7-btrfs-next-167+ #1 SMP PREEMPT_DYNAMIC Thu Jul 11 17:54:07 WEST 2024
   MKFS_OPTIONS  -- /dev/sdc
   MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1

   (...)
   btrfs/065 23s ... ^C^C^C
   Iteration 134, errors 1, leaks 0, Wed Jul 24 12:14:33 PM WEST 2024, flakey errors: 0 MKFS_OPTIONS="" MOUNT_OPTIONS=""

   SCRATCH_DEV=/dev/sdc is mounted but not on SCRATCH_MNT=/home/fdmanana/btrfs-tests/scratch_1 - aborting
   Already mounted result:
   /dev/sdc /home/fdmanana/btrfs-tests/scratch_1 /dev/sdc /home/fdmanana/btrfs-tests/dev/065.mnt
   grep: results/btrfs/065.out.bad: No such file or directory
   Error iteration 134, total errors 2, leaks 0
   'results/btrfs/065.full' -> '/home/fdmanana/failures/btrfs_065/134/065.full'

Running 'mount' to see what's going on:

   $ mount
   (...)
   /dev/sdb on /home/fdmanana/btrfs-tests/dev type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)
   /dev/sdc on /home/fdmanana/btrfs-tests/scratch_1 type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)
   /dev/sdc on /home/fdmanana/btrfs-tests/dev/065.mnt type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=2627,subvol=/subvol_3395330)

Then this makes the next attempt to run a test (./check) always fail due
to the extra mount of the subvolume, requiring one to manually umount the
subvolume before running fstests again.

So update _btrfs_kill_stress_subvolume_pid() to also unmount the subvolume.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 common/btrfs    | 2 ++
 tests/btrfs/060 | 9 +++++----
 tests/btrfs/065 | 9 +++++----
 tests/btrfs/066 | 9 +++++----
 tests/btrfs/067 | 9 +++++----
 tests/btrfs/068 | 9 +++++----
 6 files changed, 27 insertions(+), 20 deletions(-)

Comments

Anand Jain July 25, 2024, 3:39 a.m. UTC | #1 
On 7/24/24 21:58, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When killing a test that is using the subvolume stress worker, we may end
> up in a situation where we end up leaving a subvolume mounted which makes
> the shutdown sequence fail. Example when killing a script that keeps
> running fstests in a loop:
> 
>     FSTYP         -- btrfs
>     PLATFORM      -- Linux/x86_64 debian0 6.10.0-rc7-btrfs-next-167+ #1 SMP PREEMPT_DYNAMIC Thu Jul 11 17:54:07 WEST 2024
>     MKFS_OPTIONS  -- /dev/sdc
>     MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> 
>     (...)
>     btrfs/065 23s ... ^C^C^C
>     Iteration 134, errors 1, leaks 0, Wed Jul 24 12:14:33 PM WEST 2024, flakey errors: 0 MKFS_OPTIONS="" MOUNT_OPTIONS=""
> 
>     SCRATCH_DEV=/dev/sdc is mounted but not on SCRATCH_MNT=/home/fdmanana/btrfs-tests/scratch_1 - aborting
>     Already mounted result:
>     /dev/sdc /home/fdmanana/btrfs-tests/scratch_1 /dev/sdc /home/fdmanana/btrfs-tests/dev/065.mnt
>     grep: results/btrfs/065.out.bad: No such file or directory
>     Error iteration 134, total errors 2, leaks 0
>     'results/btrfs/065.full' -> '/home/fdmanana/failures/btrfs_065/134/065.full'
> 
> Running 'mount' to see what's going on:
> 
>     $ mount
>     (...)
>     /dev/sdb on /home/fdmanana/btrfs-tests/dev type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)
>     /dev/sdc on /home/fdmanana/btrfs-tests/scratch_1 type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)
>     /dev/sdc on /home/fdmanana/btrfs-tests/dev/065.mnt type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=2627,subvol=/subvol_3395330)
> 
> Then this makes the next attempt to run a test (./check) always fail due
> to the extra mount of the subvolume, requiring one to manually umount the
> subvolume before running fstests again.
> 
> So update _btrfs_kill_stress_subvolume_pid() to also unmount the subvolume.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thx.
diff mbox series

Patch

diff --git a/common/btrfs b/common/btrfs
index c0be7c08..95a9c8e6 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -362,11 +362,13 @@  _btrfs_kill_stress_subvolume_pid()
 {
 	local subvol_pid=$1
 	local stop_file=$2
+	local subvol_mnt=$3
 
 	touch $stop_file
 	# Ignore if process already died.
 	wait $subvol_pid &> /dev/null
 	rm -f $stop_file
+	$UMOUNT_PROG $subvol_mnt &> /dev/null
 }
 
 # stress btrfs by running scrub in a loop
diff --git a/tests/btrfs/060 b/tests/btrfs/060
index 00f57841..75c10bd2 100755
--- a/tests/btrfs/060
+++ b/tests/btrfs/060
@@ -14,8 +14,9 @@  _cleanup()
 {
 	cd /
 	rm -rf $tmp.*
-	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ]; then
-		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ] && \
+		   [ ! -z "$subvol_mnt" ]; then
+		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	fi
 	if [ ! -z "$balance_pid" ]; then
 		_btrfs_kill_stress_balance_pid $balance_pid
@@ -34,11 +35,11 @@  _require_scratch_dev_pool 4
 _btrfs_get_profile_configs
 
 stop_file=$TEST_DIR/$seq.stop.$$
+subvol_mnt=$TEST_DIR/$seq.mnt
 
 run_test()
 {
 	local mkfs_opts=$1
-	local subvol_mnt=$TEST_DIR/$seq.mnt
 
 	echo "Test $mkfs_opts" >>$seqres.full
 
@@ -69,7 +70,7 @@  run_test()
 	wait $fsstress_pid
 	unset fsstress_pid
 
-	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	unset subvol_pid
 	_btrfs_kill_stress_balance_pid $balance_pid
 	unset balance_pid
diff --git a/tests/btrfs/065 b/tests/btrfs/065
index 5fb635ab..b87c66d6 100755
--- a/tests/btrfs/065
+++ b/tests/btrfs/065
@@ -14,8 +14,9 @@  _cleanup()
 {
 	cd /
 	rm -rf $tmp.*
-	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ]; then
-		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ] && \
+		   [ ! -z "$subvol_mnt" ]; then
+		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	fi
 	if [ ! -z "$replace_pid" ]; then
 		_btrfs_kill_stress_replace_pid $replace_pid
@@ -35,12 +36,12 @@  _require_scratch_dev_pool_equal_size
 _btrfs_get_profile_configs replace
 
 stop_file=$TEST_DIR/$seq.stop.$$
+subvol_mnt=$TEST_DIR/$seq.mnt
 
 run_test()
 {
 	local mkfs_opts=$1
 	local saved_scratch_dev_pool=$SCRATCH_DEV_POOL
-	local subvol_mnt=$TEST_DIR/$seq.mnt
 
 	echo "Test $mkfs_opts" >>$seqres.full
 
@@ -77,7 +78,7 @@  run_test()
 	wait $fsstress_pid
 	unset fsstress_pid
 
-	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	unset subvol_pid
 	_btrfs_kill_stress_replace_pid $replace_pid
 	unset replace_pid
diff --git a/tests/btrfs/066 b/tests/btrfs/066
index 30fa438a..cc7cd9b7 100755
--- a/tests/btrfs/066
+++ b/tests/btrfs/066
@@ -14,8 +14,9 @@  _cleanup()
 {
 	cd /
 	rm -rf $tmp.*
-	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ]; then
-		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ] && \
+		   [ ! -z "$subvol_mnt" ]; then
+		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	fi
 	if [ ! -z "$scrub_pid" ]; then
 		_btrfs_kill_stress_scrub_pid $scrub_pid
@@ -34,11 +35,11 @@  _require_scratch_dev_pool 4
 _btrfs_get_profile_configs
 
 stop_file=$TEST_DIR/$seq.stop.$$
+subvol_mnt=$TEST_DIR/$seq.mnt
 
 run_test()
 {
 	local mkfs_opts=$1
-	local subvol_mnt=$TEST_DIR/$seq.mnt
 
 	echo "Test $mkfs_opts" >>$seqres.full
 
@@ -69,7 +70,7 @@  run_test()
 	wait $fsstress_pid
 	unset fsstress_pid
 
-	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	unset subvol_pid
 	_btrfs_kill_stress_scrub_pid $scrub_pid
 	unset scrub_pid
diff --git a/tests/btrfs/067 b/tests/btrfs/067
index 899b96da..0b473050 100755
--- a/tests/btrfs/067
+++ b/tests/btrfs/067
@@ -14,8 +14,9 @@  _cleanup()
 {
 	cd /
 	rm -rf $tmp.*
-	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ]; then
-		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ] && \
+		   [ ! -z "$subvol_mnt" ]; then
+		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	fi
 	if [ ! -z "$defrag_pid" ]; then
 		_btrfs_kill_stress_defrag_pid $defrag_pid
@@ -34,12 +35,12 @@  _require_scratch_dev_pool 4
 _btrfs_get_profile_configs
 
 stop_file=$TEST_DIR/$seq.stop.$$
+subvol_mnt=$TEST_DIR/$seq.mnt
 
 run_test()
 {
 	local mkfs_opts=$1
 	local with_compress=$2
-	local subvol_mnt=$TEST_DIR/$seq.mnt
 
 	echo "Test $mkfs_opts with $with_compress" >>$seqres.full
 
@@ -70,7 +71,7 @@  run_test()
 	wait $fsstress_pid
 	unset fsstress_pid
 
-	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	unset subvol_pid
 	_btrfs_kill_stress_defrag_pid $defrag_pid
 	unset defrag_pid
diff --git a/tests/btrfs/068 b/tests/btrfs/068
index 48b6cdb0..83e932e8 100755
--- a/tests/btrfs/068
+++ b/tests/btrfs/068
@@ -15,8 +15,9 @@  _cleanup()
 {
 	cd /
 	rm -rf $tmp.*
-	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ]; then
-		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	if [ ! -z "$stop_file" ] && [ ! -z "$subvol_pid" ] && \
+		   [ ! -z "$subvol_mnt" ]; then
+		_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	fi
 	if [ ! -z "$remount_pid" ]; then
 		_btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
@@ -35,11 +36,11 @@  _require_scratch_dev_pool 4
 _btrfs_get_profile_configs
 
 stop_file=$TEST_DIR/$seq.stop.$$
+subvol_mnt=$TEST_DIR/$seq.mnt
 
 run_test()
 {
 	local mkfs_opts=$1
-	local subvol_mnt=$TEST_DIR/$seq.mnt
 
 	echo "Test $mkfs_opts with $with_compress" >>$seqres.full
 
@@ -70,7 +71,7 @@  run_test()
 	wait $fsstress_pid
 	unset fsstress_pid
 
-	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file
+	_btrfs_kill_stress_subvolume_pid $subvol_pid $stop_file $subvol_mnt
 	unset subvol_pid
 	_btrfs_kill_stress_remount_compress_pid $remount_pid $SCRATCH_MNT
 	unset remount_pid