[Devel] [PATCH hci-8.1 0/6] selftests: fix sticky failure in runners

[Devel] [PATCH hci-8.1 0/6] selftests: fix sticky failure in runners

[Eva Kurchatova]

Patchset for task https://virtuozzo.atlassian.net/browse/VSTOR-134205

Blocked on https://virtuozzo.atlassian.net/browse/VSTOR-150731

Eva Kurchatova (6):
  ms/selftests/bpf: Upon failures, exit with code 1 in test_xsk.sh
  selftests/memfd: fail run_hugetlbfs_test.sh when a test fails
  selftests: sigaltstack: report the failures the signal handler records
  selftests/zram: fail when a step fails
  selftests/futex: fail run.sh when a test fails
  selftests/futex: fail futex_waitv when the waiter failed

 tools/testing/selftests/bpf/test_xsk.sh       |  2 +
 .../selftests/futex/functional/futex_waitv.c  |  6 ++
 .../testing/selftests/futex/functional/run.sh | 66 ++++++++++---------
 .../selftests/memfd/run_hugetlbfs_test.sh     |  7 +-
 tools/testing/selftests/sigaltstack/sas.c     |  3 +-
 tools/testing/selftests/zram/zram.sh          | 11 +++-
 tools/testing/selftests/zram/zram01.sh        |  4 ++
 tools/testing/selftests/zram/zram02.sh        |  1 +
 tools/testing/selftests/zram/zram_lib.sh      | 24 ++++---
 9 files changed, 78 insertions(+), 46 deletions(-)

-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/6] ms/selftests/bpf: Upon failures, exit with code 1 in test_xsk.sh

[Eva Kurchatova]

Currently, even if some subtests fails, the end result will still yield
"ok 1 selftests: bpf: test_xsk.sh". Fix it by exiting with 1 if there are
any failures.

Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
Link: https://lore.kernel.org/bpf/20250828-selftests-bpf-test_xsk_ret-v1-1-e6656c01f397@suse.com
(cherry picked from commit 2a912258c90e895363c0ffc0be8a47f112ab67b7)

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/bpf/test_xsk.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 65aafe0003db..62db060298a4 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -241,4 +241,6 @@ done
 
 if [ $failures -eq 0 ]; then
         echo "All tests successful!"
+else
+	exit 1
 fi
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/6] selftests/memfd: fail run_hugetlbfs_test.sh when a test fails

[Eva Kurchatova]

The script runs memfd_test and the fuse test on hugetlbfs, puts the
huge page count back and exits with the status of whatever ran last,
so a test that failed or aborted was reported as passed. The hugetlbfs
F_SEAL_EXEC bug went unnoticed that way until the abort was seen by eye.
Exit with a failure when either test did.

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/memfd/run_hugetlbfs_test.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/memfd/run_hugetlbfs_test.sh b/tools/testing/selftests/memfd/run_hugetlbfs_test.sh
index fb633eeb0290..083bd72d9d41 100755
--- a/tools/testing/selftests/memfd/run_hugetlbfs_test.sh
+++ b/tools/testing/selftests/memfd/run_hugetlbfs_test.sh
@@ -57,8 +57,9 @@ fi
 #
 # Run the hugetlbfs test
 #
-./memfd_test hugetlbfs
-./run_fuse_test.sh hugetlbfs
+ret=0
+./memfd_test hugetlbfs || ret=1
+./run_fuse_test.sh hugetlbfs || ret=1
 
 #
 # Give back any huge pages allocated for the test
@@ -66,3 +67,5 @@ fi
 if [ -n "$nr_hugepgs" ]; then
 	echo $nr_hugepgs > /proc/sys/vm/nr_hugepages
 fi
+
+exit $ret
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 3/6] selftests: sigaltstack: report the failures the signal handler records

[Eva Kurchatova]

The handler records a failed check with ksft_test_result_fail() and
the test goes on, then ends with ksft_exit_pass() whatever was
recorded, so a failed check was reported as a pass. End by the
recorded results.

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/sigaltstack/sas.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 07227fab1cc9..758d8014cab1 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -192,6 +192,5 @@ int main(void)
 	ksft_test_result_pass(
 			"sigaltstack is still SS_AUTODISARM after signal\n");
 
-	ksft_exit_pass();
-	return 0;
+	ksft_finished();
 }
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 4/6] selftests/zram: fail when a step fails

[Eva Kurchatova]

The library prints FAIL for a step that fails and carries on, zram01
and zram02 print their verdict and exit 0 either way, and zram.sh
exits with the status of zram02 alone, so no zram failure reached the
runner. A failed step now fails its test, a failed test fails the
suite.

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/zram/zram.sh     | 11 +++++++++--
 tools/testing/selftests/zram/zram01.sh   |  4 ++++
 tools/testing/selftests/zram/zram02.sh   |  1 +
 tools/testing/selftests/zram/zram_lib.sh | 24 +++++++++++++++---------
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/zram/zram.sh b/tools/testing/selftests/zram/zram.sh
index b0b91d9b0dc2..56adeb17768f 100755
--- a/tools/testing/selftests/zram/zram.sh
+++ b/tools/testing/selftests/zram/zram.sh
@@ -4,15 +4,22 @@ TCID="zram.sh"
 
 . ./zram_lib.sh
 
+# A failure outranks a skip, a skip outranks a pass
+result () {
+	[ $1 -eq $ksft_skip ] && [ $ret -ne 0 ] || ret=$1
+}
+
 run_zram () {
 echo "--------------------"
 echo "running zram tests"
 echo "--------------------"
-./zram01.sh
+./zram01.sh || result $?
 echo ""
-./zram02.sh
+./zram02.sh || result $?
 }
 
 check_prereqs
 
+ret=0
 run_zram
+exit $ret
diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index 8f4affe34f3e..8ab63c98c1af 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -33,6 +33,9 @@ zram_algs="lzo"
 
 zram_fill_fs()
 {
+	# An earlier step failed, and after a failed mount the directory
+	# would be filled on the host's filesystem
+	[ $ERR_CODE -ne 0 ] && return
 	for i in $(seq $dev_start $dev_end); do
 		echo "fill zram$i..."
 		local b=0
@@ -70,6 +73,7 @@ zram_cleanup
 
 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..54e17bbfdbb7 100755
--- a/tools/testing/selftests/zram/zram02.sh
+++ b/tools/testing/selftests/zram/zram02.sh
@@ -39,6 +39,7 @@ zram_cleanup
 
 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 21ec1966de76..25e32cb956df 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -131,12 +131,14 @@ zram_max_streams()
 	for max_s in $zram_max_streams; do
 		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"
+			{ echo "FAIL failed to set '$max_s' to $sys_path"; ERR_CODE=-1; }
 		sleep 1
 		local max_streams=$(cat $sys_path)
 
-		[ "$max_s" -ne "$max_streams" ] && \
-			echo "FAIL can't set max_streams '$max_s', get $max_stream"
+		if [ "$max_s" -ne "$max_streams" ]; then
+			echo "FAIL can't set max_streams '$max_s', get $max_streams"
+			ERR_CODE=-1
+		fi
 
 		i=$(($i + 1))
 		echo "$sys_path = '$max_streams'"
@@ -156,7 +158,7 @@ zram_compress_alg()
 	for alg in $zram_algs; do
 		local sys_path="/sys/block/zram${i}/comp_algorithm"
 		echo "$alg" >	$sys_path || \
-			echo "FAIL can't set '$alg' to $sys_path"
+			{ echo "FAIL can't set '$alg' to $sys_path"; ERR_CODE=-1; }
 		i=$(($i + 1))
 		echo "$sys_path = '$alg'"
 	done
@@ -171,7 +173,7 @@ zram_set_disksizes()
 	for ds in $zram_sizes; do
 		local sys_path="/sys/block/zram${i}/disksize"
 		echo "$ds" >	$sys_path || \
-			echo "FAIL can't set '$ds' to $sys_path"
+			{ echo "FAIL can't set '$ds' to $sys_path"; ERR_CODE=-1; }
 
 		i=$(($i + 1))
 		echo "$sys_path = '$ds'"
@@ -188,7 +190,7 @@ zram_set_memlimit()
 	for ds in $zram_mem_limits; do
 		local sys_path="/sys/block/zram${i}/mem_limit"
 		echo "$ds" >	$sys_path || \
-			echo "FAIL can't set '$ds' to $sys_path"
+			{ echo "FAIL can't set '$ds' to $sys_path"; ERR_CODE=-1; }
 
 		i=$(($i + 1))
 		echo "$sys_path = '$ds'"
@@ -205,13 +207,15 @@ zram_makeswap()
 		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"
+			ERR_CODE=-1
 		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"
+			ERR_CODE=-1
 		fi
 
 		echo "done with /dev/zram$i"
@@ -229,6 +233,7 @@ zram_swapoff()
 		if [ $? -ne 0 ]; then
 			cat err.log
 			echo "FAIL swapoff /dev/zram$i failed"
+			ERR_CODE=-1
 		fi
 	done
 	dev_makeswap=-1
@@ -248,6 +253,7 @@ zram_makefs()
 		if [ $? -ne 0 ]; then
 			cat err.log
 			echo "FAIL failed to make $fs on /dev/zram$i"
+			ERR_CODE=-1
 		fi
 		i=$(($i + 1))
 		echo "zram mkfs.$fs: OK"
@@ -261,7 +267,7 @@ zram_mount()
 		echo "mount /dev/zram$i"
 		mkdir zram$i
 		mount /dev/zram$i zram$i > /dev/null || \
-			echo "FAIL mount /dev/zram$i failed"
+			{ echo "FAIL mount /dev/zram$i failed"; ERR_CODE=-1; }
 		dev_mounted=$i
 	done
 
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 5/6] selftests/futex: fail run.sh when a test fails

[Eva Kurchatova]

run.sh runs the tests one after the other and exits with the status of
the last one, so a failure in any other was reported as a pass. Exit
with a failure when any test did.

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 .../testing/selftests/futex/functional/run.sh | 66 ++++++++++---------
 1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/run.sh b/tools/testing/selftests/futex/functional/run.sh
index 5470088dc4df..39e71d91bebf 100755
--- a/tools/testing/selftests/futex/functional/run.sh
+++ b/tools/testing/selftests/futex/functional/run.sh
@@ -30,61 +30,65 @@ if [ "$USE_COLOR" -eq 1 ]; then
     COLOR="-c"
 fi
 
+ret=0
+
 
 echo
 # requeue pi testing
 # without timeouts
-./futex_requeue_pi $COLOR
-./futex_requeue_pi $COLOR -b
-./futex_requeue_pi $COLOR -b -l
-./futex_requeue_pi $COLOR -b -o
-./futex_requeue_pi $COLOR -l
-./futex_requeue_pi $COLOR -o
+./futex_requeue_pi $COLOR || ret=1
+./futex_requeue_pi $COLOR -b || ret=1
+./futex_requeue_pi $COLOR -b -l || ret=1
+./futex_requeue_pi $COLOR -b -o || ret=1
+./futex_requeue_pi $COLOR -l || ret=1
+./futex_requeue_pi $COLOR -o || ret=1
 # with timeouts
-./futex_requeue_pi $COLOR -b -l -t 5000
-./futex_requeue_pi $COLOR -l -t 5000
-./futex_requeue_pi $COLOR -b -l -t 500000
-./futex_requeue_pi $COLOR -l -t 500000
-./futex_requeue_pi $COLOR -b -t 5000
-./futex_requeue_pi $COLOR -t 5000
-./futex_requeue_pi $COLOR -b -t 500000
-./futex_requeue_pi $COLOR -t 500000
-./futex_requeue_pi $COLOR -b -o -t 5000
-./futex_requeue_pi $COLOR -l -t 5000
-./futex_requeue_pi $COLOR -b -o -t 500000
-./futex_requeue_pi $COLOR -l -t 500000
+./futex_requeue_pi $COLOR -b -l -t 5000 || ret=1
+./futex_requeue_pi $COLOR -l -t 5000 || ret=1
+./futex_requeue_pi $COLOR -b -l -t 500000 || ret=1
+./futex_requeue_pi $COLOR -l -t 500000 || ret=1
+./futex_requeue_pi $COLOR -b -t 5000 || ret=1
+./futex_requeue_pi $COLOR -t 5000 || ret=1
+./futex_requeue_pi $COLOR -b -t 500000 || ret=1
+./futex_requeue_pi $COLOR -t 500000 || ret=1
+./futex_requeue_pi $COLOR -b -o -t 5000 || ret=1
+./futex_requeue_pi $COLOR -l -t 5000 || ret=1
+./futex_requeue_pi $COLOR -b -o -t 500000 || ret=1
+./futex_requeue_pi $COLOR -l -t 500000 || ret=1
 # with long timeout
-./futex_requeue_pi $COLOR -b -l -t 2000000000
-./futex_requeue_pi $COLOR -l -t 2000000000
+./futex_requeue_pi $COLOR -b -l -t 2000000000 || ret=1
+./futex_requeue_pi $COLOR -l -t 2000000000 || ret=1
 
 
 echo
-./futex_requeue_pi_mismatched_ops $COLOR
+./futex_requeue_pi_mismatched_ops $COLOR || ret=1
 
 echo
-./futex_requeue_pi_signal_restart $COLOR
+./futex_requeue_pi_signal_restart $COLOR || ret=1
 
 echo
-./futex_wait_timeout $COLOR
+./futex_wait_timeout $COLOR || ret=1
 
 echo
-./futex_wait_wouldblock $COLOR
+./futex_wait_wouldblock $COLOR || ret=1
 
 echo
-./futex_wait_uninitialized_heap $COLOR
-./futex_wait_private_mapped_file $COLOR
+./futex_wait_uninitialized_heap $COLOR || ret=1
+./futex_wait_private_mapped_file $COLOR || ret=1
 
 echo
-./futex_wait $COLOR
+./futex_wait $COLOR || ret=1
 
 echo
-./futex_requeue $COLOR
+./futex_requeue $COLOR || ret=1
 
 echo
-./futex_waitv $COLOR
+./futex_waitv $COLOR || ret=1
 
 echo
-./futex_priv_hash $COLOR
+./futex_priv_hash $COLOR || ret=1
 
 echo
-./futex_numa_mpol $COLOR
+./futex_numa_mpol $COLOR || ret=1
+
+exit $ret
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 6/6] selftests/futex: fail futex_waitv when the waiter failed

[Eva Kurchatova]

The waiter thread records a failed futex_waitv() with
ksft_test_result_fail() and the test exits with the status its main
thread collected, so a waiter that returned the wrong index or an
error was reported as a pass; and the main thread never waited for the
waiter, so its result could land after the verdict. Join the waiter
after each wake and fail when it recorded a failure.

https://virtuozzo.atlassian.net/browse/VSTOR-134205
Feature: fix selftests

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/futex/functional/futex_waitv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/futex/functional/futex_waitv.c b/tools/testing/selftests/futex/functional/futex_waitv.c
index a94337f677e1..20fc537c400c 100644
--- a/tools/testing/selftests/futex/functional/futex_waitv.c
+++ b/tools/testing/selftests/futex/functional/futex_waitv.c
@@ -100,6 +100,7 @@ int main(int argc, char *argv[])
 	usleep(WAKE_WAIT_US);
 
 	res = futex_wake(u64_to_ptr(waitv[NR_FUTEXES - 1].uaddr), 1, FUTEX_PRIVATE_FLAG);
+	pthread_join(waiter, NULL);
 	if (res != 1) {
 		ksft_test_result_fail("futex_wake private returned: %d %s\n",
 				      res ? errno : res,
@@ -133,6 +134,7 @@ int main(int argc, char *argv[])
 	usleep(WAKE_WAIT_US);
 
 	res = futex_wake(u64_to_ptr(waitv[NR_FUTEXES - 1].uaddr), 1, 0);
+	pthread_join(waiter, NULL);
 	if (res != 1) {
 		ksft_test_result_fail("futex_wake shared returned: %d %s\n",
 				      res ? errno : res,
@@ -232,6 +234,10 @@ int main(int argc, char *argv[])
 		ksft_test_result_pass("futex_waitv invalid clockid\n");
 	}
 
+	/* The waiter records its own failures */
+	if (ksft_get_fail_cnt())
+		ret = RET_FAIL;
+
 	ksft_print_cnts();
 	return ret;
 }
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/6] selftests/memfd: fail run_hugetlbfs_test.sh when a test fails

[no-reply]

> The script runs memfd_test and the fuse test on hugetlbfs, puts the
> huge page count back and exits with the status of whatever ran last,
> so a test that failed or aborted was reported as passed. The hugetlbfs
> F_SEAL_EXEC bug went unnoticed that way until the abort was seen by eye.
> Exit with a failure when either test did.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-134205
> Feature: fix selftests
> 
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/6] ms/selftests/bpf: Upon failures, exit with code 1 in test_xsk.sh

[no-reply]

> Currently, even if some subtests fails, the end result will still yield
> "ok 1 selftests: bpf: test_xsk.sh". Fix it by exiting with 1 if there are
> any failures.
> 
> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Link: https://lore.kernel.org/bpf/20250828-selftests-bpf-test_xsk_ret-v1-1-e6656c01f397@suse.com
> (cherry picked from commit 2a912258c90e895363c0ffc0be8a47f112ab67b7)
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-134205
> Feature: fix selftests
> 
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 3/6] selftests: sigaltstack: report the failures the signal handler records

[no-reply]

> The handler records a failed check with ksft_test_result_fail() and
> the test goes on, then ends with ksft_exit_pass() whatever was
> recorded, so a failed check was reported as a pass. End by the
> recorded results.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-134205
> Feature: fix selftests
> 
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=3


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 4/6] selftests/zram: fail when a step fails

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/zram: zram_load() and zram_cleanup() failures still yield PASS
--

commit 0339f0cb6294df6e9df86072b01edefd848cf680
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests/zram: fail when a step fails

This patch makes the zram selftest library set ERR_CODE when a step fails.
It makes zram01 and zram02 exit 1 on failure, and makes zram.sh exit with
the combined result of both tests.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-134205

> diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
> --- a/tools/testing/selftests/zram/zram_lib.sh
> +++ b/tools/testing/selftests/zram/zram_lib.sh

[ ... ]

> @@ -131,12 +131,14 @@ zram_max_streams()
>  	for max_s in $zram_max_streams; do
>  		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"
> +			{ echo "FAIL failed to set '$max_s' to $sys_path"; ERR_CODE=-1; }
>  		sleep 1
>  		local max_streams=$(cat $sys_path)
>  
> -		[ "$max_s" -ne "$max_streams" ] && \
> -			echo "FAIL can't set max_streams '$max_s', get $max_stream"
> +		if [ "$max_s" -ne "$max_streams" ]; then
> +			echo "FAIL can't set max_streams '$max_s', get $max_streams"
> +			ERR_CODE=-1
> +		fi

[Severity: Low]
This isn't a regression in the new code, but is the max_streams check
complete?

If cat returns an empty or non-numeric value, the test
[ "$max_s" -ne "$max_streams" ] fails with a shell error (status 2).  The
if statement treats that as false, so the FAIL branch is skipped and
ERR_CODE stays unset in zram_max_streams().

This is caught only if the preceding echo to max_comp_streams also failed.

[ ... ]

> @@ -261,7 +267,7 @@ zram_mount()
>  		echo "mount /dev/zram$i"
>  		mkdir zram$i
>  		mount /dev/zram$i zram$i > /dev/null || \
> -			echo "FAIL mount /dev/zram$i failed"
> +			{ echo "FAIL mount /dev/zram$i failed"; ERR_CODE=-1; }
>  		dev_mounted=$i
>  	done

[Severity: Low]
The commit message says "A failed step now fails its test", but some library
steps still do not record failures in ERR_CODE.  Can a zram test still print
PASS and exit 0 after a library step failed?

In zram_load(), the result of these commands is not checked:

    cat /sys/class/zram-control/hot_add > /dev/null
    ...
    modprobe zram num_devices=$dev_num

A failure there is caught only indirectly, if a later sysfs write fails.

In zram_cleanup(), the result of these commands is not checked:

    swapoff /dev/zram$i
    ...
    umount /dev/zram$i
    ...
    echo 1 > /sys/block/zram${i}/reset

zram_cleanup() runs before the verdict in zram01 and zram02.  A teardown
failure that leaves swap or a mount behind therefore still ends in PASS.

Should these steps set ERR_CODE=-1 on failure as well, or should the commit
message say that load and cleanup are excluded?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=4

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 5/6] selftests/futex: fail run.sh when a test fails

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/futex: futex_wait_wouldblock reports PASS on futex_waitv failure
--

commit af44cf5afc0f94fd2ab55dd4e0f6be72e650b92f
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests/futex: fail run.sh when a test fails

This patch changes run.sh in the futex functional selftests so that it
records a failure from any test binary and exits 1 at the end. Before this,
run.sh exited with the status of the last test only.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-134205

> diff --git a/tools/testing/selftests/futex/functional/run.sh b/tools/testing/selftests/futex/functional/run.sh
> --- a/tools/testing/selftests/futex/functional/run.sh
> +++ b/tools/testing/selftests/futex/functional/run.sh

[ ... ]

>  echo
> -./futex_wait_wouldblock $COLOR
> +./futex_wait_wouldblock $COLOR || ret=1

[Severity: Medium]
This isn't a bug in run.sh itself, but it exposes a reporting problem in
futex_wait_wouldblock.c that this patch does not address.

In main() of futex_wait_wouldblock.c, the futex_waitv failure branch
reports a pass and then sets a failing return value:

	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
	if (!res || errno != EWOULDBLOCK) {
		ksft_test_result_pass("futex_waitv returned: %d %s\n",
				      res ? errno : res,
				      res ? strerror(errno) : "");
		ret = RET_FAIL;
	}

If futex_waitv() returns 0, or fails with an errno other than EWOULDBLOCK,
the test prints an "ok" line and then exits non-zero.

Before this patch, run.sh ignored that exit status. With the new
"|| ret=1", run.sh now exits 1 on this path, but the output contains no
failing line to explain why.

The preceding futex_wait branch in the same function uses
ksft_test_result_fail() for the equivalent condition.

Should the futex_waitv failure branch call ksft_test_result_fail() instead
of ksft_test_result_pass()?

I compared this commit with the end of the series (d7a86a7b) and found no
change to futex_wait_wouldblock.c. The patch that touches futex_waitv.c does
not cover it either.

>  echo
> -./futex_wait_uninitialized_heap $COLOR
> -./futex_wait_private_mapped_file $COLOR
> +./futex_wait_uninitialized_heap $COLOR || ret=1
> +./futex_wait_private_mapped_file $COLOR || ret=1

[ ... ]

> +./futex_numa_mpol $COLOR || ret=1
> +
> +exit $ret

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=5

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 6/6] selftests/futex: fail futex_waitv when the waiter failed

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/futex: futex_waitv double-reports subtest when waiterfn() fails
--

commit d7a86a7b77fbc111f003b6a1f5173be520b5ac79
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests/futex: fail futex_waitv when the waiter failed

This patch joins the waiter thread after each futex_wake() in the private
and shared subtests of futex_waitv. It also sets the exit status to
RET_FAIL if ksft_get_fail_cnt() is non-zero at the end of main().

Link: https://virtuozzo.atlassian.net/browse/VSTOR-134205

> diff --git a/tools/testing/selftests/futex/functional/futex_waitv.c b/tools/testing/selftests/futex/functional/futex_waitv.c
> --- a/tools/testing/selftests/futex/functional/futex_waitv.c
> +++ b/tools/testing/selftests/futex/functional/futex_waitv.c

[ ... ]

> @@ -100,6 +100,7 @@ int main(int argc, char *argv[])
>  	usleep(WAKE_WAIT_US);
>  
>  	res = futex_wake(u64_to_ptr(waitv[NR_FUTEXES - 1].uaddr), 1, FUTEX_PRIVATE_FLAG);
> +	pthread_join(waiter, NULL);
>  	if (res != 1) {
>  		ksft_test_result_fail("futex_wake private returned: %d %s\n",
>  				      res ? errno : res,
>  				      res ? strerror(errno) : "");
>  		ret = RET_FAIL;
>  	} else {
>  		ksft_test_result_pass("futex_waitv private\n");
>  	}

[Severity: Medium]
Can this report the same subtest twice?

If waiterfn() fails, it calls ksft_test_result_fail() itself:

waiterfn()
    res = futex_waitv(waitv, NR_FUTEXES, 0, &to, CLOCK_MONOTONIC);
    if (res < 0) {
        ksft_test_result_fail("futex_waitv returned: %d %s\n", ...);

The pthread_join() here discards the waiter's return value. If
futex_wake() then returns 1, main() still reaches
ksft_test_result_pass("futex_waitv private\n").

That would print both a "not ok" and an "ok" line for the same subtest.
The total number of results would then exceed ksft_set_plan(7).

The same applies to the shared case, which ends in
ksft_test_result_pass("futex_waitv shared\n").

Would it be cleaner for waiterfn() to return its status, so that main()
collects it through pthread_join() and reports a single result per subtest?

[ ... ]

> @@ -232,6 +234,10 @@ int main(int argc, char *argv[])
>  		ksft_test_result_pass("futex_waitv invalid clockid\n");
>  	}
>  
> +	/* The waiter records its own failures */
> +	if (ksft_get_fail_cnt())
> +		ret = RET_FAIL;
> +

[Severity: Medium]
Does this check only fix the exit code?

ksft_get_fail_cnt() is a global counter. It cannot tell a waiter failure
from a failure in main(). It also cannot attribute the failure to a
particular subtest.

Returning the waiter's status through pthread_join() would make this check
unnecessary.

>  	ksft_print_cnts();
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261026124139.606106-1-eva.kurchatova@virtuozzo.com?part=6

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel