[PATCH sched_ext for-7.4] selftests/sched_ext: Fix rt_stall runner abort

[PATCH sched_ext for-7.4] selftests/sched_ext: Fix rt_stall runner abort

[Cheng-Yang Chou]

rt_stall uses ksft_exit_fail() and friends, which call exit(), so a
single rt_stall failure or a fork() error kills the runner and skips
the remaining tests.

Drop the ksft_* usage, report the handshake failure back from
wait_ready(), and let run() return SCX_TEST_FAIL so the runner records
the failure and moves on. Since the test now returns rather than
exiting, route every path after the forks through one cleanup label, so
a failure cannot leave the SCHED_FIFO task spinning on CORE_ID for the
tests that follow.

Also destroy the struct_ops link before checking the exit kind, so the
link is released when that check fails.

Fixes: be621a76341c ("selftests/sched_ext: Add test for sched_ext dl_server")
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
---
 tools/testing/selftests/sched_ext/rt_stall.c | 80 ++++++++++----------
 1 file changed, 42 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/sched_ext/rt_stall.c b/tools/testing/selftests/sched_ext/rt_stall.c
index a5041fc2e44f..a1552b38a6e8 100644
--- a/tools/testing/selftests/sched_ext/rt_stall.c
+++ b/tools/testing/selftests/sched_ext/rt_stall.c
@@ -17,7 +17,6 @@
 #include <scx/common.h>
 #include "rt_stall.bpf.skel.h"
 #include "scx_test.h"
-#include "../kselftest.h"
 
 #define CORE_ID		0	/* CPU to pin tasks to */
 #define RUN_TIME        5	/* How long to run the test in seconds */
@@ -35,15 +34,17 @@ static void signal_ready(int fd)
 }
 
 /* Wait for a child to signal readiness via a pipe */
-static void wait_ready(int fd)
+static bool wait_ready(int fd)
 {
+	bool ready;
 	char c;
 
-	if (read(fd, &c, 1) != 1) {
+	ready = read(fd, &c, 1) == 1;
+	if (!ready)
 		perror("read from ready pipe");
-		exit(EXIT_FAILURE);
-	}
 	close(fd);
+
+	return ready;
 }
 
 /* Simple busy-wait function for test tasks */
@@ -151,13 +152,11 @@ static bool sched_stress_test(bool is_ext)
 	float ext_runtime, rt_runtime, actual_ratio;
 	int ext_pid, rt_pid;
 	int ext_ready[2], rt_ready[2];
-
-	ksft_print_header();
-	ksft_set_plan(1);
+	bool ret = false;
 
 	if (pipe(ext_ready) || pipe(rt_ready)) {
 		perror("pipe");
-		ksft_exit_fail();
+		return false;
 	}
 
 	/* Create and set up a EXT task */
@@ -172,7 +171,7 @@ static bool sched_stress_test(bool is_ext)
 		exit(0);
 	} else if (ext_pid < 0) {
 		perror("fork task");
-		ksft_exit_fail();
+		return false;
 	}
 
 	/* Create an RT task */
@@ -188,7 +187,7 @@ static bool sched_stress_test(bool is_ext)
 		exit(0);
 	} else if (rt_pid < 0) {
 		perror("fork for RT task");
-		ksft_exit_fail();
+		goto out_kill_ext;
 	}
 
 	/*
@@ -199,45 +198,47 @@ static bool sched_stress_test(bool is_ext)
 	 */
 	close(ext_ready[1]);
 	close(rt_ready[1]);
-	wait_ready(ext_ready[0]);
-	wait_ready(rt_ready[0]);
+	if (!wait_ready(ext_ready[0]) || !wait_ready(rt_ready[0]))
+		goto out_kill;
 
 	/* Let the processes run for the specified time */
 	sleep(RUN_TIME);
 
 	/* Get runtime for the EXT task */
 	ext_runtime = get_process_runtime(ext_pid);
-	if (ext_runtime == -1)
-		ksft_exit_fail_msg("Error getting runtime for %s task (PID %d)\n",
-				   class_str, ext_pid);
-	ksft_print_msg("Runtime of %s task (PID %d) is %f seconds\n",
-		       class_str, ext_pid, ext_runtime);
+	if (ext_runtime == -1) {
+		fprintf(stderr, "Failed to read %s task runtime\n", class_str);
+		goto out_kill;
+	}
 
 	/* Get runtime for the RT task */
 	rt_runtime = get_process_runtime(rt_pid);
-	if (rt_runtime == -1)
-		ksft_exit_fail_msg("Error getting runtime for RT task (PID %d)\n", rt_pid);
-	ksft_print_msg("Runtime of RT task (PID %d) is %f seconds\n", rt_pid, rt_runtime);
-
-	/* Kill the processes */
-	kill(ext_pid, SIGKILL);
-	kill(rt_pid, SIGKILL);
-	waitpid(ext_pid, NULL, 0);
-	waitpid(rt_pid, NULL, 0);
+	if (rt_runtime == -1) {
+		fprintf(stderr, "Failed to read RT task runtime\n");
+		goto out_kill;
+	}
 
 	/* Verify that the scx task got enough runtime */
 	actual_ratio = ext_runtime / (ext_runtime + rt_runtime);
-	ksft_print_msg("%s task got %.2f%% of total runtime\n",
-		       class_str, actual_ratio * 100);
+	fprintf(stderr, "%s task ran %.3fs, RT task ran %.3fs (%.2f%% of runtime)\n",
+		class_str, ext_runtime, rt_runtime, actual_ratio * 100);
 
-	if (actual_ratio >= expected_min_ratio) {
-		ksft_test_result_pass("PASS: %s task got more than %.2f%% of runtime\n",
-				      class_str, expected_min_ratio * 100);
-		return true;
+	if (actual_ratio < expected_min_ratio) {
+		fprintf(stderr, "%s task got less than %.2f%% of runtime\n",
+			class_str, expected_min_ratio * 100);
+		goto out_kill;
 	}
-	ksft_test_result_fail("FAIL: %s task got less than %.2f%% of runtime\n",
-			      class_str, expected_min_ratio * 100);
-	return false;
+
+	ret = true;
+
+out_kill:
+	kill(rt_pid, SIGKILL);
+	waitpid(rt_pid, NULL, 0);
+out_kill_ext:
+	kill(ext_pid, SIGKILL);
+	waitpid(ext_pid, NULL, 0);
+
+	return ret;
 }
 
 static enum scx_test_status run(void *ctx)
@@ -263,14 +264,17 @@ static enum scx_test_status run(void *ctx)
 			link = bpf_map__attach_struct_ops(skel->maps.rt_stall_ops);
 			SCX_FAIL_IF(!link, "Failed to attach scheduler");
 		}
+
 		res = sched_stress_test(is_ext);
+
 		if (is_ext) {
-			SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_NONE));
+			int exit_kind = skel->data->uei.kind;
 			bpf_link__destroy(link);
+			SCX_EQ(exit_kind, EXIT_KIND(SCX_EXIT_NONE));
 		}
 
 		if (!res)
-			ksft_exit_fail();
+			return SCX_TEST_FAIL;
 	}
 
 	return SCX_TEST_PASS;
-- 
2.55.0

Re: [PATCH sched_ext for-7.4] selftests/sched_ext: Fix rt_stall runner abort

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/sched_ext: file descriptor leaks in error paths
--

commit bd4cf2003fa0e397ca084d4257a65e3f8e741ed8
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>

selftests/sched_ext: Fix rt_stall runner abort

This commit drops the ksft_exit_fail() usage in rt_stall in favor of returning
false, allowing the test runner to handle the failure. It also refactors the
cleanup paths to avoid leaving SCHED_FIFO tasks spinning on failure.

> diff --git a/tools/testing/selftests/sched_ext/rt_stall.c b/tools/testing/selftests/sched_ext/rt_stall.c
> index a5041fc2e44fd..a1552b38a6e8c 100644
> --- a/tools/testing/selftests/sched_ext/rt_stall.c
> +++ b/tools/testing/selftests/sched_ext/rt_stall.c
[ ... ]
> @@ -151,13 +152,11 @@ static bool sched_stress_test(bool is_ext)
>  	float ext_runtime, rt_runtime, actual_ratio;
>  	int ext_pid, rt_pid;
>  	int ext_ready[2], rt_ready[2];
> -
> -	ksft_print_header();
> -	ksft_set_plan(1);
> +	bool ret = false;
>  
>  	if (pipe(ext_ready) || pipe(rt_ready)) {
>  		perror("pipe");
> -		ksft_exit_fail();
> +		return false;
>  	}

[Severity: Medium]
If the first pipe creation succeeds but the second fails, does this early
return leak the ext_ready file descriptors?

[ ... ]
> @@ -172,7 +171,7 @@ static bool sched_stress_test(bool is_ext)
>  		exit(0);
>  	} else if (ext_pid < 0) {
>  		perror("fork task");
> -		ksft_exit_fail();
> +		return false;
>  	}

[Severity: Medium]
In the event that the first fork fails, is there a chance that all four
pipe file descriptors allocated earlier will be left open?

[ ... ]
> @@ -188,7 +187,7 @@ static bool sched_stress_test(bool is_ext)
>  		exit(0);
>  	} else if (rt_pid < 0) {
>  		perror("fork for RT task");
> -		ksft_exit_fail();
> +		goto out_kill_ext;
>  	}

[Severity: Medium]
Could jumping to out_kill_ext here cause the pipe file descriptors to be
leaked, since they aren't explicitly closed at that label?

[ ... ]
> @@ -199,45 +198,47 @@ static bool sched_stress_test(bool is_ext)
>  	 */
>  	close(ext_ready[1]);
>  	close(rt_ready[1]);
> -	wait_ready(ext_ready[0]);
> -	wait_ready(rt_ready[0]);
> +	if (!wait_ready(ext_ready[0]) || !wait_ready(rt_ready[0]))
> +		goto out_kill;

[Severity: Medium]
If wait_ready(ext_ready[0]) returns false, won't the short-circuit evaluation
skip calling wait_ready(rt_ready[0])? Since wait_ready() handles closing
the file descriptor, this could leave rt_ready[0] open.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922155037.17494-1-yphbchou0911@gmail.com?part=1

Re: [PATCH sched_ext for-7.4] selftests/sched_ext: Fix rt_stall runner abort

[Tejun Heo]

Applied to sched_ext/for-7.4.

Thanks.
--
tejuns