On Mon, Apr 29, 2024 at 04:40:12AM +0000, Dorine Tipo wrote:
[PATCH] selftests:arm64: Test PR_SVE_VL_INHERIT after a double fork
Nit: Please follow the same commit message prefix convention as in the existing code (e.g., try git log tools/testing/selftests/arm64/fp/ ).
[...]
Add a new test, double_fork_test() to check the inheritance of the SVE vector length after a double fork. The `EXPECTED_TESTS` macro has been updated to account for this additional test. This patch addresses task 7 on the TODO list.
Signed-off-by: Dorine Tipo dorine.a.tipo@gmail.com
tools/testing/selftests/arm64/fp/za-fork.c | 95 +++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/fp/za-fork.c b/tools/testing/selftests/arm64/fp/za-fork.c index 587b94648222..35229e570dcf 100644 --- a/tools/testing/selftests/arm64/fp/za-fork.c +++ b/tools/testing/selftests/arm64/fp/za-fork.c @@ -11,7 +11,7 @@
#include "kselftest.h"
-#define EXPECTED_TESTS 1 +#define EXPECTED_TESTS 2
int fork_test(void); int verify_fork(void); @@ -69,6 +69,97 @@ int fork_test_c(void) }
"git am" or "patch" report the patch as corrupted here, any idea why?
Fixing the whitespace here allows the patch to be applied, but it still doesn't seem to build, so I think there are bigger problems here.
}
+int double_fork_test(void) +{
- pid_t newpid, grandchild_pid, waiting;
- int ret, child_status, parent_result;
- ret = prctl(PR_SVE_SET_VL, vl | PR_SVE_VL_INHERIT);
- if (ret < 0)
ksft_exit_fail_msg("Failed to set SVE VL %d\n", vl);
Thanks for the attempt, but this file doesn't look like the correct place to add these tests. This file is specific to testing SME (the "ZA" register array is specific to SME). Looking at the history of the TODO file, it looks like the entry you're looking at was related to the tests in vec-syscfg.c. That looks like a better place to test the API behaviour of these prctl calls; most of the needed framework is already there.
So, please consider adapting vec-syscfg.c to add the double-fork tests, rather than trying to write this from scratch.
(See commit e96595c55d23 ("kselftest/arm64: Add a TODO list for floating point tests") for history.)
If you haven't already done so, you should go and look at the following to get a better idea of how these prctl calls are supposed to work:
Documentation/arch/arm64/sve.rst Documentation/arch/arm64/sme.rst
The prctl(2) man page also documents the SVE prctls:
https://man7.org/linux/man-pages/man2/prctl.2.html .
[more comments below]
- newpid = fork();
- if (newpid == 0) {
/* In child */if (!verify_fork()) {ksft_print_msg("ZA state invalid in child\n");exit(0);}grandchild_pid = fork();if (grandchild_pid == 0) {/* in grandchild */if (!verfy_fork()) {ksft_print_msg("ZA state invalid in grandchild\n");exit(0);}ret = prctl(PR_SVE_GET_VL);if (ret & PR_SVE_VL_INHERIT) {ksft_print_msg("prctl() reports _INHERIT\n");return;}ksft_print_msg("prctl() does not report _INHERIT\n");} else if (grandchild_pid < 0) {ksft_print_msg("fork() failed in first child: %d\n", grandchild_pid);return 0;}/* Wait for the grandchild process to exit */waiting = waitpid(grandchild_pid, &child_status, 0);if (waiting < 0) {if (errno == EINTR)continue;ksft_print_msg("waitpid() failed: %d\n", errno);return 0;}if (waiting != grandchild_pid) {ksft_print_msg("waitpid() returned wrong PID\n");return 0;}if (!WIFEXITED(child_status)) {ksft_print_msg("grandchild did not exit\n");return 0;}exit(1);}- }
- if (newpid < 0) {
ksft_print_msg("fork() failed: %d\n", newpid);return 0;- }
- parent_result = verify_fork();
- if (!parent_result)
ksft_print_msg("ZA state invalid in parent\n");- for (;;) {
waiting = waitpid(newpid, &child_status, 0);if (waiting < 0) {if (errno == EINTR)continue;ksft_print_msg("waitpid() failed: %d\n", errno);return 0;}if (waiting != newpid) {ksft_print_msg("waitpid() returned wrong PID\n");return 0;}if (!WIFEXITED(child_status)) {ksft_print_msg("child did not exit\n");return 0;}return WEXITSTATUS(child_status) && parent_result;- }
+}
int main(int argc, char **argv) { int ret, i; @@ -86,11 +177,13 @@ int main(int argc, char **argv) ret = open("/proc/sys/abi/sme_default_vector_length", O_RDONLY, 0); if (ret >= 0) { ksft_test_result(fork_test(), "fork_test\n");
ksft_test_result(double_fork_test(), "double_fork_test\n");} else { ksft_print_msg("SME not supported\n"); for (i = 0; i < EXPECTED_TESTS; i++) { ksft_test_result_skip("fork_test\n");
ksft_test_result_skip("double_fork_test\n");
I'm not very familiar with the kselftest framework, but this looks weird to me.
Why should we skip both tests twice when there are two tests?
(Looking at the history, I think this loop may have been left behind from an old edit and perhaps shouldn't be there any more. Comparing with the way ksft_test_result_skip() is used in other tests may provide a clue. Maybe Mark Brown remembers...)
}}
And finally... how are you running these tests?
Cheers ---Dave