[RFC PATCH v2 1/4] selftests: zram: track owned devices and report cleanup failures

[Matthias Goergens <matthias.goergens@gmail.com>]

hot_add returns the lowest available device ID, which need not equal the
number of existing devices. With devices 0 and 2 present, the tests
allocate device 1 but configure and clean up device 2. Record the returned
IDs and successful swap and mount activations instead of assuming ranges.

Stop zram01 before filling after mount failure. Remove only directories
created for successful mounts, and preserve devices when swapoff or
unmount fails, avoiding deletion through a mount cleanup could not release.

Wait for udev probes before reset and removal: a worker holding the device
open can cause EBUSY. Bound these waits and treat timeouts as warnings,
since unrelated events can delay the global queue. Propagate actual
teardown errors while continuing cleanup of other devices. Preserve both
test results in the registered runner so zram02 cannot hide zram01 failure.

Propagate swap activation and swapoff errors to the test result.

Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
 tools/testing/selftests/zram/zram.sh     |   9 ++
 tools/testing/selftests/zram/zram01.sh   |   9 +-
 tools/testing/selftests/zram/zram02.sh   |   9 +-
 tools/testing/selftests/zram/zram_lib.sh | 161 ++++++++++++++++-------
 4 files changed, 136 insertions(+), 52 deletions(-)

diff --git a/tools/testing/selftests/zram/zram.sh b/tools/testing/selftests/zram/zram.sh
index b0b91d9b0dc2..b597cf186835 100755
--- a/tools/testing/selftests/zram/zram.sh
+++ b/tools/testing/selftests/zram/zram.sh
@@ -5,12 +5,21 @@ TCID="zram.sh"
 . ./zram_lib.sh
 
 run_zram () {
+local ret status
+
 echo "--------------------"
 echo "running zram tests"
 echo "--------------------"
 ./zram01.sh
+ret=$?
 echo ""
 ./zram02.sh
+status=$?
+if [ "$status" -ne 0 ] &&
+   { [ "$ret" -eq 0 ] || [ "$ret" -eq "$ksft_skip" ]; }; then
+	ret=$status
+fi
+return "$ret"
 }
 
 check_prereqs
diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index 8f4affe34f3e..185f68471b7b 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -33,7 +33,7 @@ zram_algs="lzo"
 
 zram_fill_fs()
 {
-	for i in $(seq $dev_start $dev_end); do
+	for i in $dev_ids; do
 		echo "fill zram$i..."
 		local b=0
 		while [ true ]; do
@@ -57,19 +57,20 @@ zram_fill_fs()
 }
 
 check_prereqs
-zram_load
+zram_load || { zram_cleanup; exit 1; }
 zram_max_streams
 zram_compress_alg
 zram_set_disksizes
 zram_set_memlimit
 zram_makefs
-zram_mount
+zram_mount || { zram_cleanup; exit 1; }
 
 zram_fill_fs
-zram_cleanup
+zram_cleanup || ERR_CODE=1
 
 if [ $ERR_CODE -ne 0 ]; then
 	echo "$TCID : [FAIL]"
+	exit 1
 else
 	echo "$TCID : [PASS]"
 fi
diff --git a/tools/testing/selftests/zram/zram02.sh b/tools/testing/selftests/zram/zram02.sh
index 2418b0c4ed13..2420dda987d4 100755
--- a/tools/testing/selftests/zram/zram02.sh
+++ b/tools/testing/selftests/zram/zram02.sh
@@ -29,16 +29,17 @@ zram_sizes="1048576" # 1M
 zram_mem_limits="1M"
 
 check_prereqs
-zram_load
+zram_load || { zram_cleanup; exit 1; }
 zram_max_streams
 zram_set_disksizes
 zram_set_memlimit
-zram_makeswap
-zram_swapoff
-zram_cleanup
+zram_makeswap || ERR_CODE=1
+zram_swapoff || ERR_CODE=1
+zram_cleanup || ERR_CODE=1
 
 if [ $ERR_CODE -ne 0 ]; then
 	echo "$TCID : [FAIL]"
+	exit 1
 else
 	echo "$TCID : [PASS]"
 fi
diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
index 0d44d83888f9..4b134f70726d 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -5,10 +5,10 @@
 # Author: Alexey Kodanev <alexey.kodanev@oracle.com>
 # Modified: Naresh Kamboju <naresh.kamboju@linaro.org>
 
-dev_makeswap=-1
-dev_mounted=-1
-dev_start=0
-dev_end=-1
+# IDs returned by hot_add, in allocation order; old kernels use 0..dev_num-1.
+dev_ids=""
+dev_swap_ids=""
+dev_mount_ids=""
 module_load=-1
 sys_control=-1
 # Kselftest framework requirement - SKIP code is 4.
@@ -44,32 +44,72 @@ kernel_gte()
 	return 1
 }
 
+zram_wait_for_udev()
+{
+	# Probing triggered by device changes can still hold the device open.
+	# The queue is global; only the subsequent teardown can establish failure.
+	if command -v udevadm >/dev/null 2>&1; then
+		udevadm settle --timeout=5 ||
+			echo "udev queue did not settle; attempting cleanup" >&2
+	fi
+	return 0
+}
+
 zram_cleanup()
 {
 	echo "zram cleanup"
 	local i=
-	for i in $(seq $dev_start $dev_makeswap); do
-		swapoff /dev/zram$i
+	local ret=0
+	local busy_ids=""
+	for i in $dev_ids; do
+		case " $dev_swap_ids " in
+			*" $i "*) ;;
+			*)
+				# A signal can arrive after a helper activates swap but
+				# before its caller records the ID.
+				grep -q "^/dev/zram${i}[[:space:]]" /proc/swaps ||
+					continue
+				;;
+		esac
+		if ! swapoff /dev/zram$i; then
+			ret=1
+			busy_ids="$busy_ids $i"
+		fi
 	done
 
-	for i in $(seq $dev_start $dev_mounted); do
-		umount /dev/zram$i
+	for i in $dev_mount_ids; do
+		if ! umount /dev/zram$i; then
+			ret=1
+			busy_ids="$busy_ids $i"
+		fi
 	done
 
-	for i in $(seq $dev_start $dev_end); do
-		echo 1 > /sys/block/zram${i}/reset
-		rm -rf zram$i
+	zram_wait_for_udev
+	for i in $dev_ids; do
+		case " $busy_ids " in
+			*" $i "*) continue ;;
+		esac
+		echo 1 > /sys/block/zram${i}/reset || ret=1
+		case " $dev_mount_ids " in
+			*" $i "*) rmdir "zram$i" || ret=1 ;;
+		esac
 	done
+	# Reset emits another device-change event before removal.
+	zram_wait_for_udev
 
 	if [ $sys_control -eq 1 ]; then
-		for i in $(seq $dev_start $dev_end); do
-			echo $i > /sys/class/zram-control/hot_remove
+		for i in $dev_ids; do
+			case " $busy_ids " in
+				*" $i "*) continue ;;
+			esac
+			echo $i > /sys/class/zram-control/hot_remove || ret=1
 		done
 	fi
 
 	if [ $module_load -eq 1 ]; then
-		rmmod zram > /dev/null 2>&1
+		rmmod zram || ret=1
 	fi
+	return "$ret"
 }
 
 zram_load()
@@ -80,15 +120,23 @@ zram_load()
 	if [ -d "/sys/class/zram-control" ]; then
 		echo "zram modules already loaded, kernel supports" \
 			"zram-control interface"
-		dev_start=$(ls /dev/zram* | wc -w)
-		dev_end=$(($dev_start + $dev_num - 1))
 		sys_control=1
 
-		for i in $(seq $dev_start $dev_end); do
-			cat /sys/class/zram-control/hot_add > /dev/null
+		for i in $(seq 1 $dev_num); do
+			if ! id=$(cat /sys/class/zram-control/hot_add); then
+				echo "FAIL zram hot_add failed" >&2
+				return 1
+			fi
+			case "$id" in
+				''|*[!0-9]*)
+					echo "FAIL invalid zram hot_add ID: $id" >&2
+					return 1
+					;;
+			esac
+			dev_ids="$dev_ids $id"
 		done
 
-		echo "all zram devices (/dev/zram$dev_start~$dev_end" \
+		echo "all zram devices ($dev_ids)" \
 			"successfully created"
 		return 0
 	fi
@@ -112,8 +160,11 @@ zram_load()
 	fi
 
 	module_load=1
-	dev_end=$(($dev_num - 1))
-	echo "all zram devices (/dev/zram0~$dev_end) successfully created"
+	local last=$(($dev_num - 1))
+	for i in $(seq 0 $last); do
+		dev_ids="$dev_ids $i"
+	done
+	echo "all zram devices (/dev/zram0~$last) successfully created"
 }
 
 zram_max_streams()
@@ -127,8 +178,10 @@ zram_max_streams()
 		return 0
 	fi
 
-	local i=$dev_start
+	set -- $dev_ids
 	for max_s in $zram_max_streams; do
+		local i=$1
+		shift
 		local sys_path="/sys/block/zram${i}/max_comp_streams"
 		echo $max_s > $sys_path || \
 			echo "FAIL failed to set '$max_s' to $sys_path"
@@ -138,7 +191,6 @@ zram_max_streams()
 		[ "$max_s" -ne "$max_streams" ] && \
 			echo "FAIL can't set max_streams '$max_s', get $max_stream"
 
-		i=$(($i + 1))
 		echo "$sys_path = '$max_streams'"
 	done
 
@@ -149,15 +201,17 @@ zram_compress_alg()
 {
 	echo "test that we can set compression algorithm"
 
-	local i=$dev_start
+	set -- $dev_ids
+	local i=$1
 	local algs=$(cat /sys/block/zram${i}/comp_algorithm)
 	echo "supported algs: $algs"
 
 	for alg in $zram_algs; do
+		local i=$1
+		shift
 		local sys_path="/sys/block/zram${i}/comp_algorithm"
 		echo "$alg" >	$sys_path || \
 			echo "FAIL can't set '$alg' to $sys_path"
-		i=$(($i + 1))
 		echo "$sys_path = '$alg'"
 	done
 
@@ -167,13 +221,14 @@ zram_compress_alg()
 zram_set_disksizes()
 {
 	echo "set disk size to zram device(s)"
-	local i=$dev_start
+	set -- $dev_ids
 	for ds in $zram_sizes; do
+		local i=$1
+		shift
 		local sys_path="/sys/block/zram${i}/disksize"
 		echo "$ds" >	$sys_path || \
 			echo "FAIL can't set '$ds' to $sys_path"
 
-		i=$(($i + 1))
 		echo "$sys_path = '$ds'"
 	done
 
@@ -184,13 +239,14 @@ zram_set_memlimit()
 {
 	echo "set memory limit to zram device(s)"
 
-	local i=$dev_start
+	set -- $dev_ids
 	for ds in $zram_mem_limits; do
+		local i=$1
+		shift
 		local sys_path="/sys/block/zram${i}/mem_limit"
 		echo "$ds" >	$sys_path || \
 			echo "FAIL can't set '$ds' to $sys_path"
 
-		i=$(($i + 1))
 		echo "$sys_path = '$ds'"
 	done
 
@@ -200,46 +256,59 @@ zram_set_memlimit()
 zram_makeswap()
 {
 	echo "make swap with zram device(s)"
-	local i=$dev_start
-	for i in $(seq $dev_start $dev_end); do
+	local i
+	local ret=0
+	for i in $dev_ids; do
 		mkswap /dev/zram$i > err.log 2>&1
 		if [ $? -ne 0 ]; then
 			cat err.log
-			echo "FAIL mkswap /dev/zram$1 failed"
+			echo "FAIL mkswap /dev/zram$i failed"
+			ret=1
+			continue
 		fi
 
 		swapon /dev/zram$i > err.log 2>&1
 		if [ $? -ne 0 ]; then
 			cat err.log
-			echo "FAIL swapon /dev/zram$1 failed"
+			echo "FAIL swapon /dev/zram$i failed"
+			ret=1
+			continue
 		fi
 
 		echo "done with /dev/zram$i"
-		dev_makeswap=$i
+		dev_swap_ids="$dev_swap_ids $i"
 	done
 
-	echo "zram making zram mkswap and swapon: OK"
+	[ "$ret" -eq 0 ] && echo "zram making zram mkswap and swapon: OK"
+	return "$ret"
 }
 
 zram_swapoff()
 {
 	local i=
-	for i in $(seq $dev_start $dev_end); do
+	local failed_ids=""
+	local ret=0
+	for i in $dev_swap_ids; do
 		swapoff /dev/zram$i > err.log 2>&1
 		if [ $? -ne 0 ]; then
 			cat err.log
 			echo "FAIL swapoff /dev/zram$i failed"
+			ret=1
+			failed_ids="$failed_ids $i"
 		fi
 	done
-	dev_makeswap=-1
+	dev_swap_ids=$failed_ids
 
-	echo "zram swapoff: OK"
+	[ "$ret" -eq 0 ] && echo "zram swapoff: OK"
+	return "$ret"
 }
 
 zram_makefs()
 {
-	local i=$dev_start
+	set -- $dev_ids
 	for fs in $zram_filesystems; do
+		local i=$1
+		shift
 		# if requested fs not supported default it to ext2
 		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
 
@@ -249,7 +318,6 @@ zram_makefs()
 			cat err.log
 			echo "FAIL failed to make $fs on /dev/zram$i"
 		fi
-		i=$(($i + 1))
 		echo "zram mkfs.$fs: OK"
 	done
 }
@@ -257,13 +325,18 @@ zram_makefs()
 zram_mount()
 {
 	local i=0
-	for i in $(seq $dev_start $dev_end); do
+	for i in $dev_ids; do
 		echo "mount /dev/zram$i"
-		mkdir zram$i
-		mount /dev/zram$i zram$i > /dev/null || \
+		mkdir "zram$i" || return 1
+		if mount /dev/zram$i "zram$i" > /dev/null; then
+			dev_mount_ids="$dev_mount_ids $i"
+		else
 			echo "FAIL mount /dev/zram$i failed"
-		dev_mounted=$i
+			rmdir "zram$i" || return 1
+			return 1
+		fi
 	done
 
 	echo "zram mount of zram device(s): OK"
+	return 0
 }
-- 
2.55.0