From 18378b0e49d97719467ce9b098aa859b1b750bb4 Mon Sep 17 00:00:00 2001 From: Juntong Deng Date: Tue, 26 Sep 2023 04:35:52 +0800 Subject: [PATCH 01/51] selftests/damon: Add executable permission to test scripts When running the test for the damon subsystem, there are a lot of warnings because test scripts do not have executable permission, for example: Warning: file debugfs_attrs.sh is not executable Warning: file debugfs_schemes.sh is not executable Warning: file debugfs_target_ids.sh is not executable ... This patch adds executable permission to test scripts to eliminate these warnings. Signed-off-by: Juntong Deng Reviewed-by: SeongJae Park Signed-off-by: Shuah Khan --- tools/testing/selftests/damon/debugfs_attrs.sh | 0 .../testing/selftests/damon/debugfs_duplicate_context_creation.sh | 0 tools/testing/selftests/damon/debugfs_empty_targets.sh | 0 tools/testing/selftests/damon/debugfs_huge_count_read_write.sh | 0 tools/testing/selftests/damon/debugfs_rm_non_contexts.sh | 0 tools/testing/selftests/damon/debugfs_schemes.sh | 0 tools/testing/selftests/damon/debugfs_target_ids.sh | 0 tools/testing/selftests/damon/lru_sort.sh | 0 tools/testing/selftests/damon/reclaim.sh | 0 tools/testing/selftests/damon/sysfs.sh | 0 tools/testing/selftests/damon/sysfs_update_removed_scheme_dir.sh | 0 11 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_attrs.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_duplicate_context_creation.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_empty_targets.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_huge_count_read_write.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_rm_non_contexts.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_schemes.sh mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_target_ids.sh mode change 100644 => 100755 tools/testing/selftests/damon/lru_sort.sh mode change 100644 => 100755 tools/testing/selftests/damon/reclaim.sh mode change 100644 => 100755 tools/testing/selftests/damon/sysfs.sh mode change 100644 => 100755 tools/testing/selftests/damon/sysfs_update_removed_scheme_dir.sh diff --git a/tools/testing/selftests/damon/debugfs_attrs.sh b/tools/testing/selftests/damon/debugfs_attrs.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_duplicate_context_creation.sh b/tools/testing/selftests/damon/debugfs_duplicate_context_creation.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_empty_targets.sh b/tools/testing/selftests/damon/debugfs_empty_targets.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh b/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_rm_non_contexts.sh b/tools/testing/selftests/damon/debugfs_rm_non_contexts.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_schemes.sh b/tools/testing/selftests/damon/debugfs_schemes.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/debugfs_target_ids.sh b/tools/testing/selftests/damon/debugfs_target_ids.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/lru_sort.sh b/tools/testing/selftests/damon/lru_sort.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/reclaim.sh b/tools/testing/selftests/damon/reclaim.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/sysfs.sh b/tools/testing/selftests/damon/sysfs.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/damon/sysfs_update_removed_scheme_dir.sh b/tools/testing/selftests/damon/sysfs_update_removed_scheme_dir.sh old mode 100644 new mode 100755 From 078a2ead544c56fbe6a3bed8cc90b5151a328b81 Mon Sep 17 00:00:00 2001 From: Justin Stitt Date: Tue, 12 Sep 2023 21:03:50 +0000 Subject: [PATCH 02/51] selftests/rseq: fix kselftest Clang build warnings When building with Clang, I am getting many warnings from the selftests/rseq tree. Here's one such example from rseq tree: | param_test.c:1234:10: error: address argument to atomic operation must be a pointer to _Atomic type ('intptr_t *' (aka 'long *') invalid) | 1234 | while (!atomic_load(&args->percpu_list_ptr)) {} | | ^ ~~~~~~~~~~~~~~~~~~~~~~ | /usr/local/google/home/justinstitt/repos/tc-build/build/llvm/final/lib/clang/18/include/stdatomic.h:140:29: note: expanded from macro 'atomic_load' | 140 | #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST) | | ^ ~~~~~~ Use compiler builtins `__atomic_load_n()` and `__atomic_store_n()` with accompanying __ATOMIC_ACQUIRE and __ATOMIC_RELEASE, respectively. This will fix the warnings because the compiler builtins do not expect their arguments to have _Atomic type. This should also make TSAN happier. Link: https://github.com/ClangBuiltLinux/linux/issues/1698 Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61 Suggested-by: Mathieu Desnoyers Signed-off-by: Justin Stitt Reviewed-by: Mathieu Desnoyers Signed-off-by: Shuah Khan --- tools/testing/selftests/rseq/param_test.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/rseq/param_test.c b/tools/testing/selftests/rseq/param_test.c index bf951a490bb4..20403d58345c 100644 --- a/tools/testing/selftests/rseq/param_test.c +++ b/tools/testing/selftests/rseq/param_test.c @@ -1231,7 +1231,7 @@ void *test_membarrier_worker_thread(void *arg) } /* Wait for initialization. */ - while (!atomic_load(&args->percpu_list_ptr)) {} + while (!__atomic_load_n(&args->percpu_list_ptr, __ATOMIC_ACQUIRE)) {} for (i = 0; i < iters; ++i) { int ret; @@ -1299,22 +1299,22 @@ void *test_membarrier_manager_thread(void *arg) test_membarrier_init_percpu_list(&list_a); test_membarrier_init_percpu_list(&list_b); - atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a); + __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_a, __ATOMIC_RELEASE); - while (!atomic_load(&args->stop)) { + while (!__atomic_load_n(&args->stop, __ATOMIC_ACQUIRE)) { /* list_a is "active". */ cpu_a = rand() % CPU_SETSIZE; /* * As list_b is "inactive", we should never see changes * to list_b. */ - if (expect_b != atomic_load(&list_b.c[cpu_b].head->data)) { + if (expect_b != __atomic_load_n(&list_b.c[cpu_b].head->data, __ATOMIC_ACQUIRE)) { fprintf(stderr, "Membarrier test failed\n"); abort(); } /* Make list_b "active". */ - atomic_store(&args->percpu_list_ptr, (intptr_t)&list_b); + __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_b, __ATOMIC_RELEASE); if (rseq_membarrier_expedited(cpu_a) && errno != ENXIO /* missing CPU */) { perror("sys_membarrier"); @@ -1324,27 +1324,27 @@ void *test_membarrier_manager_thread(void *arg) * Cpu A should now only modify list_b, so the values * in list_a should be stable. */ - expect_a = atomic_load(&list_a.c[cpu_a].head->data); + expect_a = __atomic_load_n(&list_a.c[cpu_a].head->data, __ATOMIC_ACQUIRE); cpu_b = rand() % CPU_SETSIZE; /* * As list_a is "inactive", we should never see changes * to list_a. */ - if (expect_a != atomic_load(&list_a.c[cpu_a].head->data)) { + if (expect_a != __atomic_load_n(&list_a.c[cpu_a].head->data, __ATOMIC_ACQUIRE)) { fprintf(stderr, "Membarrier test failed\n"); abort(); } /* Make list_a "active". */ - atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a); + __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_a, __ATOMIC_RELEASE); if (rseq_membarrier_expedited(cpu_b) && errno != ENXIO /* missing CPU*/) { perror("sys_membarrier"); abort(); } /* Remember a value from list_b. */ - expect_b = atomic_load(&list_b.c[cpu_b].head->data); + expect_b = __atomic_load_n(&list_b.c[cpu_b].head->data, __ATOMIC_ACQUIRE); } test_membarrier_free_percpu_list(&list_a); @@ -1401,7 +1401,7 @@ void test_membarrier(void) } } - atomic_store(&thread_args.stop, 1); + __atomic_store_n(&thread_args.stop, 1, __ATOMIC_RELEASE); ret = pthread_join(manager_thread, NULL); if (ret) { errno = ret; From 1da3f00e5e0c615819448e259b6d0dd012359b52 Mon Sep 17 00:00:00 2001 From: Javier Carrasco Date: Wed, 4 Oct 2023 15:47:41 +0200 Subject: [PATCH 03/51] selftests/user_events: add gitignore file user_events builds a series of binaries that can be ignored by git. Signed-off-by: Javier Carrasco Signed-off-by: Shuah Khan --- tools/testing/selftests/user_events/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 tools/testing/selftests/user_events/.gitignore diff --git a/tools/testing/selftests/user_events/.gitignore b/tools/testing/selftests/user_events/.gitignore new file mode 100644 index 000000000000..f570febd211b --- /dev/null +++ b/tools/testing/selftests/user_events/.gitignore @@ -0,0 +1,4 @@ +abi_test +dyn_test +ftrace_test +perf_test From 7359da1cc2fe10e18d4f9ddcc0786ac89e5a6170 Mon Sep 17 00:00:00 2001 From: Javier Carrasco Date: Wed, 4 Oct 2023 15:47:42 +0200 Subject: [PATCH 04/51] selftests/tdx: add gitignore file tdx builds a tdx_guest_test binary that can be ignored by git. Signed-off-by: Javier Carrasco Signed-off-by: Shuah Khan --- tools/testing/selftests/tdx/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/testing/selftests/tdx/.gitignore diff --git a/tools/testing/selftests/tdx/.gitignore b/tools/testing/selftests/tdx/.gitignore new file mode 100644 index 000000000000..5db4d15cc673 --- /dev/null +++ b/tools/testing/selftests/tdx/.gitignore @@ -0,0 +1 @@ +tdx_guest_test From 876d7fcd1f4c7475b79fc3b923058f5ac7524646 Mon Sep 17 00:00:00 2001 From: Javier Carrasco Date: Wed, 4 Oct 2023 15:47:43 +0200 Subject: [PATCH 05/51] selftests/dmabuf-heaps: add gitignore file dmabuf-heaps builds a dmabuf-heap binary that can be ignored by git. Signed-off-by: Javier Carrasco Signed-off-by: Shuah Khan --- tools/testing/selftests/dmabuf-heaps/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/testing/selftests/dmabuf-heaps/.gitignore diff --git a/tools/testing/selftests/dmabuf-heaps/.gitignore b/tools/testing/selftests/dmabuf-heaps/.gitignore new file mode 100644 index 000000000000..b500e76b9045 --- /dev/null +++ b/tools/testing/selftests/dmabuf-heaps/.gitignore @@ -0,0 +1 @@ +dmabuf-heap From 6d3d638d1865a6e34787c6e20eee1c6d8d32fd40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= Date: Wed, 4 Oct 2023 14:48:36 +0200 Subject: [PATCH 06/51] kbuild: Let builtin have precedence over modules for kselftest-merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kselftest-merge target walks all kselftests configs, and merges them. However, builtin does not have precedence over modules. This breaks some of the tests, e.g.: $ grep CONFIG_NF_NAT tools/testing/selftests/{bpf,net}/config tools/testing/selftests/bpf/config:CONFIG_NF_NAT=y tools/testing/selftests/net/config:CONFIG_NF_NAT=m Here, the net config will set NF_NAT to module, which makes it clunky to run the BPF tests. Add '-y' to scripts/kconfig/merge_config.sh. Signed-off-by: Björn Töpel Signed-off-by: Shuah Khan --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 57698d048e2c..41fc7681fbab 100644 --- a/Makefile +++ b/Makefile @@ -1368,7 +1368,7 @@ PHONY += kselftest-merge kselftest-merge: $(if $(wildcard $(objtree)/.config),, $(error No .config exists, config your kernel first!)) $(Q)find $(srctree)/tools/testing/selftests -name config | \ - xargs $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config + xargs $(srctree)/scripts/kconfig/merge_config.sh -y -m $(objtree)/.config $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig # --------------------------------------------------------------------------- From 37013b557b7f39e603b4459ef82dc8b848f98a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= Date: Wed, 4 Oct 2023 14:48:37 +0200 Subject: [PATCH 07/51] kbuild: Merge per-arch config for kselftest-merge target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some kselftests has a per-arch config, e.g. tools/testing/selftests/bpf/config.s390x. Make sure these configs are picked up by the kselftest-merge target. Signed-off-by: Björn Töpel Signed-off-by: Shuah Khan --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 41fc7681fbab..fd8329474e86 100644 --- a/Makefile +++ b/Makefile @@ -1367,7 +1367,7 @@ kselftest-%: headers FORCE PHONY += kselftest-merge kselftest-merge: $(if $(wildcard $(objtree)/.config),, $(error No .config exists, config your kernel first!)) - $(Q)find $(srctree)/tools/testing/selftests -name config | \ + $(Q)find $(srctree)/tools/testing/selftests -name config -o -name config.$(UTS_MACHINE) | \ xargs $(srctree)/scripts/kconfig/merge_config.sh -y -m $(objtree)/.config $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig From 5b45a753776be5d21cf395ec97e81c9187fbeaca Mon Sep 17 00:00:00 2001 From: Javier Carrasco Date: Sun, 1 Oct 2023 10:37:44 +0200 Subject: [PATCH 08/51] selftests: uevent filtering: fix return on error in uevent_listener The ret variable is used to check function return values and assigning values to it on error has no effect as it is an unused value. The current implementation uses an additional variable (fret) to return the error value, which in this case is unnecessary and lead to the above described misuse. There is no restriction in the current implementation to always return -1 on error and the actual negative error value can be returned safely without storing -1 in a specific variable. Simplify the error checking by using a single variable which always holds the returned value. Signed-off-by: Javier Carrasco Signed-off-by: Shuah Khan --- tools/testing/selftests/uevent/uevent_filtering.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/uevent/uevent_filtering.c b/tools/testing/selftests/uevent/uevent_filtering.c index 5cebfb356345..dbe55f3a66f4 100644 --- a/tools/testing/selftests/uevent/uevent_filtering.c +++ b/tools/testing/selftests/uevent/uevent_filtering.c @@ -78,7 +78,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent, { int sk_fd, ret; socklen_t sk_addr_len; - int fret = -1, rcv_buf_sz = __UEVENT_BUFFER_SIZE; + int rcv_buf_sz = __UEVENT_BUFFER_SIZE; uint64_t sync_add = 1; struct sockaddr_nl sk_addr = { 0 }, rcv_addr = { 0 }; char buf[__UEVENT_BUFFER_SIZE] = { 0 }; @@ -121,6 +121,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent, if ((size_t)sk_addr_len != sizeof(sk_addr)) { fprintf(stderr, "Invalid socket address size\n"); + ret = -1; goto on_error; } @@ -147,11 +148,12 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent, ret = write_nointr(sync_fd, &sync_add, sizeof(sync_add)); close(sync_fd); if (ret != sizeof(sync_add)) { + ret = -1; fprintf(stderr, "Failed to synchronize with parent process\n"); goto on_error; } - fret = 0; + ret = 0; for (;;) { ssize_t r; @@ -187,7 +189,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent, on_error: close(sk_fd); - return fret; + return ret; } int trigger_uevent(unsigned int times) From 1c71a121c7ff41bf6605b2604408d109eab047ae Mon Sep 17 00:00:00 2001 From: Javier Carrasco Date: Sun, 1 Oct 2023 15:18:09 +0200 Subject: [PATCH 09/51] selftests: static_keys: fix test name in messages As a general rule, the name of the selftest is printed at the beginning of every message. Use "static_keys" (name of the test itself) consistently instead of mixing "static_key" and "static_keys" at the beginning of the messages in the test_static_keys script. Signed-off-by: Javier Carrasco Signed-off-by: Shuah Khan --- tools/testing/selftests/static_keys/test_static_keys.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/static_keys/test_static_keys.sh b/tools/testing/selftests/static_keys/test_static_keys.sh index fc9f8cde7d42..3b0f17b81ac2 100755 --- a/tools/testing/selftests/static_keys/test_static_keys.sh +++ b/tools/testing/selftests/static_keys/test_static_keys.sh @@ -6,18 +6,18 @@ ksft_skip=4 if ! /sbin/modprobe -q -n test_static_key_base; then - echo "static_key: module test_static_key_base is not found [SKIP]" + echo "static_keys: module test_static_key_base is not found [SKIP]" exit $ksft_skip fi if ! /sbin/modprobe -q -n test_static_keys; then - echo "static_key: module test_static_keys is not found [SKIP]" + echo "static_keys: module test_static_keys is not found [SKIP]" exit $ksft_skip fi if /sbin/modprobe -q test_static_key_base; then if /sbin/modprobe -q test_static_keys; then - echo "static_key: ok" + echo "static_keys: ok" /sbin/modprobe -q -r test_static_keys /sbin/modprobe -q -r test_static_key_base else @@ -25,6 +25,6 @@ if /sbin/modprobe -q test_static_key_base; then /sbin/modprobe -q -r test_static_key_base fi else - echo "static_key: [FAIL]" + echo "static_keys: [FAIL]" exit 1 fi From 907f33028871fa7c9a3db1efd467b78ef82cce20 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 28 Sep 2023 16:38:11 +0200 Subject: [PATCH 10/51] kselftest: Add a ksft_perror() helper The standard library perror() function provides a convenient way to print an error message based on the current errno but this doesn't play nicely with KTAP output. Provide a helper which does an equivalent thing in a KTAP compatible format. nolibc doesn't have a strerror() and adding the table of strings required doesn't seem like a good fit for what it's trying to do so when we're using that only print the errno. Signed-off-by: Mark Brown Reviewed-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 529d29a35900..af9f1202d423 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -48,6 +48,7 @@ #include #include #include +#include #include #endif @@ -155,6 +156,19 @@ static inline void ksft_print_msg(const char *msg, ...) va_end(args); } +static inline void ksft_perror(const char *msg) +{ +#ifndef NOLIBC + ksft_print_msg("%s: %s (%d)\n", msg, strerror(errno), errno); +#else + /* + * nolibc doesn't provide strerror() and it seems + * inappropriate to add one, just print the errno. + */ + ksft_print_msg("%s: %d)\n", msg, errno); +#endif +} + static inline void ksft_test_result_pass(const char *msg, ...) { int saved_errno = errno; From 47903c1d153d5178d54ea086e2bfa5298506f04f Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 28 Sep 2023 16:38:12 +0200 Subject: [PATCH 11/51] selftests/exec: Convert execveat test to generate KTAP output Currently the execveat test does not produce KTAP output but rather a custom format. This means that we only get a pass/fail for the suite, not for each individual test that the suite does. Convert to using the standard kselftest output functions which result in KTAP output being generated. The main trick with this is that, being an exec() related test, the program executes itself and returns specific exit codes to verify success meaning that we need to only use the top level kselftest header/summary functions when invoked directly rather than when run as part of a test. Signed-off-by: Mark Brown Reviewed-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/exec/execveat.c | 87 +++++++++++++++---------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c index 67bf7254a48f..bf79d664c8e6 100644 --- a/tools/testing/selftests/exec/execveat.c +++ b/tools/testing/selftests/exec/execveat.c @@ -23,6 +23,9 @@ #include "../kselftest.h" +#define TESTS_EXPECTED 51 +#define TEST_NAME_LEN (PATH_MAX * 4) + static char longpath[2 * PATH_MAX] = ""; static char *envp[] = { "IN_TEST=yes", NULL, NULL }; static char *argv[] = { "execveat", "99", NULL }; @@ -43,71 +46,85 @@ static int execveat_(int fd, const char *path, char **argv, char **envp, static int _check_execveat_fail(int fd, const char *path, int flags, int expected_errno, const char *errno_str) { + char test_name[TEST_NAME_LEN]; int rc; errno = 0; - printf("Check failure of execveat(%d, '%s', %d) with %s... ", - fd, path?:"(null)", flags, errno_str); + snprintf(test_name, sizeof(test_name), + "Check failure of execveat(%d, '%s', %d) with %s", + fd, path?:"(null)", flags, errno_str); rc = execveat_(fd, path, argv, envp, flags); if (rc > 0) { - printf("[FAIL] (unexpected success from execveat(2))\n"); + ksft_print_msg("unexpected success from execveat(2)\n"); + ksft_test_result_fail("%s\n", test_name); return 1; } if (errno != expected_errno) { - printf("[FAIL] (expected errno %d (%s) not %d (%s)\n", - expected_errno, strerror(expected_errno), - errno, strerror(errno)); + ksft_print_msg("expected errno %d (%s) not %d (%s)\n", + expected_errno, strerror(expected_errno), + errno, strerror(errno)); + ksft_test_result_fail("%s\n", test_name); return 1; } - printf("[OK]\n"); + ksft_test_result_pass("%s\n", test_name); return 0; } static int check_execveat_invoked_rc(int fd, const char *path, int flags, int expected_rc, int expected_rc2) { + char test_name[TEST_NAME_LEN]; int status; int rc; pid_t child; int pathlen = path ? strlen(path) : 0; if (pathlen > 40) - printf("Check success of execveat(%d, '%.20s...%s', %d)... ", - fd, path, (path + pathlen - 20), flags); + snprintf(test_name, sizeof(test_name), + "Check success of execveat(%d, '%.20s...%s', %d)... ", + fd, path, (path + pathlen - 20), flags); else - printf("Check success of execveat(%d, '%s', %d)... ", - fd, path?:"(null)", flags); + snprintf(test_name, sizeof(test_name), + "Check success of execveat(%d, '%s', %d)... ", + fd, path?:"(null)", flags); + child = fork(); if (child < 0) { - printf("[FAIL] (fork() failed)\n"); + ksft_perror("fork() failed"); + ksft_test_result_fail("%s\n", test_name); return 1; } if (child == 0) { /* Child: do execveat(). */ rc = execveat_(fd, path, argv, envp, flags); - printf("[FAIL]: execveat() failed, rc=%d errno=%d (%s)\n", - rc, errno, strerror(errno)); + ksft_print_msg("execveat() failed, rc=%d errno=%d (%s)\n", + rc, errno, strerror(errno)); + ksft_test_result_fail("%s\n", test_name); exit(1); /* should not reach here */ } /* Parent: wait for & check child's exit status. */ rc = waitpid(child, &status, 0); if (rc != child) { - printf("[FAIL] (waitpid(%d,...) returned %d)\n", child, rc); + ksft_print_msg("waitpid(%d,...) returned %d\n", child, rc); + ksft_test_result_fail("%s\n", test_name); return 1; } if (!WIFEXITED(status)) { - printf("[FAIL] (child %d did not exit cleanly, status=%08x)\n", - child, status); + ksft_print_msg("child %d did not exit cleanly, status=%08x\n", + child, status); + ksft_test_result_fail("%s\n", test_name); return 1; } if ((WEXITSTATUS(status) != expected_rc) && (WEXITSTATUS(status) != expected_rc2)) { - printf("[FAIL] (child %d exited with %d not %d nor %d)\n", - child, WEXITSTATUS(status), expected_rc, expected_rc2); + ksft_print_msg("child %d exited with %d not %d nor %d\n", + child, WEXITSTATUS(status), expected_rc, + expected_rc2); + ksft_test_result_fail("%s\n", test_name); return 1; } - printf("[OK]\n"); + ksft_test_result_pass("%s\n", test_name); return 0; } @@ -129,11 +146,9 @@ static int open_or_die(const char *filename, int flags) { int fd = open(filename, flags); - if (fd < 0) { - printf("Failed to open '%s'; " + if (fd < 0) + ksft_exit_fail_msg("Failed to open '%s'; " "check prerequisites are available\n", filename); - exit(1); - } return fd; } @@ -162,8 +177,7 @@ static int check_execveat_pathmax(int root_dfd, const char *src, int is_script) char *cwd = getcwd(NULL, 0); if (!cwd) { - printf("Failed to getcwd(), errno=%d (%s)\n", - errno, strerror(errno)); + ksft_perror("Failed to getcwd()"); return 2; } strcpy(longpath, cwd); @@ -193,12 +207,12 @@ static int check_execveat_pathmax(int root_dfd, const char *src, int is_script) */ fd = open(longpath, O_RDONLY); if (fd > 0) { - printf("Invoke copy of '%s' via filename of length %zu:\n", - src, strlen(longpath)); + ksft_print_msg("Invoke copy of '%s' via filename of length %zu:\n", + src, strlen(longpath)); fail += check_execveat(fd, "", AT_EMPTY_PATH); } else { - printf("Failed to open length %zu filename, errno=%d (%s)\n", - strlen(longpath), errno, strerror(errno)); + ksft_print_msg("Failed to open length %zu filename, errno=%d (%s)\n", + strlen(longpath), errno, strerror(errno)); fail++; } @@ -405,28 +419,31 @@ int main(int argc, char **argv) const char *in_test = getenv("IN_TEST"); if (verbose) { - printf(" invoked with:"); + ksft_print_msg("invoked with:\n"); for (ii = 0; ii < argc; ii++) - printf(" [%d]='%s'", ii, argv[ii]); - printf("\n"); + ksft_print_msg("\t[%d]='%s\n'", ii, argv[ii]); } /* Check expected environment transferred. */ if (!in_test || strcmp(in_test, "yes") != 0) { - printf("[FAIL] (no IN_TEST=yes in env)\n"); + ksft_print_msg("no IN_TEST=yes in env\n"); return 1; } /* Use the final argument as an exit code. */ rc = atoi(argv[argc - 1]); - fflush(stdout); + exit(rc); } else { + ksft_print_header(); + ksft_set_plan(TESTS_EXPECTED); prerequisites(); if (verbose) envp[1] = "VERBOSE=1"; rc = run_tests(); if (rc > 0) printf("%d tests failed\n", rc); + ksft_finished(); } + return rc; } From 071af0c9e582bc47e379e39490a2bc1adfe4ec68 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Wed, 27 Sep 2023 12:18:58 +0200 Subject: [PATCH 12/51] selftests: timers: Convert posix_timers test to generate KTAP output Currently the posix_timers test does not produce KTAP output but rather a custom format. This means that we only get a pass/fail for the suite, not for each individual test that the suite does. Convert to using the standard kselftest output functions which result in KTAP output being generated. As part of this fix the printing of diagnostics in the unlikely event that the pthread APIs fail, these were using perror() but the API functions directly return an error code instead of setting errno. Signed-off-by: Mark Brown Signed-off-by: Shuah Khan --- tools/testing/selftests/timers/posix_timers.c | 81 ++++++++++--------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c index 8a17c0e8d82b..d49dd3ffd0d9 100644 --- a/tools/testing/selftests/timers/posix_timers.c +++ b/tools/testing/selftests/timers/posix_timers.c @@ -76,22 +76,21 @@ static int check_diff(struct timeval start, struct timeval end) static int check_itimer(int which) { + const char *name; int err; struct timeval start, end; struct itimerval val = { .it_value.tv_sec = DELAY, }; - printf("Check itimer "); - if (which == ITIMER_VIRTUAL) - printf("virtual... "); + name = "ITIMER_VIRTUAL"; else if (which == ITIMER_PROF) - printf("prof... "); + name = "ITIMER_PROF"; else if (which == ITIMER_REAL) - printf("real... "); - - fflush(stdout); + name = "ITIMER_REAL"; + else + return -1; done = 0; @@ -104,13 +103,13 @@ static int check_itimer(int which) err = gettimeofday(&start, NULL); if (err < 0) { - perror("Can't call gettimeofday()\n"); + ksft_perror("Can't call gettimeofday()"); return -1; } err = setitimer(which, &val, NULL); if (err < 0) { - perror("Can't set timer\n"); + ksft_perror("Can't set timer"); return -1; } @@ -123,20 +122,18 @@ static int check_itimer(int which) err = gettimeofday(&end, NULL); if (err < 0) { - perror("Can't call gettimeofday()\n"); + ksft_perror("Can't call gettimeofday()"); return -1; } - if (!check_diff(start, end)) - printf("[OK]\n"); - else - printf("[FAIL]\n"); + ksft_test_result(check_diff(start, end) == 0, "%s\n", name); return 0; } static int check_timer_create(int which) { + const char *type; int err; timer_t id; struct timeval start, end; @@ -144,31 +141,32 @@ static int check_timer_create(int which) .it_value.tv_sec = DELAY, }; - printf("Check timer_create() "); if (which == CLOCK_THREAD_CPUTIME_ID) { - printf("per thread... "); + type = "thread"; } else if (which == CLOCK_PROCESS_CPUTIME_ID) { - printf("per process... "); + type = "process"; + } else { + ksft_print_msg("Unknown timer_create() type %d\n", which); + return -1; } - fflush(stdout); done = 0; err = timer_create(which, NULL, &id); if (err < 0) { - perror("Can't create timer\n"); + ksft_perror("Can't create timer"); return -1; } signal(SIGALRM, sig_handler); err = gettimeofday(&start, NULL); if (err < 0) { - perror("Can't call gettimeofday()\n"); + ksft_perror("Can't call gettimeofday()"); return -1; } err = timer_settime(id, 0, &val, NULL); if (err < 0) { - perror("Can't set timer\n"); + ksft_perror("Can't set timer"); return -1; } @@ -176,14 +174,12 @@ static int check_timer_create(int which) err = gettimeofday(&end, NULL); if (err < 0) { - perror("Can't call gettimeofday()\n"); + ksft_perror("Can't call gettimeofday()"); return -1; } - if (!check_diff(start, end)) - printf("[OK]\n"); - else - printf("[FAIL]\n"); + ksft_test_result(check_diff(start, end) == 0, + "timer_create() per %s\n", type); return 0; } @@ -220,25 +216,25 @@ static int check_timer_distribution(void) .it_interval.tv_nsec = 1000 * 1000, }; - printf("Check timer_create() per process signal distribution... "); - fflush(stdout); - remain = nthreads + 1; /* worker threads + this thread */ signal(SIGALRM, distribution_handler); err = timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id); if (err < 0) { - perror("Can't create timer\n"); + ksft_perror("Can't create timer"); return -1; } err = timer_settime(id, 0, &val, NULL); if (err < 0) { - perror("Can't set timer\n"); + ksft_perror("Can't set timer"); return -1; } for (i = 0; i < nthreads; i++) { - if (pthread_create(&threads[i], NULL, distribution_thread, NULL)) { - perror("Can't create thread\n"); + err = pthread_create(&threads[i], NULL, distribution_thread, + NULL); + if (err) { + ksft_print_msg("Can't create thread: %s (%d)\n", + strerror(errno), errno); return -1; } } @@ -247,25 +243,30 @@ static int check_timer_distribution(void) while (__atomic_load_n(&remain, __ATOMIC_RELAXED)); for (i = 0; i < nthreads; i++) { - if (pthread_join(threads[i], NULL)) { - perror("Can't join thread\n"); + err = pthread_join(threads[i], NULL); + if (err) { + ksft_print_msg("Can't join thread: %s (%d)\n", + strerror(errno), errno); return -1; } } if (timer_delete(id)) { - perror("Can't delete timer\n"); + ksft_perror("Can't delete timer"); return -1; } - printf("[OK]\n"); + ksft_test_result_pass("check_timer_distribution\n"); return 0; } int main(int argc, char **argv) { - printf("Testing posix timers. False negative may happen on CPU execution \n"); - printf("based timers if other threads run on the CPU...\n"); + ksft_print_header(); + ksft_set_plan(6); + + ksft_print_msg("Testing posix timers. False negative may happen on CPU execution \n"); + ksft_print_msg("based timers if other threads run on the CPU...\n"); if (check_itimer(ITIMER_VIRTUAL) < 0) return ksft_exit_fail(); @@ -294,5 +295,5 @@ int main(int argc, char **argv) if (check_timer_distribution() < 0) return ksft_exit_fail(); - return ksft_exit_pass(); + ksft_finished(); } From 0a6fa8f03e1497d3d1bf2e55f1e5b082dda227d4 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Wed, 27 Sep 2023 12:18:59 +0200 Subject: [PATCH 13/51] selftests: timers: Convert nsleep-lat test to generate KTAP output Currently the nsleep-lat test does not produce KTAP output but rather a custom format. This means that we only get a pass/fail for the suite, not for each individual test that the suite does. Convert to using the standard kselftest output functions which result in KTAP output being generated. Signed-off-by: Mark Brown Signed-off-by: Shuah Khan --- tools/testing/selftests/timers/nsleep-lat.c | 26 ++++++++++----------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/tools/testing/selftests/timers/nsleep-lat.c b/tools/testing/selftests/timers/nsleep-lat.c index eb3e79ed7b4a..edb5acacf214 100644 --- a/tools/testing/selftests/timers/nsleep-lat.c +++ b/tools/testing/selftests/timers/nsleep-lat.c @@ -118,7 +118,7 @@ int nanosleep_lat_test(int clockid, long long ns) clock_gettime(clockid, &end); if (((timespec_sub(start, end)/count)-ns) > UNRESONABLE_LATENCY) { - printf("Large rel latency: %lld ns :", (timespec_sub(start, end)/count)-ns); + ksft_print_msg("Large rel latency: %lld ns :", (timespec_sub(start, end)/count)-ns); return -1; } @@ -132,20 +132,23 @@ int nanosleep_lat_test(int clockid, long long ns) } if (latency/count > UNRESONABLE_LATENCY) { - printf("Large abs latency: %lld ns :", latency/count); + ksft_print_msg("Large abs latency: %lld ns :", latency/count); return -1; } return 0; } - +#define SKIPPED_CLOCK_COUNT 3 int main(int argc, char **argv) { long long length; int clockid, ret; + ksft_print_header(); + ksft_set_plan(NR_CLOCKIDS - CLOCK_REALTIME - SKIPPED_CLOCK_COUNT); + for (clockid = CLOCK_REALTIME; clockid < NR_CLOCKIDS; clockid++) { /* Skip cputime clockids since nanosleep won't increment cputime */ @@ -154,9 +157,6 @@ int main(int argc, char **argv) clockid == CLOCK_HWSPECIFIC) continue; - printf("nsleep latency %-26s ", clockstring(clockid)); - fflush(stdout); - length = 10; while (length <= (NSEC_PER_SEC * 10)) { ret = nanosleep_lat_test(clockid, length); @@ -167,14 +167,12 @@ int main(int argc, char **argv) } if (ret == UNSUPPORTED) { - printf("[UNSUPPORTED]\n"); - continue; + ksft_test_result_skip("%s\n", clockstring(clockid)); + } else { + ksft_test_result(ret >= 0, "%s\n", + clockstring(clockid)); } - if (ret < 0) { - printf("[FAILED]\n"); - return ksft_exit_fail(); - } - printf("[OK]\n"); } - return ksft_exit_pass(); + + ksft_finished(); } From dfc033059bb6aa7d219fc25d113b3886131b0a6c Mon Sep 17 00:00:00 2001 From: Muhammad Usama Anjum Date: Fri, 6 Oct 2023 15:07:35 +0500 Subject: [PATCH 14/51] selftests: capabilities: remove duplicate unneeded defines These duplicate defines should automatically be picked up from kernel headers. Use KHDR_INCLUDES to add kernel header files. Signed-off-by: Muhammad Usama Anjum Signed-off-by: Shuah Khan --- tools/testing/selftests/capabilities/Makefile | 2 +- tools/testing/selftests/capabilities/test_execve.c | 8 -------- tools/testing/selftests/capabilities/validate_cap.c | 8 -------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/tools/testing/selftests/capabilities/Makefile b/tools/testing/selftests/capabilities/Makefile index 6e9d98d457d5..411ac098308f 100644 --- a/tools/testing/selftests/capabilities/Makefile +++ b/tools/testing/selftests/capabilities/Makefile @@ -2,7 +2,7 @@ TEST_GEN_FILES := validate_cap TEST_GEN_PROGS := test_execve -CFLAGS += -O2 -g -std=gnu99 -Wall +CFLAGS += -O2 -g -std=gnu99 -Wall $(KHDR_INCLUDES) LDLIBS += -lcap-ng -lrt -ldl include ../lib.mk diff --git a/tools/testing/selftests/capabilities/test_execve.c b/tools/testing/selftests/capabilities/test_execve.c index df0ef02b4036..e3a352b020a7 100644 --- a/tools/testing/selftests/capabilities/test_execve.c +++ b/tools/testing/selftests/capabilities/test_execve.c @@ -20,14 +20,6 @@ #include "../kselftest.h" -#ifndef PR_CAP_AMBIENT -#define PR_CAP_AMBIENT 47 -# define PR_CAP_AMBIENT_IS_SET 1 -# define PR_CAP_AMBIENT_RAISE 2 -# define PR_CAP_AMBIENT_LOWER 3 -# define PR_CAP_AMBIENT_CLEAR_ALL 4 -#endif - static int nerrs; static pid_t mpid; /* main() pid is used to avoid duplicate test counts */ diff --git a/tools/testing/selftests/capabilities/validate_cap.c b/tools/testing/selftests/capabilities/validate_cap.c index cdfc94268fe6..60b4e7b716a7 100644 --- a/tools/testing/selftests/capabilities/validate_cap.c +++ b/tools/testing/selftests/capabilities/validate_cap.c @@ -9,14 +9,6 @@ #include "../kselftest.h" -#ifndef PR_CAP_AMBIENT -#define PR_CAP_AMBIENT 47 -# define PR_CAP_AMBIENT_IS_SET 1 -# define PR_CAP_AMBIENT_RAISE 2 -# define PR_CAP_AMBIENT_LOWER 3 -# define PR_CAP_AMBIENT_CLEAR_ALL 4 -#endif - #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 19) # define HAVE_GETAUXVAL #endif From 44eebacd6b8f633eb3a38a6db093658636b844e8 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Anjum Date: Fri, 6 Oct 2023 15:07:36 +0500 Subject: [PATCH 15/51] selftests: clone3: remove duplicate defines Remove duplicate defines which are already included in kernel headers. MAX_PID_NS_LEVEL macro is used inside kernel only. It isn't exposed to userspace. So it is never defined in test application. Remove #ifndef in this case. Signed-off-by: Muhammad Usama Anjum Signed-off-by: Shuah Khan --- .../clone3/clone3_cap_checkpoint_restore.c | 2 -- .../testing/selftests/clone3/clone3_clear_sighand.c | 4 ---- tools/testing/selftests/clone3/clone3_selftests.h | 13 ------------- tools/testing/selftests/clone3/clone3_set_tid.c | 2 -- 4 files changed, 21 deletions(-) diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c index 52d3f0364bda..31b56d625655 100644 --- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c +++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c @@ -27,9 +27,7 @@ #include "../kselftest_harness.h" #include "clone3_selftests.h" -#ifndef MAX_PID_NS_LEVEL #define MAX_PID_NS_LEVEL 32 -#endif static void child_exit(int ret) { diff --git a/tools/testing/selftests/clone3/clone3_clear_sighand.c b/tools/testing/selftests/clone3/clone3_clear_sighand.c index 47a8c0fc3676..54a8b2445be9 100644 --- a/tools/testing/selftests/clone3/clone3_clear_sighand.c +++ b/tools/testing/selftests/clone3/clone3_clear_sighand.c @@ -16,10 +16,6 @@ #include "../kselftest.h" #include "clone3_selftests.h" -#ifndef CLONE_CLEAR_SIGHAND -#define CLONE_CLEAR_SIGHAND 0x100000000ULL -#endif - static void nop_handler(int signo) { } diff --git a/tools/testing/selftests/clone3/clone3_selftests.h b/tools/testing/selftests/clone3/clone3_selftests.h index e81ffaaee02b..3d2663fe50ba 100644 --- a/tools/testing/selftests/clone3/clone3_selftests.h +++ b/tools/testing/selftests/clone3/clone3_selftests.h @@ -15,10 +15,6 @@ #define ptr_to_u64(ptr) ((__u64)((uintptr_t)(ptr))) -#ifndef CLONE_INTO_CGROUP -#define CLONE_INTO_CGROUP 0x200000000ULL /* Clone into a specific cgroup given the right permissions. */ -#endif - #ifndef __NR_clone3 #define __NR_clone3 -1 #endif @@ -32,18 +28,9 @@ struct __clone_args { __aligned_u64 stack; __aligned_u64 stack_size; __aligned_u64 tls; -#ifndef CLONE_ARGS_SIZE_VER0 -#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ -#endif __aligned_u64 set_tid; __aligned_u64 set_tid_size; -#ifndef CLONE_ARGS_SIZE_VER1 -#define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */ -#endif __aligned_u64 cgroup; -#ifndef CLONE_ARGS_SIZE_VER2 -#define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */ -#endif }; static pid_t sys_clone3(struct __clone_args *args, size_t size) diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c index 0229e9ebb995..ed785afb6077 100644 --- a/tools/testing/selftests/clone3/clone3_set_tid.c +++ b/tools/testing/selftests/clone3/clone3_set_tid.c @@ -23,9 +23,7 @@ #include "../kselftest.h" #include "clone3_selftests.h" -#ifndef MAX_PID_NS_LEVEL #define MAX_PID_NS_LEVEL 32 -#endif static int pipe_1[2]; static int pipe_2[2]; From ec54424923cf943b51dd5bf75fcbe27b0ca2c6ef Mon Sep 17 00:00:00 2001 From: Muhammad Usama Anjum Date: Fri, 6 Oct 2023 15:07:37 +0500 Subject: [PATCH 16/51] selftests: core: remove duplicate defines Remove duplicate defines which are already defined in kernel headers and re-definition isn't required. Signed-off-by: Muhammad Usama Anjum Signed-off-by: Shuah Khan --- .../testing/selftests/core/close_range_test.c | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/tools/testing/selftests/core/close_range_test.c b/tools/testing/selftests/core/close_range_test.c index 749239930ca8..534576f06df1 100644 --- a/tools/testing/selftests/core/close_range_test.c +++ b/tools/testing/selftests/core/close_range_test.c @@ -16,34 +16,6 @@ #include "../kselftest_harness.h" #include "../clone3/clone3_selftests.h" -#ifndef __NR_close_range - #if defined __alpha__ - #define __NR_close_range 546 - #elif defined _MIPS_SIM - #if _MIPS_SIM == _MIPS_SIM_ABI32 /* o32 */ - #define __NR_close_range (436 + 4000) - #endif - #if _MIPS_SIM == _MIPS_SIM_NABI32 /* n32 */ - #define __NR_close_range (436 + 6000) - #endif - #if _MIPS_SIM == _MIPS_SIM_ABI64 /* n64 */ - #define __NR_close_range (436 + 5000) - #endif - #elif defined __ia64__ - #define __NR_close_range (436 + 1024) - #else - #define __NR_close_range 436 - #endif -#endif - -#ifndef CLOSE_RANGE_UNSHARE -#define CLOSE_RANGE_UNSHARE (1U << 1) -#endif - -#ifndef CLOSE_RANGE_CLOEXEC -#define CLOSE_RANGE_CLOEXEC (1U << 2) -#endif - static inline int sys_close_range(unsigned int fd, unsigned int max_fd, unsigned int flags) { From f1020c687153609f246f3314db5b74821025c185 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Anjum Date: Fri, 6 Oct 2023 15:07:38 +0500 Subject: [PATCH 17/51] selftests: firmware: remove duplicate unneeded defines These duplicate defines should automatically be picked up from kernel headers. Signed-off-by: Muhammad Usama Anjum Signed-off-by: Shuah Khan --- tools/testing/selftests/firmware/fw_namespace.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_namespace.c b/tools/testing/selftests/firmware/fw_namespace.c index 4c6f0cd83c5b..04757dc7e546 100644 --- a/tools/testing/selftests/firmware/fw_namespace.c +++ b/tools/testing/selftests/firmware/fw_namespace.c @@ -17,10 +17,6 @@ #include #include -#ifndef CLONE_NEWNS -# define CLONE_NEWNS 0x00020000 -#endif - static char *fw_path = NULL; static void die(char *fmt, ...) From 2531f374f922e77ba51f24d1aa6fa11c7f4c36b8 Mon Sep 17 00:00:00 2001 From: Marcos Paulo de Souza Date: Mon, 9 Oct 2023 12:09:29 -0300 Subject: [PATCH 18/51] Documentation: kselftests: Remove references to bpf tests Currently the bpf selftests are skipped by default, so is someone would like to run the tests one would need to run: $ make TARGETS=bpf SKIP_TARGETS="" kselftest To overwrite the SKIP_TARGETS that defines bpf by default. Also, following the BPF instructions[1], to run the bpf selftests one would need to enter in the tools/testing/selftests/bpf/ directory, and then run make, which is not the standard way to run selftests per it's documentation. For the reasons above stop mentioning bpf in the kselftests as examples of how to run a test suite. [1]: Documentation/bpf/bpf_devel_QA.rst Signed-off-by: Marcos Paulo de Souza Signed-off-by: Shuah Khan --- Documentation/dev-tools/kselftest.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index deede972f254..ab376b316c36 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -112,7 +112,7 @@ You can specify multiple tests to skip:: You can also specify a restricted list of tests to run together with a dedicated skiplist:: - $ make TARGETS="bpf breakpoints size timers" SKIP_TARGETS=bpf kselftest + $ make TARGETS="breakpoints size timers" SKIP_TARGETS=size kselftest See the top-level tools/testing/selftests/Makefile for the list of all possible targets. @@ -165,7 +165,7 @@ To see the list of available tests, the `-l` option can be used:: The `-c` option can be used to run all the tests from a test collection, or the `-t` option for specific single tests. Either can be used multiple times:: - $ ./run_kselftest.sh -c bpf -c seccomp -t timers:posix_timers -t timer:nanosleep + $ ./run_kselftest.sh -c size -c seccomp -t timers:posix_timers -t timer:nanosleep For other features see the script usage output, seen with the `-h` option. @@ -210,7 +210,7 @@ option is supported, such as:: tests by using variables specified in `Running a subset of selftests`_ section:: - $ make -C tools/testing/selftests gen_tar TARGETS="bpf" FORMAT=.xz + $ make -C tools/testing/selftests gen_tar TARGETS="size" FORMAT=.xz .. _tar's auto-compress: https://www.gnu.org/software/tar/manual/html_node/gzip.html#auto_002dcompress From e33a02ed6a4fbadfa7813831f1f52874c991d324 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:25 +0200 Subject: [PATCH 19/51] selftests: Add printf attribute to kselftest prints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kselftest header defines multiple variadic functions that use printf along with other logic. There is no format checking for the variadic functions that use printing inside kselftest.h. Because of this the compiler won't be able to catch instances of mismatched printf formats and debugging tests might be more difficult. Add the common __printf() attribute macro to kselftest.h. Add __printf() attribute to every function using formatted printing with variadic arguments. Adding the attribute and compiling all selftests exposes a number of -Wformat warnings which were previously unnoticed due to a lack of format specifiers checking by the compiler. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index af9f1202d423..5696199c16f9 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -78,6 +78,8 @@ #define KSFT_XPASS 3 #define KSFT_SKIP 4 +#define __printf(a, b) __attribute__((format(printf, a, b))) + /* counters */ struct ksft_count { unsigned int ksft_pass; @@ -144,7 +146,7 @@ static inline void ksft_print_cnts(void) ksft_cnt.ksft_xskip, ksft_cnt.ksft_error); } -static inline void ksft_print_msg(const char *msg, ...) +static inline __printf(1, 2) void ksft_print_msg(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -169,7 +171,7 @@ static inline void ksft_perror(const char *msg) #endif } -static inline void ksft_test_result_pass(const char *msg, ...) +static inline __printf(1, 2) void ksft_test_result_pass(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -183,7 +185,7 @@ static inline void ksft_test_result_pass(const char *msg, ...) va_end(args); } -static inline void ksft_test_result_fail(const char *msg, ...) +static inline __printf(1, 2) void ksft_test_result_fail(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -209,7 +211,7 @@ static inline void ksft_test_result_fail(const char *msg, ...) ksft_test_result_fail(fmt, ##__VA_ARGS__);\ } while (0) -static inline void ksft_test_result_xfail(const char *msg, ...) +static inline __printf(1, 2) void ksft_test_result_xfail(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -223,7 +225,7 @@ static inline void ksft_test_result_xfail(const char *msg, ...) va_end(args); } -static inline void ksft_test_result_skip(const char *msg, ...) +static inline __printf(1, 2) void ksft_test_result_skip(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -238,7 +240,7 @@ static inline void ksft_test_result_skip(const char *msg, ...) } /* TODO: how does "error" differ from "fail" or "skip"? */ -static inline void ksft_test_result_error(const char *msg, ...) +static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -285,7 +287,7 @@ static inline int ksft_exit_fail(void) ksft_cnt.ksft_xfail + \ ksft_cnt.ksft_xskip) -static inline int ksft_exit_fail_msg(const char *msg, ...) +static inline __printf(1, 2) int ksft_exit_fail_msg(const char *msg, ...) { int saved_errno = errno; va_list args; @@ -312,7 +314,7 @@ static inline int ksft_exit_xpass(void) exit(KSFT_XPASS); } -static inline int ksft_exit_skip(const char *msg, ...) +static inline __printf(1, 2) int ksft_exit_skip(const char *msg, ...) { int saved_errno = errno; va_list args; From 3aa779a9d141f65adbe519653b6aee7188c49e27 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:26 +0200 Subject: [PATCH 20/51] selftests/cachestat: Fix print_cachestat format Compiling cachestat selftest after adding a __printf() attribute to ksft_print_msg() exposes a -Wformat warning in print_cachestat(). The format specifier in printf() call expects long int variables and received long long int. Change format specifiers to long long int so they match passed variables. Signed-off-by: Maciej Wieczor-Retman Acked-by: Nhat Pham Signed-off-by: Shuah Khan --- tools/testing/selftests/cachestat/test_cachestat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c index 4804c7dc7b31..b171fd53b004 100644 --- a/tools/testing/selftests/cachestat/test_cachestat.c +++ b/tools/testing/selftests/cachestat/test_cachestat.c @@ -27,7 +27,7 @@ static const char * const dev_files[] = { void print_cachestat(struct cachestat *cs) { ksft_print_msg( - "Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n", + "Using cachestat: Cached: %llu, Dirty: %llu, Writeback: %llu, Evicted: %llu, Recently Evicted: %llu\n", cs->nr_cache, cs->nr_dirty, cs->nr_writeback, cs->nr_evicted, cs->nr_recently_evicted); } From 287d29827ffc97e6978fa030b7412330a93dd38a Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:27 +0200 Subject: [PATCH 21/51] selftests/openat2: Fix wrong format specifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiling openat2 selftest after adding a __printf() attribute to ksft_print_msg() exposes a -Wformat warning in test_openat2_flags(). The wrong format specifier is used for printing test.how->flags variable. Change the format specifier to %llX so it matches the printed variable. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Signed-off-by: Shuah Khan --- tools/testing/selftests/openat2/openat2_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c index 7fb902099de4..9024754530b2 100644 --- a/tools/testing/selftests/openat2/openat2_test.c +++ b/tools/testing/selftests/openat2/openat2_test.c @@ -300,7 +300,7 @@ void test_openat2_flags(void) ksft_print_msg("openat2 unexpectedly returned "); if (fdpath) - ksft_print_msg("%d['%s'] with %X (!= %X)\n", + ksft_print_msg("%d['%s'] with %X (!= %llX)\n", fd, fdpath, fdflags, test->how.flags); else From 4d7f4e8158b62f63031510cdc24acc520956c091 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:28 +0200 Subject: [PATCH 22/51] selftests/pidfd: Fix ksft print formats Compiling pidfd selftest after adding a __printf() attribute to ksft_print_msg() and ksft_test_result_pass() exposes -Wformat warnings in error_report(), test_pidfd_poll_exec_thread(), child_poll_exec_test(), test_pidfd_poll_leader_exit_thread(), child_poll_leader_exit_test(). The ksft_test_result_pass() in error_report() expects a string but doesn't provide any argument after the format string. All the other calls to ksft_print_msg() in the functions mentioned above have format strings that don't match with other passed arguments. Fix format specifiers so they match the passed variables. Add a missing variable to ksft_test_result_pass() inside error_report() so it matches other cases in the switch statement. Fixes: 2def297ec7fb ("pidfd: add tests for NSpid info in fdinfo") Signed-off-by: Maciej Wieczor-Retman Signed-off-by: Shuah Khan --- tools/testing/selftests/pidfd/pidfd_fdinfo_test.c | 2 +- tools/testing/selftests/pidfd/pidfd_test.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index 4e86f927880c..01cc37bf611c 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -62,7 +62,7 @@ static void error_report(struct error *err, const char *test_name) break; case PIDFD_PASS: - ksft_test_result_pass("%s test: Passed\n"); + ksft_test_result_pass("%s test: Passed\n", test_name); break; default: diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 00a07e7c571c..c081ae91313a 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -381,13 +381,13 @@ static int test_pidfd_send_signal_syscall_support(void) static void *test_pidfd_poll_exec_thread(void *priv) { - ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n", + ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n", getpid(), syscall(SYS_gettid)); ksft_print_msg("Child Thread: doing exec of sleep\n"); execl("/bin/sleep", "sleep", str(CHILD_THREAD_MIN_WAIT), (char *)NULL); - ksft_print_msg("Child Thread: DONE. pid %d tid %d\n", + ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n", getpid(), syscall(SYS_gettid)); return NULL; } @@ -427,7 +427,7 @@ static int child_poll_exec_test(void *args) { pthread_t t1; - ksft_print_msg("Child (pidfd): starting. pid %d tid %d\n", getpid(), + ksft_print_msg("Child (pidfd): starting. pid %d tid %ld\n", getpid(), syscall(SYS_gettid)); pthread_create(&t1, NULL, test_pidfd_poll_exec_thread, NULL); /* @@ -480,10 +480,10 @@ static void test_pidfd_poll_exec(int use_waitpid) static void *test_pidfd_poll_leader_exit_thread(void *priv) { - ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n", + ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n", getpid(), syscall(SYS_gettid)); sleep(CHILD_THREAD_MIN_WAIT); - ksft_print_msg("Child Thread: DONE. pid %d tid %d\n", getpid(), syscall(SYS_gettid)); + ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n", getpid(), syscall(SYS_gettid)); return NULL; } @@ -492,7 +492,7 @@ static int child_poll_leader_exit_test(void *args) { pthread_t t1, t2; - ksft_print_msg("Child: starting. pid %d tid %d\n", getpid(), syscall(SYS_gettid)); + ksft_print_msg("Child: starting. pid %d tid %ld\n", getpid(), syscall(SYS_gettid)); pthread_create(&t1, NULL, test_pidfd_poll_leader_exit_thread, NULL); pthread_create(&t2, NULL, test_pidfd_poll_leader_exit_thread, NULL); From a8cfb036115cf840978dd9c384a5cf30899e14b2 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:29 +0200 Subject: [PATCH 23/51] selftests/sigaltstack: Fix wrong format specifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiling sigaltstack selftest after adding a __printf() attribute to ksft_print_msg() exposes -Wformat warning in main(). The format specifier inside ksft_print_msg() expects a long unsigned int but the passed variable is of unsigned int type. Fix the format specifier so it matches the passed variable. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Signed-off-by: Shuah Khan --- tools/testing/selftests/sigaltstack/sas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c index 98d37cb744fb..07227fab1cc9 100644 --- a/tools/testing/selftests/sigaltstack/sas.c +++ b/tools/testing/selftests/sigaltstack/sas.c @@ -111,7 +111,7 @@ int main(void) /* Make sure more than the required minimum. */ stack_size = getauxval(AT_MINSIGSTKSZ) + SIGSTKSZ; - ksft_print_msg("[NOTE]\tthe stack size is %lu\n", stack_size); + ksft_print_msg("[NOTE]\tthe stack size is %u\n", stack_size); ksft_print_header(); ksft_set_plan(3); From 07bd3c38809216e2c32bd479fd489143e72deb1f Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:30 +0200 Subject: [PATCH 24/51] selftests/kvm: Replace attribute with macro The __printf() macro is used in many tools in the linux kernel to validate the format specifiers in functions that use printf. The kvm selftest uses it without putting it in a macro definition while it also imports the kselftests.h header where the macro attribute is defined. Use __printf() from kselftests.h instead of the full attribute. Signed-off-by: Maciej Wieczor-Retman Signed-off-by: Shuah Khan --- tools/testing/selftests/kvm/include/test_util.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h index 7e614adc6cf4..8e5f413a593d 100644 --- a/tools/testing/selftests/kvm/include/test_util.h +++ b/tools/testing/selftests/kvm/include/test_util.h @@ -33,7 +33,7 @@ static inline int _no_printf(const char *format, ...) { return 0; } #define pr_info(...) _no_printf(__VA_ARGS__) #endif -void print_skip(const char *fmt, ...) __attribute__((format(printf, 1, 2))); +void __printf(1, 2) print_skip(const char *fmt, ...); #define __TEST_REQUIRE(f, fmt, ...) \ do { \ if (!(f)) \ @@ -46,9 +46,9 @@ ssize_t test_write(int fd, const void *buf, size_t count); ssize_t test_read(int fd, void *buf, size_t count); int test_seq_read(const char *path, char **bufp, size_t *sizep); -void test_assert(bool exp, const char *exp_str, - const char *file, unsigned int line, const char *fmt, ...) - __attribute__((format(printf, 5, 6))); +void __printf(5, 6) test_assert(bool exp, const char *exp_str, + const char *file, unsigned int line, + const char *fmt, ...); #define TEST_ASSERT(e, fmt, ...) \ test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__) From d3772e7badd2cd3813e2efba0034f6e39aecc97f Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:31 +0200 Subject: [PATCH 25/51] selftests/mm: Substitute attribute with a macro Compiling mm selftest after adding a __printf() attribute to ksft_print_msg() exposes -Wformat warning in remap_region(). Fix the wrong format specifier causing the warning. The mm selftest uses the printf attribute in its full form. Since the header file that uses it also includes kselftests.h it can use the macro defined there. Use __printf() included with kselftests.h instead of the full attribute. Signed-off-by: Maciej Wieczor-Retman Signed-off-by: Shuah Khan --- tools/testing/selftests/mm/mremap_test.c | 2 +- tools/testing/selftests/mm/pkey-helpers.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c index 5c3773de9f0f..1dbfcf6df255 100644 --- a/tools/testing/selftests/mm/mremap_test.c +++ b/tools/testing/selftests/mm/mremap_test.c @@ -338,7 +338,7 @@ static long long remap_region(struct config c, unsigned int threshold_mb, char c = (char) rand(); if (((char *) dest_addr)[i] != c) { - ksft_print_msg("Data after remap doesn't match at offset %d\n", + ksft_print_msg("Data after remap doesn't match at offset %llu\n", i); ksft_print_msg("Expected: %#x\t Got: %#x\n", c & 0xff, ((char *) dest_addr)[i] & 0xff); diff --git a/tools/testing/selftests/mm/pkey-helpers.h b/tools/testing/selftests/mm/pkey-helpers.h index 92f3be3dd8e5..1af3156a9db8 100644 --- a/tools/testing/selftests/mm/pkey-helpers.h +++ b/tools/testing/selftests/mm/pkey-helpers.h @@ -34,7 +34,7 @@ extern int test_nr; extern int iteration_nr; #ifdef __GNUC__ -__attribute__((format(printf, 1, 2))) +__printf(1, 2) #endif static inline void sigsafe_printf(const char *format, ...) { From 27c734f4402243f478348108d8a2ebd6e3e5e8a3 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Fri, 13 Oct 2023 13:36:32 +0200 Subject: [PATCH 26/51] selftests/resctrl: Fix wrong format specifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiling resctrl selftest after adding a __printf() attribute to ksft_print_msg() exposes -Wformat warning in show_cache_info(). The format specifier used expects a variable of type int but a long unsigned int variable is passed instead. Change the format specifier to match the passed variable. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index d3cbb829ff6a..a5d082cd2d53 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -294,7 +294,7 @@ int show_cache_info(unsigned long sum_llc_val, int no_of_bits, ret = platform && abs((int)diff_percent) > max_diff_percent && (cmt ? (abs(avg_diff) > max_diff) : true); - ksft_print_msg("%s Check cache miss rate within %d%%\n", + ksft_print_msg("%s Check cache miss rate within %lu%%\n", ret ? "Fail:" : "Pass:", max_diff_percent); ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent)); From 4a28c7665c2a1ac0400864eabb0c641e135f61aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:32 +0300 Subject: [PATCH 27/51] selftests/resctrl: Ensure the benchmark commands fits to its array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark command is copied into an array in the stack. The array is BENCHMARK_ARGS items long but the command line could try to provide a longer command. Argument size is also fixed by BENCHMARK_ARG_SIZE (63 bytes of space after fitting the terminating \0 character) and user could have inputted argument longer than that. Return error in case the benchmark command does not fit to the space allocated for it. Fixes: ecdbb911f22d ("selftests/resctrl: Add MBM test") Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Shaopeng Tan Reviewed-by: "Wieczor-Retman, Maciej" Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_tests.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index d511daeb6851..9e2bc8ba95f1 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -255,9 +255,14 @@ int main(int argc, char **argv) return ksft_exit_skip("Not running as root. Skipping...\n"); if (has_ben) { + if (argc - ben_ind >= BENCHMARK_ARGS) + ksft_exit_fail_msg("Too long benchmark command.\n"); + /* Extract benchmark command from command line. */ for (i = ben_ind; i < argc; i++) { benchmark_cmd[i - ben_ind] = benchmark_cmd_area[i]; + if (strlen(argv[i]) >= BENCHMARK_ARG_SIZE) + ksft_exit_fail_msg("Too long benchmark command argument.\n"); sprintf(benchmark_cmd[i - ben_ind], "%s", argv[i]); } benchmark_cmd[ben_count] = NULL; From 5eb6360eeeb661c7589bc344c1c51e9dfcb63451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:33 +0300 Subject: [PATCH 28/51] selftests/resctrl: Correct benchmark command help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark command must be the last argument because it consumes all the remaining arguments but help misleadingly shows it as the first argument. The benchmark command is also shown in quotes but it does not match with the code. Correct -b argument place in the help message and remove the quotes. Tweak also how the options are presented by using ... notation. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 9e2bc8ba95f1..53bd2127b7bf 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -52,8 +52,8 @@ int get_vendor(void) static void cmd_help(void) { - printf("usage: resctrl_tests [-h] [-b \"benchmark_cmd [options]\"] [-t test list] [-n no_of_bits]\n"); - printf("\t-b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CMT\n"); + printf("usage: resctrl_tests [-h] [-t test list] [-n no_of_bits] [-b benchmark_cmd [option]...]\n"); + printf("\t-b benchmark_cmd [option]...: run specified benchmark for MBM, MBA and CMT\n"); printf("\t default benchmark is builtin fill_buf\n"); printf("\t-t test list: run tests specified in the test list, "); printf("e.g. -t mbm,mba,cmt,cat\n"); From 47e36f16c7846bf3627ff68525e02555c53dc99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:34 +0300 Subject: [PATCH 29/51] selftests/resctrl: Remove bw_report and bm_type from main() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bw_report is always set to "reads" and bm_type is set to "fill_buf" but is never used. Set bw_report directly to "reads" in MBA/MBM test and remove bm_type. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/mba_test.c | 4 ++-- tools/testing/selftests/resctrl/mbm_test.c | 4 ++-- tools/testing/selftests/resctrl/resctrl.h | 4 ++-- .../testing/selftests/resctrl/resctrl_tests.c | 18 +++++++----------- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 4d2f145804b8..094424d835d0 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -141,7 +141,7 @@ void mba_test_cleanup(void) remove(RESULT_FILE_NAME); } -int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd) +int mba_schemata_change(int cpu_no, char **benchmark_cmd) { struct resctrl_val_param param = { .resctrl_val = MBA_STR, @@ -149,7 +149,7 @@ int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd) .mongrp = "m1", .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, - .bw_report = bw_report, + .bw_report = "reads", .setup = mba_setup }; int ret; diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index c7de6f5977f6..3e4a800e0e40 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -109,7 +109,7 @@ void mbm_test_cleanup(void) remove(RESULT_FILE_NAME); } -int mbm_bw_change(size_t span, int cpu_no, char *bw_report, char **benchmark_cmd) +int mbm_bw_change(size_t span, int cpu_no, char **benchmark_cmd) { struct resctrl_val_param param = { .resctrl_val = MBM_STR, @@ -118,7 +118,7 @@ int mbm_bw_change(size_t span, int cpu_no, char *bw_report, char **benchmark_cmd .span = span, .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, - .bw_report = bw_report, + .bw_report = "reads", .setup = mbm_setup }; int ret; diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 838d1a438f33..f3446ac664c2 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -98,10 +98,10 @@ int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu, int group_fd, unsigned long flags); int run_fill_buf(size_t span, int memflush, int op, bool once); int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param); -int mbm_bw_change(size_t span, int cpu_no, char *bw_report, char **benchmark_cmd); +int mbm_bw_change(size_t span, int cpu_no, char **benchmark_cmd); void tests_cleanup(void); void mbm_test_cleanup(void); -int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd); +int mba_schemata_change(int cpu_no, char **benchmark_cmd); void mba_test_cleanup(void); int get_cbm_mask(char *cache_type, char *cbm_mask); int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size); diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 53bd2127b7bf..7f3c8432ca5c 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -70,8 +70,7 @@ void tests_cleanup(void) cat_test_cleanup(); } -static void run_mbm_test(char **benchmark_cmd, size_t span, - int cpu_no, char *bw_report) +static void run_mbm_test(char **benchmark_cmd, size_t span, int cpu_no) { int res; @@ -88,7 +87,7 @@ static void run_mbm_test(char **benchmark_cmd, size_t span, goto umount; } - res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd); + res = mbm_bw_change(span, cpu_no, benchmark_cmd); ksft_test_result(!res, "MBM: bw change\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); @@ -97,7 +96,7 @@ umount: umount_resctrlfs(); } -static void run_mba_test(char **benchmark_cmd, int cpu_no, char *bw_report) +static void run_mba_test(char **benchmark_cmd, int cpu_no) { int res; @@ -114,7 +113,7 @@ static void run_mba_test(char **benchmark_cmd, int cpu_no, char *bw_report) goto umount; } - res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd); + res = mba_schemata_change(cpu_no, benchmark_cmd); ksft_test_result(!res, "MBA: schemata change\n"); umount: @@ -174,9 +173,9 @@ umount: int main(int argc, char **argv) { bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true; - char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64], bm_type[64]; char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE]; int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0; + char *benchmark_cmd[BENCHMARK_ARGS]; int ben_ind, ben_count, tests = 0; size_t span = 250 * MB; bool cat_test = true; @@ -279,9 +278,6 @@ int main(int argc, char **argv) benchmark_cmd[5] = NULL; } - sprintf(bw_report, "reads"); - sprintf(bm_type, "fill_buf"); - if (!check_resctrlfs_support()) return ksft_exit_skip("resctrl FS does not exist. Enable X86_CPU_RESCTRL config option.\n"); @@ -293,10 +289,10 @@ int main(int argc, char **argv) ksft_set_plan(tests ? : 4); if (mbm_test) - run_mbm_test(benchmark_cmd, span, cpu_no, bw_report); + run_mbm_test(benchmark_cmd, span, cpu_no); if (mba_test) - run_mba_test(benchmark_cmd, cpu_no, bw_report); + run_mba_test(benchmark_cmd, cpu_no); if (cmt_test) run_cmt_test(benchmark_cmd, cpu_no); From b1a901e078c4ee4a6fe13021c4577ef5f3155251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:35 +0300 Subject: [PATCH 30/51] selftests/resctrl: Simplify span lifetime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit struct resctrl_val_param contains span member. resctrl_val(), however, never uses it because the value of span is embedded into the default benchmark command and parsed from it by run_benchmark(). Remove span from resctrl_val_param. Provide DEFAULT_SPAN for the code that needs it. CMT and CAT tests communicate span that is different from the DEFAULT_SPAN between their internal functions which is converted into passing it directly as a parameter. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/cache.c | 5 +++-- tools/testing/selftests/resctrl/cat_test.c | 13 +++++++------ tools/testing/selftests/resctrl/cmt_test.c | 11 ++++++----- tools/testing/selftests/resctrl/mbm_test.c | 5 ++--- tools/testing/selftests/resctrl/resctrl.h | 8 ++++---- tools/testing/selftests/resctrl/resctrl_tests.c | 9 ++++----- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index a5d082cd2d53..bcbca356d56a 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -205,10 +205,11 @@ int measure_cache_vals(struct resctrl_val_param *param, int bm_pid) * cache_val: execute benchmark and measure LLC occupancy resctrl * and perf cache miss for the benchmark * @param: parameters passed to cache_val() + * @span: buffer size for the benchmark * * Return: 0 on success. non-zero on failure. */ -int cat_val(struct resctrl_val_param *param) +int cat_val(struct resctrl_val_param *param, size_t span) { int memflush = 1, operation = 0, ret = 0; char *resctrl_val = param->resctrl_val; @@ -245,7 +246,7 @@ int cat_val(struct resctrl_val_param *param) if (ret) break; - if (run_fill_buf(param->span, memflush, operation, true)) { + if (run_fill_buf(span, memflush, operation, true)) { fprintf(stderr, "Error-running fill buffer\n"); ret = -1; goto pe_close; diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 3848dfb46aba..97b87285ab2a 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -41,7 +41,7 @@ static int cat_setup(struct resctrl_val_param *p) return ret; } -static int check_results(struct resctrl_val_param *param) +static int check_results(struct resctrl_val_param *param, size_t span) { char *token_array[8], temp[512]; unsigned long sum_llc_perf_miss = 0; @@ -76,7 +76,7 @@ static int check_results(struct resctrl_val_param *param) fclose(fp); no_of_bits = count_bits(param->mask); - return show_cache_info(sum_llc_perf_miss, no_of_bits, param->span / 64, + return show_cache_info(sum_llc_perf_miss, no_of_bits, span / 64, MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, get_vendor() == ARCH_INTEL, false); } @@ -96,6 +96,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) char cbm_mask[256]; int count_of_bits; char pipe_message; + size_t span; /* Get default cbm mask for L3/L2 cache */ ret = get_cbm_mask(cache_type, cbm_mask); @@ -140,7 +141,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) /* Set param values for parent thread which will be allocated bitmask * with (max_bits - n) bits */ - param.span = cache_size * (count_of_bits - n) / count_of_bits; + span = cache_size * (count_of_bits - n) / count_of_bits; strcpy(param.ctrlgrp, "c2"); strcpy(param.mongrp, "m2"); strcpy(param.filename, RESULT_FILE_NAME2); @@ -162,7 +163,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) param.mask = l_mask_1; strcpy(param.ctrlgrp, "c1"); strcpy(param.mongrp, "m1"); - param.span = cache_size * n / count_of_bits; + span = cache_size * n / count_of_bits; strcpy(param.filename, RESULT_FILE_NAME1); param.num_of_runs = 0; param.cpu_no = sibling_cpu_no; @@ -176,9 +177,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) remove(param.filename); - ret = cat_val(¶m); + ret = cat_val(¶m, span); if (ret == 0) - ret = check_results(¶m); + ret = check_results(¶m, span); if (bm_pid == 0) { /* Tell parent that child is ready */ diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index cb2197647c6c..9d8e38e995ef 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -27,7 +27,7 @@ static int cmt_setup(struct resctrl_val_param *p) return 0; } -static int check_results(struct resctrl_val_param *param, int no_of_bits) +static int check_results(struct resctrl_val_param *param, size_t span, int no_of_bits) { char *token_array[8], temp[512]; unsigned long sum_llc_occu_resc = 0; @@ -58,7 +58,7 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits) } fclose(fp); - return show_cache_info(sum_llc_occu_resc, no_of_bits, param->span, + return show_cache_info(sum_llc_occu_resc, no_of_bits, span, MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, true, true); } @@ -74,6 +74,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) unsigned long long_mask; char cbm_mask[256]; int count_of_bits; + size_t span; int ret; if (!validate_resctrl_feature_request(CMT_STR)) @@ -105,13 +106,13 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, .mask = ~(long_mask << n) & long_mask, - .span = cache_size * n / count_of_bits, .num_of_runs = 0, .setup = cmt_setup, }; + span = cache_size * n / count_of_bits; if (strcmp(benchmark_cmd[0], "fill_buf") == 0) - sprintf(benchmark_cmd[1], "%zu", param.span); + sprintf(benchmark_cmd[1], "%zu", span); remove(RESULT_FILE_NAME); @@ -119,7 +120,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) if (ret) goto out; - ret = check_results(¶m, n); + ret = check_results(¶m, span, n); out: cmt_test_cleanup(); diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 3e4a800e0e40..b830fc84338b 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -109,13 +109,12 @@ void mbm_test_cleanup(void) remove(RESULT_FILE_NAME); } -int mbm_bw_change(size_t span, int cpu_no, char **benchmark_cmd) +int mbm_bw_change(int cpu_no, char **benchmark_cmd) { struct resctrl_val_param param = { .resctrl_val = MBM_STR, .ctrlgrp = "c1", .mongrp = "m1", - .span = span, .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, .bw_report = "reads", @@ -129,7 +128,7 @@ int mbm_bw_change(size_t span, int cpu_no, char **benchmark_cmd) if (ret) goto out; - ret = check_results(span); + ret = check_results(DEFAULT_SPAN); out: mbm_test_cleanup(); diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index f3446ac664c2..bcd0d2060f81 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -38,6 +38,8 @@ #define END_OF_TESTS 1 +#define DEFAULT_SPAN (250 * MB) + #define PARENT_EXIT(err_msg) \ do { \ perror(err_msg); \ @@ -52,7 +54,6 @@ * @ctrlgrp: Name of the control monitor group (con_mon grp) * @mongrp: Name of the monitor group (mon grp) * @cpu_no: CPU number to which the benchmark would be binded - * @span: Memory bytes accessed in each benchmark iteration * @filename: Name of file to which the o/p should be written * @bw_report: Bandwidth report type (reads vs writes) * @setup: Call back function to setup test environment @@ -62,7 +63,6 @@ struct resctrl_val_param { char ctrlgrp[64]; char mongrp[64]; int cpu_no; - size_t span; char filename[64]; char *bw_report; unsigned long mask; @@ -98,7 +98,7 @@ int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu, int group_fd, unsigned long flags); int run_fill_buf(size_t span, int memflush, int op, bool once); int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param); -int mbm_bw_change(size_t span, int cpu_no, char **benchmark_cmd); +int mbm_bw_change(int cpu_no, char **benchmark_cmd); void tests_cleanup(void); void mbm_test_cleanup(void); int mba_schemata_change(int cpu_no, char **benchmark_cmd); @@ -108,7 +108,7 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); -int cat_val(struct resctrl_val_param *param); +int cat_val(struct resctrl_val_param *param, size_t span); void cat_test_cleanup(void); int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type); int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd); diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 7f3c8432ca5c..64e28bde5131 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -70,7 +70,7 @@ void tests_cleanup(void) cat_test_cleanup(); } -static void run_mbm_test(char **benchmark_cmd, size_t span, int cpu_no) +static void run_mbm_test(char **benchmark_cmd, int cpu_no) { int res; @@ -87,7 +87,7 @@ static void run_mbm_test(char **benchmark_cmd, size_t span, int cpu_no) goto umount; } - res = mbm_bw_change(span, cpu_no, benchmark_cmd); + res = mbm_bw_change(cpu_no, benchmark_cmd); ksft_test_result(!res, "MBM: bw change\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); @@ -177,7 +177,6 @@ int main(int argc, char **argv) int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0; char *benchmark_cmd[BENCHMARK_ARGS]; int ben_ind, ben_count, tests = 0; - size_t span = 250 * MB; bool cat_test = true; for (i = 0; i < argc; i++) { @@ -271,7 +270,7 @@ int main(int argc, char **argv) benchmark_cmd[i] = benchmark_cmd_area[i]; strcpy(benchmark_cmd[0], "fill_buf"); - sprintf(benchmark_cmd[1], "%zu", span); + sprintf(benchmark_cmd[1], "%u", DEFAULT_SPAN); strcpy(benchmark_cmd[2], "1"); strcpy(benchmark_cmd[3], "0"); strcpy(benchmark_cmd[4], "false"); @@ -289,7 +288,7 @@ int main(int argc, char **argv) ksft_set_plan(tests ? : 4); if (mbm_test) - run_mbm_test(benchmark_cmd, span, cpu_no); + run_mbm_test(benchmark_cmd, cpu_no); if (mba_test) run_mba_test(benchmark_cmd, cpu_no); From 47809eb70cdc81221e03baafe8c1768801a9184e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:36 +0300 Subject: [PATCH 31/51] selftests/resctrl: Reorder resctrl FS prep code and benchmark_cmd init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark command is initialized before resctrl FS check and preparation code that can call ksft_exit_skip(). There is no strong reason why the resctrl FS support check and unmounting it (if already mounted), has to be done after the benchmark command initialization. Move benchmark command initialization such that it is done not until right before the tests commence. This simplifies rollback handling when benchmark command initialization starts to use dynamic allocation (in a change following this). Signed-off-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_tests.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 64e28bde5131..f105e00d6099 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -252,6 +252,14 @@ int main(int argc, char **argv) if (geteuid() != 0) return ksft_exit_skip("Not running as root. Skipping...\n"); + if (!check_resctrlfs_support()) + return ksft_exit_skip("resctrl FS does not exist. Enable X86_CPU_RESCTRL config option.\n"); + + if (umount_resctrlfs()) + return ksft_exit_skip("resctrl FS unmount failed.\n"); + + filter_dmesg(); + if (has_ben) { if (argc - ben_ind >= BENCHMARK_ARGS) ksft_exit_fail_msg("Too long benchmark command.\n"); @@ -277,14 +285,6 @@ int main(int argc, char **argv) benchmark_cmd[5] = NULL; } - if (!check_resctrlfs_support()) - return ksft_exit_skip("resctrl FS does not exist. Enable X86_CPU_RESCTRL config option.\n"); - - if (umount_resctrlfs()) - return ksft_exit_skip("resctrl FS unmount failed.\n"); - - filter_dmesg(); - ksft_set_plan(tests ? : 4); if (mbm_test) From e33cb5702a9f287d829b0e9e6abe57f6a4aba6d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:37 +0300 Subject: [PATCH 32/51] selftests/resctrl: Make benchmark command const and build it with pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark command is used in multiple tests so it should not be mutated by the tests but CMT test alters span argument. Due to the order of tests (CMT test runs last), mutating the span argument in CMT test does not trigger any real problems currently. Mark benchmark_cmd strings as const and setup the benchmark command using pointers. Because the benchmark command becomes const, the input arguments can be used directly. Besides being simpler, using the input arguments directly also removes the internal size restriction. CMT test has to create a copy of the benchmark command before altering the benchmark command. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/cmt_test.c | 25 +++++++++--- tools/testing/selftests/resctrl/mba_test.c | 2 +- tools/testing/selftests/resctrl/mbm_test.c | 2 +- tools/testing/selftests/resctrl/resctrl.h | 10 +++-- .../testing/selftests/resctrl/resctrl_tests.c | 39 ++++++++----------- tools/testing/selftests/resctrl/resctrl_val.c | 10 ++++- 6 files changed, 53 insertions(+), 35 deletions(-) diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index 9d8e38e995ef..cf2f5e92dea6 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -68,14 +68,17 @@ void cmt_test_cleanup(void) remove(RESULT_FILE_NAME); } -int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) +int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd) { + const char * const *cmd = benchmark_cmd; + const char *new_cmd[BENCHMARK_ARGS]; unsigned long cache_size = 0; unsigned long long_mask; + char *span_str = NULL; char cbm_mask[256]; int count_of_bits; size_t span; - int ret; + int ret, i; if (!validate_resctrl_feature_request(CMT_STR)) return -1; @@ -111,12 +114,23 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) }; span = cache_size * n / count_of_bits; - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) - sprintf(benchmark_cmd[1], "%zu", span); + + if (strcmp(cmd[0], "fill_buf") == 0) { + /* Duplicate the command to be able to replace span in it */ + for (i = 0; benchmark_cmd[i]; i++) + new_cmd[i] = benchmark_cmd[i]; + new_cmd[i] = NULL; + + ret = asprintf(&span_str, "%zu", span); + if (ret < 0) + return -1; + new_cmd[1] = span_str; + cmd = new_cmd; + } remove(RESULT_FILE_NAME); - ret = resctrl_val(benchmark_cmd, ¶m); + ret = resctrl_val(cmd, ¶m); if (ret) goto out; @@ -124,6 +138,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) out: cmt_test_cleanup(); + free(span_str); return ret; } diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 094424d835d0..cf8284dadcb2 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -141,7 +141,7 @@ void mba_test_cleanup(void) remove(RESULT_FILE_NAME); } -int mba_schemata_change(int cpu_no, char **benchmark_cmd) +int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd) { struct resctrl_val_param param = { .resctrl_val = MBA_STR, diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index b830fc84338b..1ae131a2e246 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -109,7 +109,7 @@ void mbm_test_cleanup(void) remove(RESULT_FILE_NAME); } -int mbm_bw_change(int cpu_no, char **benchmark_cmd) +int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd) { struct resctrl_val_param param = { .resctrl_val = MBM_STR, diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index bcd0d2060f81..dd07463cdf48 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -38,6 +38,8 @@ #define END_OF_TESTS 1 +#define BENCHMARK_ARGS 64 + #define DEFAULT_SPAN (250 * MB) #define PARENT_EXIT(err_msg) \ @@ -97,11 +99,11 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu, int group_fd, unsigned long flags); int run_fill_buf(size_t span, int memflush, int op, bool once); -int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param); -int mbm_bw_change(int cpu_no, char **benchmark_cmd); +int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *param); +int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd); void tests_cleanup(void); void mbm_test_cleanup(void); -int mba_schemata_change(int cpu_no, char **benchmark_cmd); +int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd); void mba_test_cleanup(void); int get_cbm_mask(char *cache_type, char *cbm_mask); int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size); @@ -111,7 +113,7 @@ void signal_handler_unregister(void); int cat_val(struct resctrl_val_param *param, size_t span); void cat_test_cleanup(void); int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type); -int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd); +int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd); unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void); int get_core_sibling(int cpu_no); diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index f105e00d6099..89edde416f07 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -10,9 +10,6 @@ */ #include "resctrl.h" -#define BENCHMARK_ARGS 64 -#define BENCHMARK_ARG_SIZE 64 - static int detect_vendor(void) { FILE *inf = fopen("/proc/cpuinfo", "r"); @@ -70,7 +67,7 @@ void tests_cleanup(void) cat_test_cleanup(); } -static void run_mbm_test(char **benchmark_cmd, int cpu_no) +static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) { int res; @@ -96,7 +93,7 @@ umount: umount_resctrlfs(); } -static void run_mba_test(char **benchmark_cmd, int cpu_no) +static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) { int res; @@ -120,7 +117,7 @@ umount: umount_resctrlfs(); } -static void run_cmt_test(char **benchmark_cmd, int cpu_no) +static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) { int res; @@ -173,11 +170,12 @@ umount: int main(int argc, char **argv) { bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true; - char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE]; int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0; - char *benchmark_cmd[BENCHMARK_ARGS]; + const char *benchmark_cmd[BENCHMARK_ARGS]; int ben_ind, ben_count, tests = 0; + char *span_str = NULL; bool cat_test = true; + int ret; for (i = 0; i < argc; i++) { if (strcmp(argv[i], "-b") == 0) { @@ -265,23 +263,19 @@ int main(int argc, char **argv) ksft_exit_fail_msg("Too long benchmark command.\n"); /* Extract benchmark command from command line. */ - for (i = ben_ind; i < argc; i++) { - benchmark_cmd[i - ben_ind] = benchmark_cmd_area[i]; - if (strlen(argv[i]) >= BENCHMARK_ARG_SIZE) - ksft_exit_fail_msg("Too long benchmark command argument.\n"); - sprintf(benchmark_cmd[i - ben_ind], "%s", argv[i]); - } + for (i = 0; i < argc - ben_ind; i++) + benchmark_cmd[i] = argv[i + ben_ind]; benchmark_cmd[ben_count] = NULL; } else { /* If no benchmark is given by "-b" argument, use fill_buf. */ - for (i = 0; i < 5; i++) - benchmark_cmd[i] = benchmark_cmd_area[i]; - - strcpy(benchmark_cmd[0], "fill_buf"); - sprintf(benchmark_cmd[1], "%u", DEFAULT_SPAN); - strcpy(benchmark_cmd[2], "1"); - strcpy(benchmark_cmd[3], "0"); - strcpy(benchmark_cmd[4], "false"); + benchmark_cmd[0] = "fill_buf"; + ret = asprintf(&span_str, "%u", DEFAULT_SPAN); + if (ret < 0) + ksft_exit_fail_msg("Out of memory!\n"); + benchmark_cmd[1] = span_str; + benchmark_cmd[2] = "1"; + benchmark_cmd[3] = "0"; + benchmark_cmd[4] = "false"; benchmark_cmd[5] = NULL; } @@ -299,5 +293,6 @@ int main(int argc, char **argv) if (cat_test) run_cat_test(cpu_no, no_of_bits); + free(span_str); ksft_finished(); } diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index f0f6c5f6e98b..51963a6f2186 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -629,7 +629,7 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) * * Return: 0 on success. non-zero on failure. */ -int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) +int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *param) { char *resctrl_val = param->resctrl_val; unsigned long bw_resc_start = 0; @@ -710,7 +710,13 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) if (ret) goto out; - value.sival_ptr = benchmark_cmd; + /* + * The cast removes constness but nothing mutates benchmark_cmd within + * the context of this process. At the receiving process, it becomes + * argv, which is mutable, on exec() but that's after fork() so it + * doesn't matter for the process running the tests. + */ + value.sival_ptr = (void *)benchmark_cmd; /* Taskset benchmark to specified cpu */ ret = taskset_benchmark(bm_pid, param->cpu_no); From 149ff729538aabaa85c44dc58a45fb99798e5e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:38 +0300 Subject: [PATCH 33/51] selftests/resctrl: Remove ben_count variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ben_count is only used to write the terminator for the list. It is enough to use i from the loop so no need for another variable. Remove ben_count variable as it is not needed. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_tests.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 89edde416f07..48e5afb1530c 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -172,7 +172,7 @@ int main(int argc, char **argv) bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true; int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0; const char *benchmark_cmd[BENCHMARK_ARGS]; - int ben_ind, ben_count, tests = 0; + int ben_ind, tests = 0; char *span_str = NULL; bool cat_test = true; int ret; @@ -180,7 +180,6 @@ int main(int argc, char **argv) for (i = 0; i < argc; i++) { if (strcmp(argv[i], "-b") == 0) { ben_ind = i + 1; - ben_count = argc - ben_ind; argc_new = ben_ind - 1; has_ben = true; break; @@ -265,7 +264,7 @@ int main(int argc, char **argv) /* Extract benchmark command from command line. */ for (i = 0; i < argc - ben_ind; i++) benchmark_cmd[i] = argv[i + ben_ind]; - benchmark_cmd[ben_count] = NULL; + benchmark_cmd[i] = NULL; } else { /* If no benchmark is given by "-b" argument, use fill_buf. */ benchmark_cmd[0] = "fill_buf"; From f23c7925e98a1f0d9e3ee93a856dfe3d1288a3f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 4 Sep 2023 12:53:39 +0300 Subject: [PATCH 34/51] selftests/resctrl: Cleanup benchmark argument parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark argument is handled by custom argument parsing code which is more complicated than it needs to be. Process benchmark argument within the normal getopt() handling and drop unnecessary ben_ind and has_ben variables. When -b is given, terminate the argument processing as -b consumes all remaining arguments. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Reviewed-by: "Wieczor-Retman, Maciej" Signed-off-by: Shuah Khan --- .../testing/selftests/resctrl/resctrl_tests.c | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 48e5afb1530c..823672a20a43 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -169,27 +169,34 @@ umount: int main(int argc, char **argv) { - bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true; - int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0; - const char *benchmark_cmd[BENCHMARK_ARGS]; - int ben_ind, tests = 0; + bool mbm_test = true, mba_test = true, cmt_test = true; + const char *benchmark_cmd[BENCHMARK_ARGS] = {}; + int c, cpu_no = 1, i, no_of_bits = 0; char *span_str = NULL; bool cat_test = true; + int tests = 0; int ret; - for (i = 0; i < argc; i++) { - if (strcmp(argv[i], "-b") == 0) { - ben_ind = i + 1; - argc_new = ben_ind - 1; - has_ben = true; - break; - } - } - - while ((c = getopt(argc_new, argv, "ht:b:n:p:")) != -1) { + while ((c = getopt(argc, argv, "ht:b:n:p:")) != -1) { char *token; switch (c) { + case 'b': + /* + * First move optind back to the (first) optarg and + * then build the benchmark command using the + * remaining arguments. + */ + optind--; + if (argc - optind >= BENCHMARK_ARGS) + ksft_exit_fail_msg("Too long benchmark command"); + + /* Extract benchmark command from command line. */ + for (i = 0; i < argc - optind; i++) + benchmark_cmd[i] = argv[i + optind]; + benchmark_cmd[i] = NULL; + + goto last_arg; case 't': token = strtok(optarg, ","); @@ -238,6 +245,7 @@ int main(int argc, char **argv) return -1; } } +last_arg: ksft_print_header(); @@ -257,15 +265,7 @@ int main(int argc, char **argv) filter_dmesg(); - if (has_ben) { - if (argc - ben_ind >= BENCHMARK_ARGS) - ksft_exit_fail_msg("Too long benchmark command.\n"); - - /* Extract benchmark command from command line. */ - for (i = 0; i < argc - ben_ind; i++) - benchmark_cmd[i] = argv[i + ben_ind]; - benchmark_cmd[i] = NULL; - } else { + if (!benchmark_cmd[0]) { /* If no benchmark is given by "-b" argument, use fill_buf. */ benchmark_cmd[0] = "fill_buf"; ret = asprintf(&span_str, "%u", DEFAULT_SPAN); From beb7f471847663559bd0fe60af1d70e05a1d7c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:07 +0300 Subject: [PATCH 35/51] selftests/resctrl: Fix uninitialized .sa_flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signal_handler_unregister() calls sigaction() with uninitializing sa_flags in the struct sigaction. Make sure sa_flags is always initialized in signal_handler_unregister() by initializing the struct sigaction when declaring it. Also add the initialization to signal_handler_register() even if there are no know bugs in there because correctness is then obvious from the code itself. Fixes: 73c55fa5ab55 ("selftests/resctrl: Commonize the signal handler register/unregister for all tests") Suggested-by: Reinette Chatre Signed-off-by: Ilpo Järvinen Cc: Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_val.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 51963a6f2186..01bbe11a8983 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -482,7 +482,7 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr) */ int signal_handler_register(void) { - struct sigaction sigact; + struct sigaction sigact = {}; int ret = 0; sigact.sa_sigaction = ctrlc_handler; @@ -504,7 +504,7 @@ int signal_handler_register(void) */ void signal_handler_unregister(void) { - struct sigaction sigact; + struct sigaction sigact = {}; sigact.sa_handler = SIG_DFL; sigemptyset(&sigact.sa_mask); From 3aff5146445582454c35900f3c0c972987cdd595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:08 +0300 Subject: [PATCH 36/51] selftests/resctrl: Extend signal handler coverage to unmount on receiving signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unmounting resctrl FS has been moved into the per test functions in resctrl_tests.c by commit caddc0fbe495 ("selftests/resctrl: Move resctrl FS mount/umount to higher level"). In case a signal (SIGINT, SIGTERM, or SIGHUP) is received, the running selftest is aborted by ctrlc_handler() which then unmounts resctrl fs before exiting. The current section between signal_handler_register() and signal_handler_unregister(), however, does not cover the entire duration when resctrl FS is mounted. Move signal_handler_register() and signal_handler_unregister() calls from per test files into resctrl_tests.c to properly unmount resctrl fs. In order to not add signal_handler_register()/unregister() n times, create helpers test_prepare() and test_cleanup(). Do not call ksft_exit_fail_msg() in test_prepare() but only in the per test function to keep the control flow cleaner without adding calls to exit() deep into the call chain. Adjust child process kill() call in ctrlc_handler() to only be invoked if the child was already forked. Fixes: caddc0fbe495 ("selftests/resctrl: Move resctrl FS mount/umount to higher level") Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Shaopeng Tan Reviewed-by: Reinette Chatre Cc: Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/cat_test.c | 8 --- .../testing/selftests/resctrl/resctrl_tests.c | 69 ++++++++++++------- tools/testing/selftests/resctrl/resctrl_val.c | 22 +++--- 3 files changed, 55 insertions(+), 44 deletions(-) diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 97b87285ab2a..224ba8544d8a 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -167,12 +167,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) strcpy(param.filename, RESULT_FILE_NAME1); param.num_of_runs = 0; param.cpu_no = sibling_cpu_no; - } else { - ret = signal_handler_register(); - if (ret) { - kill(bm_pid, SIGKILL); - goto out; - } } remove(param.filename); @@ -209,10 +203,8 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) } close(pipefd[0]); kill(bm_pid, SIGKILL); - signal_handler_unregister(); } -out: cat_test_cleanup(); return ret; diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 823672a20a43..495aeee5b734 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -67,21 +67,45 @@ void tests_cleanup(void) cat_test_cleanup(); } +static int test_prepare(void) +{ + int res; + + res = signal_handler_register(); + if (res) { + ksft_print_msg("Failed to register signal handler\n"); + return res; + } + + res = mount_resctrlfs(); + if (res) { + signal_handler_unregister(); + ksft_print_msg("Failed to mount resctrl FS\n"); + return res; + } + return 0; +} + +static void test_cleanup(void) +{ + umount_resctrlfs(); + signal_handler_unregister(); +} + static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) { int res; ksft_print_msg("Starting MBM BW change ...\n"); - res = mount_resctrlfs(); - if (res) { - ksft_exit_fail_msg("Failed to mount resctrl FS\n"); + if (test_prepare()) { + ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; } if (!validate_resctrl_feature_request(MBM_STR) || (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBM or MBM is disabled\n"); - goto umount; + goto cleanup; } res = mbm_bw_change(cpu_no, benchmark_cmd); @@ -89,8 +113,8 @@ static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); -umount: - umount_resctrlfs(); +cleanup: + test_cleanup(); } static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) @@ -99,22 +123,21 @@ static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) ksft_print_msg("Starting MBA Schemata change ...\n"); - res = mount_resctrlfs(); - if (res) { - ksft_exit_fail_msg("Failed to mount resctrl FS\n"); + if (test_prepare()) { + ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; } if (!validate_resctrl_feature_request(MBA_STR) || (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBA or MBA is disabled\n"); - goto umount; + goto cleanup; } res = mba_schemata_change(cpu_no, benchmark_cmd); ksft_test_result(!res, "MBA: schemata change\n"); -umount: - umount_resctrlfs(); +cleanup: + test_cleanup(); } static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) @@ -123,15 +146,14 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) ksft_print_msg("Starting CMT test ...\n"); - res = mount_resctrlfs(); - if (res) { - ksft_exit_fail_msg("Failed to mount resctrl FS\n"); + if (test_prepare()) { + ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; } if (!validate_resctrl_feature_request(CMT_STR)) { ksft_test_result_skip("Hardware does not support CMT or CMT is disabled\n"); - goto umount; + goto cleanup; } res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd); @@ -139,8 +161,8 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); -umount: - umount_resctrlfs(); +cleanup: + test_cleanup(); } static void run_cat_test(int cpu_no, int no_of_bits) @@ -149,22 +171,21 @@ static void run_cat_test(int cpu_no, int no_of_bits) ksft_print_msg("Starting CAT test ...\n"); - res = mount_resctrlfs(); - if (res) { - ksft_exit_fail_msg("Failed to mount resctrl FS\n"); + if (test_prepare()) { + ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; } if (!validate_resctrl_feature_request(CAT_STR)) { ksft_test_result_skip("Hardware does not support CAT or CAT is disabled\n"); - goto umount; + goto cleanup; } res = cat_perf_miss_val(cpu_no, no_of_bits, "L3"); ksft_test_result(!res, "CAT: test\n"); -umount: - umount_resctrlfs(); +cleanup: + test_cleanup(); } int main(int argc, char **argv) diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 01bbe11a8983..b8ca6fa40b3b 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -468,7 +468,9 @@ pid_t bm_pid, ppid; void ctrlc_handler(int signum, siginfo_t *info, void *ptr) { - kill(bm_pid, SIGKILL); + /* Only kill child after bm_pid is set after fork() */ + if (bm_pid) + kill(bm_pid, SIGKILL); umount_resctrlfs(); tests_cleanup(); ksft_print_msg("Ending\n\n"); @@ -485,6 +487,8 @@ int signal_handler_register(void) struct sigaction sigact = {}; int ret = 0; + bm_pid = 0; + sigact.sa_sigaction = ctrlc_handler; sigemptyset(&sigact.sa_mask); sigact.sa_flags = SA_SIGINFO; @@ -706,10 +710,6 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par ksft_print_msg("Benchmark PID: %d\n", bm_pid); - ret = signal_handler_register(); - if (ret) - goto out; - /* * The cast removes constness but nothing mutates benchmark_cmd within * the context of this process. At the receiving process, it becomes @@ -721,19 +721,19 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par /* Taskset benchmark to specified cpu */ ret = taskset_benchmark(bm_pid, param->cpu_no); if (ret) - goto unregister; + goto out; /* Write benchmark to specified control&monitoring grp in resctrl FS */ ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, resctrl_val); if (ret) - goto unregister; + goto out; if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) || !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) { ret = initialize_mem_bw_imc(); if (ret) - goto unregister; + goto out; initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp, param->cpu_no, resctrl_val); @@ -748,7 +748,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par sizeof(pipe_message)) { perror("# failed reading message from child process"); close(pipefd[0]); - goto unregister; + goto out; } } close(pipefd[0]); @@ -757,7 +757,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par if (sigqueue(bm_pid, SIGUSR1, value) == -1) { perror("# sigqueue SIGUSR1 to child"); ret = errno; - goto unregister; + goto out; } /* Give benchmark enough time to fully run */ @@ -786,8 +786,6 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par } } -unregister: - signal_handler_unregister(); out: kill(bm_pid, SIGKILL); From 030b48fb2cf045dead8ee2c5ead560930044c029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:09 +0300 Subject: [PATCH 37/51] selftests/resctrl: Remove duplicate feature check from CMT test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test runner run_cmt_test() in resctrl_tests.c checks for CMT feature and does not run cmt_resctrl_val() if CMT is not supported. Then cmt_resctrl_val() also check is CMT is supported. Remove the duplicated feature check for CMT from cmt_resctrl_val(). Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Cc: Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/cmt_test.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index cf2f5e92dea6..50bdbce9fba9 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -80,9 +80,6 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd) size_t span; int ret, i; - if (!validate_resctrl_feature_request(CMT_STR)) - return -1; - ret = get_cbm_mask("L3", cbm_mask); if (ret) return ret; From 3a1e4a91aa454a1c589a9824d54179fdbfccde45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:10 +0300 Subject: [PATCH 38/51] selftests/resctrl: Move _GNU_SOURCE define into Makefile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _GNU_SOURCE is defined in resctrl.h. Defining _GNU_SOURCE has a large impact on what gets defined when including headers either before or after it. This can result in compile failures if .c file decides to include a standard header file before resctrl.h. It is safer to define _GNU_SOURCE in Makefile so it is always defined regardless of in which order includes are done. Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Cc: Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/Makefile | 2 +- tools/testing/selftests/resctrl/resctrl.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile index 5073dbc96125..2deac2031de9 100644 --- a/tools/testing/selftests/resctrl/Makefile +++ b/tools/testing/selftests/resctrl/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 +CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE CFLAGS += $(KHDR_INCLUDES) TEST_GEN_PROGS := resctrl_tests diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index dd07463cdf48..d9b5df95849d 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -1,5 +1,4 @@ /* SPDX-License-Identifier: GPL-2.0 */ -#define _GNU_SOURCE #ifndef RESCTRL_H #define RESCTRL_H #include From d56e5da0e0f557a206bace16bbbdad00a5800e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:11 +0300 Subject: [PATCH 39/51] selftests/resctrl: Refactor feature check to use resource and feature name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feature check in validate_resctrl_feature_request() takes in the test name string and maps that to what to check per test. Pass resource and feature names to validate_resctrl_feature_request() directly rather than deriving them from the test name inside the function which makes the feature check easier to extend for new test cases. Use !! in the return statement to make the boolean conversion more obvious even if it is not strictly necessary from correctness point of view (to avoid it looking like the function is returning a freed pointer). Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Cc: # selftests/resctrl: Remove duplicate feature check from CMT test Cc: # selftests/resctrl: Move _GNU_SOURCE define into Makefile Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl.h | 6 +- .../testing/selftests/resctrl/resctrl_tests.c | 10 +-- tools/testing/selftests/resctrl/resctrlfs.c | 65 +++++++------------ 3 files changed, 32 insertions(+), 49 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index d9b5df95849d..8578a8b4e145 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -27,10 +27,6 @@ #define RESCTRL_PATH "/sys/fs/resctrl" #define PHYS_ID_PATH "/sys/devices/system/cpu/cpu" #define INFO_PATH "/sys/fs/resctrl/info" -#define L3_PATH "/sys/fs/resctrl/info/L3" -#define MB_PATH "/sys/fs/resctrl/info/MB" -#define L3_MON_PATH "/sys/fs/resctrl/info/L3_MON" -#define L3_MON_FEATURES_PATH "/sys/fs/resctrl/info/L3_MON/mon_features" #define ARCH_INTEL 1 #define ARCH_AMD 2 @@ -87,7 +83,7 @@ int get_resource_id(int cpu_no, int *resource_id); int mount_resctrlfs(void); int umount_resctrlfs(void); int validate_bw_report_request(char *bw_report); -bool validate_resctrl_feature_request(const char *resctrl_val); +bool validate_resctrl_feature_request(const char *resource, const char *feature); char *fgrep(FILE *inf, const char *str); int taskset_benchmark(pid_t bm_pid, int cpu_no); void run_benchmark(int signum, siginfo_t *info, void *ucontext); diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 495aeee5b734..a19dcc3f8fb0 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -103,7 +103,9 @@ static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) return; } - if (!validate_resctrl_feature_request(MBM_STR) || (get_vendor() != ARCH_INTEL)) { + if (!validate_resctrl_feature_request("L3_MON", "mbm_total_bytes") || + !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") || + (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBM or MBM is disabled\n"); goto cleanup; } @@ -128,7 +130,7 @@ static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) return; } - if (!validate_resctrl_feature_request(MBA_STR) || (get_vendor() != ARCH_INTEL)) { + if (!validate_resctrl_feature_request("MB", NULL) || (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBA or MBA is disabled\n"); goto cleanup; } @@ -151,7 +153,7 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) return; } - if (!validate_resctrl_feature_request(CMT_STR)) { + if (!validate_resctrl_feature_request("L3_MON", "llc_occupancy")) { ksft_test_result_skip("Hardware does not support CMT or CMT is disabled\n"); goto cleanup; } @@ -176,7 +178,7 @@ static void run_cat_test(int cpu_no, int no_of_bits) return; } - if (!validate_resctrl_feature_request(CAT_STR)) { + if (!validate_resctrl_feature_request("L3", NULL)) { ksft_test_result_skip("Hardware does not support CAT or CAT is disabled\n"); goto cleanup; } diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index bd36ee206602..3a8111362d26 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -8,6 +8,8 @@ * Sai Praneeth Prakhya , * Fenghua Yu */ +#include + #include "resctrl.h" static int find_resctrl_mount(char *buffer) @@ -604,63 +606,46 @@ char *fgrep(FILE *inf, const char *str) /* * validate_resctrl_feature_request - Check if requested feature is valid. - * @resctrl_val: Requested feature + * @resource: Required resource (e.g., MB, L3, L2, L3_MON, etc.) + * @feature: Required monitor feature (in mon_features file). Can only be + * set for L3_MON. Must be NULL for all other resources. * - * Return: True if the feature is supported, else false. False is also - * returned if resctrl FS is not mounted. + * Return: True if the resource/feature is supported, else false. False is + * also returned if resctrl FS is not mounted. */ -bool validate_resctrl_feature_request(const char *resctrl_val) +bool validate_resctrl_feature_request(const char *resource, const char *feature) { + char res_path[PATH_MAX]; struct stat statbuf; - bool found = false; char *res; FILE *inf; int ret; - if (!resctrl_val) + if (!resource) return false; ret = find_resctrl_mount(NULL); if (ret) return false; - if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) { - if (!stat(L3_PATH, &statbuf)) - return true; - } else if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) { - if (!stat(MB_PATH, &statbuf)) - return true; - } else if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) || - !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) { - if (!stat(L3_MON_PATH, &statbuf)) { - inf = fopen(L3_MON_FEATURES_PATH, "r"); - if (!inf) - return false; + snprintf(res_path, sizeof(res_path), "%s/%s", INFO_PATH, resource); - if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) { - res = fgrep(inf, "llc_occupancy"); - if (res) { - found = true; - free(res); - } - } + if (stat(res_path, &statbuf)) + return false; - if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) { - res = fgrep(inf, "mbm_total_bytes"); - if (res) { - free(res); - res = fgrep(inf, "mbm_local_bytes"); - if (res) { - found = true; - free(res); - } - } - } - fclose(inf); - } - } + if (!feature) + return true; - return found; + snprintf(res_path, sizeof(res_path), "%s/%s/mon_features", INFO_PATH, resource); + inf = fopen(res_path, "r"); + if (!inf) + return false; + + res = fgrep(inf, feature); + free(res); + fclose(inf); + + return !!res; } int filter_dmesg(void) From 06035f019422ba17e85c11e70d6d8bdbe9fa1afd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:12 +0300 Subject: [PATCH 40/51] selftests/resctrl: Fix feature checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MBA and CMT tests expect support of other features to be able to run. When platform only supports MBA but not MBM, MBA test will fail with: Failed to open total bw file: No such file or directory When platform only supports CMT but not CAT, CMT test will fail with: Failed to open bit mask file '/sys/fs/resctrl/info/L3/cbm_mask': No such file or directory It leads to the test reporting test fail (even if no test was run at all). Extend feature checks to cover these two conditions to show these tests were skipped rather than failed. Fixes: ee0415681eb6 ("selftests/resctrl: Use resctrl/info for feature detection") Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Cc: # selftests/resctrl: Refactor feature check to use resource and feature name Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl_tests.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index a19dcc3f8fb0..2bbe3045a018 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -130,7 +130,9 @@ static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) return; } - if (!validate_resctrl_feature_request("MB", NULL) || (get_vendor() != ARCH_INTEL)) { + if (!validate_resctrl_feature_request("MB", NULL) || + !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") || + (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBA or MBA is disabled\n"); goto cleanup; } @@ -153,7 +155,8 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) return; } - if (!validate_resctrl_feature_request("L3_MON", "llc_occupancy")) { + if (!validate_resctrl_feature_request("L3_MON", "llc_occupancy") || + !validate_resctrl_feature_request("L3", NULL)) { ksft_test_result_skip("Hardware does not support CMT or CMT is disabled\n"); goto cleanup; } From ef43c30858754d99373a63dff33280a9969b49bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 2 Oct 2023 12:48:13 +0300 Subject: [PATCH 41/51] selftests/resctrl: Reduce failures due to outliers in MBA/MBM tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial value of 5% chosen for the maximum allowed percentage difference between resctrl mbm value and IMC mbm value in commit 06bd03a57f8c ("selftests/resctrl: Fix MBA/MBM results reporting format") was "randomly chosen value" (as admitted by the changelog). When running tests in our lab across a large number platforms, 5% difference upper bound for success seems a bit on the low side for the MBA and MBM tests. Some platforms produce outliers that are slightly above that, typically 6-7%, which leads MBA/MBM test frequently failing. Replace the "randomly chosen value" with a success bound that is based on those measurements across large number of platforms by relaxing the MBA/MBM success bound to 8%. The relaxed bound removes the failures due the frequent outliers. Fixed commit description style error during merge: Shuah Khan Fixes: 06bd03a57f8c ("selftests/resctrl: Fix MBA/MBM results reporting format") Signed-off-by: Ilpo Järvinen Tested-by: Shaopeng Tan Reviewed-by: Reinette Chatre Reviewed-by: Shaopeng Tan Cc: Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/mba_test.c | 2 +- tools/testing/selftests/resctrl/mbm_test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index cf8284dadcb2..d3bf4368341e 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -12,7 +12,7 @@ #define RESULT_FILE_NAME "result_mba" #define NUM_OF_RUNS 5 -#define MAX_DIFF_PERCENT 5 +#define MAX_DIFF_PERCENT 8 #define ALLOCATION_MAX 100 #define ALLOCATION_MIN 10 #define ALLOCATION_STEP 10 diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 1ae131a2e246..d3c0d30c676a 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -11,7 +11,7 @@ #include "resctrl.h" #define RESULT_FILE_NAME "result_mbm" -#define MAX_DIFF_PERCENT 5 +#define MAX_DIFF_PERCENT 8 #define NUM_OF_RUNS 5 static int From 20d96b25cc4c45823fbec5406ae5ae45da6a892e Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Tue, 10 Oct 2023 09:56:11 +0200 Subject: [PATCH 42/51] selftests/resctrl: Fix schemata write error check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken. Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked. Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose(). Remove newline character from the schema string after writing it to the schemata file so it prints correctly before function return. Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrlfs.c | 41 +++++++++++++-------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 3a8111362d26..05390afd4d6f 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -8,6 +8,7 @@ * Sai Praneeth Prakhya , * Fenghua Yu */ +#include #include #include "resctrl.h" @@ -490,9 +491,8 @@ out: */ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) { - char controlgroup[1024], schema[1024], reason[64]; - int resource_id, ret = 0; - FILE *fp; + char controlgroup[1024], reason[128], schema[1024] = {}; + int resource_id, fd, schema_len = -1, ret = 0; if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) && strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) && @@ -520,28 +520,39 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) || !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) - sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata); + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n", + "L3:", resource_id, '=', schemata); if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) || !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) - sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata); - - fp = fopen(controlgroup, "w"); - if (!fp) { - sprintf(reason, "Failed to open control group"); + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n", + "MB:", resource_id, '=', schemata); + if (schema_len < 0 || schema_len >= sizeof(schema)) { + snprintf(reason, sizeof(reason), + "snprintf() failed with return value : %d", schema_len); ret = -1; - goto out; } - if (fprintf(fp, "%s\n", schema) < 0) { - sprintf(reason, "Failed to write schemata in control group"); - fclose(fp); + fd = open(controlgroup, O_WRONLY); + if (fd < 0) { + snprintf(reason, sizeof(reason), + "open() failed : %s", strerror(errno)); ret = -1; - goto out; + goto err_schema_not_empty; } - fclose(fp); + if (write(fd, schema, schema_len) < 0) { + snprintf(reason, sizeof(reason), + "write() failed : %s", strerror(errno)); + close(fd); + ret = -1; + goto err_schema_not_empty; + } + close(fd); + +err_schema_not_empty: + schema[schema_len - 1] = 0; out: ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n", schema, ret ? " # " : "", From 508934b5d15ab79fd5895cc2a6063bc9d95f6a55 Mon Sep 17 00:00:00 2001 From: Maciej Wieczor-Retman Date: Tue, 10 Oct 2023 09:56:12 +0200 Subject: [PATCH 43/51] selftests/resctrl: Move run_benchmark() to a more fitting file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resctrlfs.c contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking. run_benchmark() is located in resctrlfs.c even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic. Move run_benchmark() to resctrl_val.c just before resctrl_val() that makes use of run_benchmark(). Make run_benchmark() static since it's not used between multiple files anymore. Remove return comment from kernel-doc since the function is type void. Signed-off-by: Maciej Wieczor-Retman Reviewed-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/resctrl.h | 1 - tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 52 ------------------- 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 8578a8b4e145..a33f414f6019 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -86,7 +86,6 @@ int validate_bw_report_request(char *bw_report); bool validate_resctrl_feature_request(const char *resource, const char *feature); char *fgrep(FILE *inf, const char *str); int taskset_benchmark(pid_t bm_pid, int cpu_no); -void run_benchmark(int signum, siginfo_t *info, void *ucontext); int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val); int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index b8ca6fa40b3b..88789678917b 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -625,6 +625,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) return 0; } +/* + * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) + * in specified signal. Direct benchmark stdio to /dev/null. + * @signum: signal number + * @info: signal info + * @ucontext: user context in signal handling + */ +static void run_benchmark(int signum, siginfo_t *info, void *ucontext) +{ + int operation, ret, memflush; + char **benchmark_cmd; + size_t span; + bool once; + FILE *fp; + + benchmark_cmd = info->si_ptr; + + /* + * Direct stdio of child to /dev/null, so that only parent writes to + * stdio (console) + */ + fp = freopen("/dev/null", "w", stdout); + if (!fp) + PARENT_EXIT("Unable to direct benchmark status to /dev/null"); + + if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { + /* Execute default fill_buf benchmark */ + span = strtoul(benchmark_cmd[1], NULL, 10); + memflush = atoi(benchmark_cmd[2]); + operation = atoi(benchmark_cmd[3]); + if (!strcmp(benchmark_cmd[4], "true")) + once = true; + else if (!strcmp(benchmark_cmd[4], "false")) + once = false; + else + PARENT_EXIT("Invalid once parameter"); + + if (run_fill_buf(span, memflush, operation, once)) + fprintf(stderr, "Error in running fill buffer\n"); + } else { + /* Execute specified benchmark */ + ret = execvp(benchmark_cmd[0], benchmark_cmd); + if (ret) + perror("wrong\n"); + } + + fclose(stdout); + PARENT_EXIT("Unable to run specified benchmark"); +} + /* * resctrl_val: execute benchmark and measure memory bandwidth on * the benchmark diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 05390afd4d6f..5ebd43683876 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -294,58 +294,6 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) return 0; } -/* - * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) - * in specified signal. Direct benchmark stdio to /dev/null. - * @signum: signal number - * @info: signal info - * @ucontext: user context in signal handling - * - * Return: void - */ -void run_benchmark(int signum, siginfo_t *info, void *ucontext) -{ - int operation, ret, memflush; - char **benchmark_cmd; - size_t span; - bool once; - FILE *fp; - - benchmark_cmd = info->si_ptr; - - /* - * Direct stdio of child to /dev/null, so that only parent writes to - * stdio (console) - */ - fp = freopen("/dev/null", "w", stdout); - if (!fp) - PARENT_EXIT("Unable to direct benchmark status to /dev/null"); - - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { - /* Execute default fill_buf benchmark */ - span = strtoul(benchmark_cmd[1], NULL, 10); - memflush = atoi(benchmark_cmd[2]); - operation = atoi(benchmark_cmd[3]); - if (!strcmp(benchmark_cmd[4], "true")) - once = true; - else if (!strcmp(benchmark_cmd[4], "false")) - once = false; - else - PARENT_EXIT("Invalid once parameter"); - - if (run_fill_buf(span, memflush, operation, once)) - fprintf(stderr, "Error in running fill buffer\n"); - } else { - /* Execute specified benchmark */ - ret = execvp(benchmark_cmd[0], benchmark_cmd); - if (ret) - perror("wrong\n"); - } - - fclose(stdout); - PARENT_EXIT("Unable to run specified benchmark"); -} - /* * create_grp - Create a group only if one doesn't exist * @grp_name: Name of the group From 27aabb2c4390561538ca76a5bd418d95037f28d5 Mon Sep 17 00:00:00 2001 From: Swapnil Sapkal Date: Thu, 12 Oct 2023 09:32:24 +0000 Subject: [PATCH 44/51] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut In selftests/amd-pstate, tbench and gitsource microbenchmarks are used to compare the performance with different governors. In current implementation the relative path to run `amd_pstate_tracer.py` is broken. Fix this by using absolute paths. Signed-off-by: Swapnil Sapkal Reviewed-by: Mario Limonciello Signed-off-by: Shuah Khan --- .../x86/amd_pstate_tracer/amd_pstate_trace.py | 3 +-- tools/testing/selftests/amd-pstate/gitsource.sh | 17 +++++++++++------ tools/testing/selftests/amd-pstate/run.sh | 9 ++++++--- tools/testing/selftests/amd-pstate/tbench.sh | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py index 904df0ea0a1e..feb9f9421c7b 100755 --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py @@ -30,8 +30,7 @@ import getopt import Gnuplot from numpy import * from decimal import * -sys.path.append('../intel_pstate_tracer') -#import intel_pstate_tracer +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer")) import intel_pstate_tracer as ipt __license__ = "GPL version 2" diff --git a/tools/testing/selftests/amd-pstate/gitsource.sh b/tools/testing/selftests/amd-pstate/gitsource.sh index 5f2171f0116d..ab195ddcba4d 100755 --- a/tools/testing/selftests/amd-pstate/gitsource.sh +++ b/tools/testing/selftests/amd-pstate/gitsource.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # SPDX-License-Identifier: GPL-2.0 # Testing and monitor the cpu desire performance, frequency, load, @@ -66,12 +66,15 @@ post_clear_gitsource() install_gitsource() { - if [ ! -d $git_name ]; then + if [ ! -d $SCRIPTDIR/$git_name ]; then + pushd $(pwd) > /dev/null 2>&1 + cd $SCRIPTDIR printf "Download gitsource, please wait a moment ...\n\n" wget -O $git_tar $gitsource_url > /dev/null 2>&1 printf "Tar gitsource ...\n\n" tar -xzf $git_tar + popd > /dev/null 2>&1 fi } @@ -79,12 +82,14 @@ install_gitsource() run_gitsource() { echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL" - ./amd_pstate_trace.py -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 & + $TRACER -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 & printf "Make and test gitsource for $1 #$2 make_cpus: $MAKE_CPUS\n" - cd $git_name - perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o ../$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > ../$OUTFILE_GIT-perf-$1-$2.log 2>&1 - cd .. + BACKUP_DIR=$(pwd) + pushd $BACKUP_DIR > /dev/null 2>&1 + cd $SCRIPTDIR/$git_name + perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1 + popd > /dev/null 2>&1 for job in `jobs -p` do diff --git a/tools/testing/selftests/amd-pstate/run.sh b/tools/testing/selftests/amd-pstate/run.sh index de4d8e9c9565..279d073c5728 100755 --- a/tools/testing/selftests/amd-pstate/run.sh +++ b/tools/testing/selftests/amd-pstate/run.sh @@ -8,9 +8,12 @@ else FILE_MAIN=DONE fi -source basic.sh -source tbench.sh -source gitsource.sh +SCRIPTDIR=`dirname "$0"` +TRACER=$SCRIPTDIR/../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py + +source $SCRIPTDIR/basic.sh +source $SCRIPTDIR/tbench.sh +source $SCRIPTDIR/gitsource.sh # amd-pstate-ut only run on x86/x86_64 AMD systems. ARCH=$(uname -m 2>/dev/null | sed -e 's/i.86/x86/' -e 's/x86_64/x86/') diff --git a/tools/testing/selftests/amd-pstate/tbench.sh b/tools/testing/selftests/amd-pstate/tbench.sh index 49c9850341f6..4d2e8ce2da3b 100755 --- a/tools/testing/selftests/amd-pstate/tbench.sh +++ b/tools/testing/selftests/amd-pstate/tbench.sh @@ -64,7 +64,7 @@ post_clear_tbench() run_tbench() { echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL" - ./amd_pstate_trace.py -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 & + $TRACER -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 & printf "Test tbench for $1 #$2 time_limit: $TIME_LIMIT procs_num: $PROCESS_NUM\n" tbench_srv > /dev/null 2>&1 & From 0996e6742399e56b547319c4b8061af79be071b2 Mon Sep 17 00:00:00 2001 From: Swapnil Sapkal Date: Thu, 12 Oct 2023 09:32:25 +0000 Subject: [PATCH 45/51] selftests/amd-pstate: Added option to provide perf binary path In selftests/amd-pstate, distro `perf` is used to capture `perf stat` while running microbenchmarks. Distro `perf` is not working with upstream kernel. Fix this by providing an option to give the perf binary path. Reviewed-by: Mario Limonciello Signed-off-by: Swapnil Sapkal Signed-off-by: Shuah Khan --- tools/testing/selftests/amd-pstate/gitsource.sh | 2 +- tools/testing/selftests/amd-pstate/run.sh | 12 +++++++++--- tools/testing/selftests/amd-pstate/tbench.sh | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/amd-pstate/gitsource.sh b/tools/testing/selftests/amd-pstate/gitsource.sh index ab195ddcba4d..4cde62f90468 100755 --- a/tools/testing/selftests/amd-pstate/gitsource.sh +++ b/tools/testing/selftests/amd-pstate/gitsource.sh @@ -88,7 +88,7 @@ run_gitsource() BACKUP_DIR=$(pwd) pushd $BACKUP_DIR > /dev/null 2>&1 cd $SCRIPTDIR/$git_name - perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1 + $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1 popd > /dev/null 2>&1 for job in `jobs -p` diff --git a/tools/testing/selftests/amd-pstate/run.sh b/tools/testing/selftests/amd-pstate/run.sh index 279d073c5728..b053eea8bb19 100755 --- a/tools/testing/selftests/amd-pstate/run.sh +++ b/tools/testing/selftests/amd-pstate/run.sh @@ -25,6 +25,7 @@ OUTFILE=selftest OUTFILE_TBENCH="$OUTFILE.tbench" OUTFILE_GIT="$OUTFILE.gitsource" +PERF=/usr/bin/perf SYSFS= CPUROOT= CPUFREQROOT= @@ -154,6 +155,7 @@ help() [-p ] [-l ] [-i ] + [-b ] [-m ] \n" exit 2 @@ -161,7 +163,7 @@ help() parse_arguments() { - while getopts ho:c:t:p:l:i:m: arg + while getopts ho:c:t:p:l:i:b:m: arg do case $arg in h) # --help @@ -192,6 +194,10 @@ parse_arguments() TRACER_INTERVAL=$OPTARG ;; + b) # --perf-binary + PERF=`realpath $OPTARG` + ;; + m) # --comparative-test COMPARATIVE_TEST=$OPTARG ;; @@ -205,8 +211,8 @@ parse_arguments() command_perf() { - if ! command -v perf > /dev/null; then - echo $msg please install perf. >&2 + if ! $PERF -v; then + echo $msg please install perf or provide perf binary path as argument >&2 exit $ksft_skip fi } diff --git a/tools/testing/selftests/amd-pstate/tbench.sh b/tools/testing/selftests/amd-pstate/tbench.sh index 4d2e8ce2da3b..2a98d9c9202e 100755 --- a/tools/testing/selftests/amd-pstate/tbench.sh +++ b/tools/testing/selftests/amd-pstate/tbench.sh @@ -68,7 +68,7 @@ run_tbench() printf "Test tbench for $1 #$2 time_limit: $TIME_LIMIT procs_num: $PROCESS_NUM\n" tbench_srv > /dev/null 2>&1 & - perf stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1 + $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1 pid=`pidof tbench_srv` kill $pid From 2eadb3299256087e901bed113b24e5807c2a9602 Mon Sep 17 00:00:00 2001 From: Yu Liao Date: Wed, 18 Oct 2023 10:08:57 +0800 Subject: [PATCH 46/51] selftests/ftrace: add loongarch support for kprobe args char tests Add loongarch support for the recently added kprobe args tests. Signed-off-by: Yu Liao Acked-by: Masami Hiramatsu (Google) Acked-by: Steven Rostedt (Google) Signed-off-by: Shuah Khan --- .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc index ff7499eb98d6..2b04603a3b40 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc @@ -28,6 +28,9 @@ s390*) mips*) ARG1=%r4 ;; +loongarch*) + ARG1=%r4 +;; *) echo "Please implement other architecture here" exit_untested From 11df28854bee7060508999c35afac53a0406fe7f Mon Sep 17 00:00:00 2001 From: Yu Liao Date: Wed, 18 Oct 2023 10:08:58 +0800 Subject: [PATCH 47/51] selftests/ftrace: Add riscv support for kprobe arg tests This is the riscv variant of commit 9855c4626c67 ("selftests/ftrace: Add ppc support for kprobe args tests"). Signed-off-by: Yu Liao Acked-by: Masami Hiramatsu (Google) Acked-by: Steven Rostedt (Google) Signed-off-by: Shuah Khan --- .../selftests/ftrace/test.d/kprobe/kprobe_args_char.tc | 3 +++ .../selftests/ftrace/test.d/kprobe/kprobe_args_string.tc | 3 +++ .../selftests/ftrace/test.d/kprobe/kprobe_args_syntax.tc | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc index 2b04603a3b40..21db6b720754 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc @@ -31,6 +31,9 @@ mips*) loongarch*) ARG1=%r4 ;; +riscv*) + ARG1=%a0 +;; *) echo "Please implement other architecture here" exit_untested diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc index a202b2ea4baf..4e086f871cee 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc @@ -31,6 +31,9 @@ mips*) loongarch*) ARG1=%r4 ;; +riscv*) + ARG1=%a0 +;; *) echo "Please implement other architecture here" exit_untested diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_syntax.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_syntax.tc index 1df61e13a812..8f1292ad80ff 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_syntax.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_syntax.tc @@ -44,6 +44,10 @@ loongarch*) GOODREG=%r4 BADREG=%r12 ;; +riscv*) + GOODREG=%a0 + BADREG=%a8 +;; *) echo "Please implement other architecture here" exit_untested From 3f6f8a8c5e11a9b384a36df4f40f0c9a653b6975 Mon Sep 17 00:00:00 2001 From: zhujun2 Date: Tue, 17 Oct 2023 18:59:21 -0700 Subject: [PATCH 48/51] selftests/efivarfs: create-read: fix a resource leak The opened file should be closed in main(), otherwise resource leak will occur that this problem was discovered by code reading Signed-off-by: zhujun2 Signed-off-by: Shuah Khan --- tools/testing/selftests/efivarfs/create-read.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/efivarfs/create-read.c b/tools/testing/selftests/efivarfs/create-read.c index 9674a19396a3..7bc7af4eb2c1 100644 --- a/tools/testing/selftests/efivarfs/create-read.c +++ b/tools/testing/selftests/efivarfs/create-read.c @@ -32,8 +32,10 @@ int main(int argc, char **argv) rc = read(fd, buf, sizeof(buf)); if (rc != 0) { fprintf(stderr, "Reading a new var should return EOF\n"); + close(fd); return EXIT_FAILURE; } + close(fd); return EXIT_SUCCESS; } From ecc4185a4d15373cadab41277098e651d8dd3397 Mon Sep 17 00:00:00 2001 From: zhujun2 Date: Mon, 16 Oct 2023 19:08:56 -0700 Subject: [PATCH 49/51] selftests:modify the incorrect print format when the argument type is 'unsigned int',printf '%u' in format string. Problem found during code reading. Update commit log with information on how the problem was found: Shuah Khan Signed-off-by: zhujun2 Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 5696199c16f9..a781e6311810 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -132,7 +132,7 @@ static inline void ksft_print_header(void) static inline void ksft_set_plan(unsigned int plan) { ksft_plan = plan; - printf("1..%d\n", ksft_plan); + printf("1..%u\n", ksft_plan); } static inline void ksft_print_cnts(void) @@ -140,7 +140,7 @@ static inline void ksft_print_cnts(void) if (ksft_plan != ksft_test_num()) printf("# Planned tests != run tests (%u != %u)\n", ksft_plan, ksft_test_num()); - printf("# Totals: pass:%d fail:%d xfail:%d xpass:%d skip:%d error:%d\n", + printf("# Totals: pass:%u fail:%u xfail:%u xpass:%u skip:%u error:%u\n", ksft_cnt.ksft_pass, ksft_cnt.ksft_fail, ksft_cnt.ksft_xfail, ksft_cnt.ksft_xpass, ksft_cnt.ksft_xskip, ksft_cnt.ksft_error); @@ -179,7 +179,7 @@ static inline __printf(1, 2) void ksft_test_result_pass(const char *msg, ...) ksft_cnt.ksft_pass++; va_start(args, msg); - printf("ok %d ", ksft_test_num()); + printf("ok %u ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -193,7 +193,7 @@ static inline __printf(1, 2) void ksft_test_result_fail(const char *msg, ...) ksft_cnt.ksft_fail++; va_start(args, msg); - printf("not ok %d ", ksft_test_num()); + printf("not ok %u ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -219,7 +219,7 @@ static inline __printf(1, 2) void ksft_test_result_xfail(const char *msg, ...) ksft_cnt.ksft_xfail++; va_start(args, msg); - printf("ok %d # XFAIL ", ksft_test_num()); + printf("ok %u # XFAIL ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -233,7 +233,7 @@ static inline __printf(1, 2) void ksft_test_result_skip(const char *msg, ...) ksft_cnt.ksft_xskip++; va_start(args, msg); - printf("ok %d # SKIP ", ksft_test_num()); + printf("ok %u # SKIP ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -248,7 +248,7 @@ static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...) ksft_cnt.ksft_error++; va_start(args, msg); - printf("not ok %d # error ", ksft_test_num()); + printf("not ok %u # error ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); From 34dce23f7e405ffb4eca04e83ee10c03a4cffb9b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Wed, 18 Oct 2023 00:01:56 +0100 Subject: [PATCH 50/51] selftests/clone3: Report descriptive test names The clone3() selftests currently report test results in a format that does not mesh entirely well with automation. They log output for each test such as: # [1382411] Trying clone3() with flags 0 (size 0) # I am the parent (1382411). My child's pid is 1382412 # I am the child, my PID is 1382412 # [1382411] clone3() with flags says: 0 expected 0 ok 1 [1382411] Result (0) matches expectation (0) This is not ideal for automated parsers since the text after the "ok 1" is treated as the test name when comparing runs by a lot of automation (tests routinely get renumbered due to things like new tests being added based on logical groupings). The PID means that the test names will frequently vary and the rest of the name being a description of results means several tests have identical text there. Address this by refactoring things so that we have a static descriptive name for each test which we use when logging passes, failures and skips and since we now have a stable name for the test to hand log that before starting the test to address the common issue reading logs where the test name is only printed after any diagnostics. The result is: # Running test 'simple clone3()' # [1562777] Trying clone3() with flags 0 (size 0) # I am the parent (1562777). My child's pid is 1562778 # I am the child, my PID is 1562778 # [1562777] clone3() with flags says: 0 expected 0 ok 1 simple clone3() In order to handle skips a bit more neatly this is done in a moderately invasive fashion where we move from a sequence of function calls to having an array of test parameters. This hopefully also makes it a little easier to see what the tests are doing when looking at both the source and the logs. Signed-off-by: Mark Brown Signed-off-by: Shuah Khan --- tools/testing/selftests/clone3/clone3.c | 269 +++++++++++++++++------- 1 file changed, 194 insertions(+), 75 deletions(-) diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index e60cf4da8fb0..9429d361059e 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -103,8 +104,8 @@ static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode) return 0; } -static void test_clone3(uint64_t flags, size_t size, int expected, - enum test_mode test_mode) +static bool test_clone3(uint64_t flags, size_t size, int expected, + enum test_mode test_mode) { int ret; @@ -114,92 +115,210 @@ static void test_clone3(uint64_t flags, size_t size, int expected, ret = call_clone3(flags, size, test_mode); ksft_print_msg("[%d] clone3() with flags says: %d expected %d\n", getpid(), ret, expected); - if (ret != expected) - ksft_test_result_fail( + if (ret != expected) { + ksft_print_msg( "[%d] Result (%d) is different than expected (%d)\n", getpid(), ret, expected); - else - ksft_test_result_pass( - "[%d] Result (%d) matches expectation (%d)\n", - getpid(), ret, expected); + return false; + } + + return true; } +typedef bool (*filter_function)(void); +typedef size_t (*size_function)(void); + +static bool not_root(void) +{ + if (getuid() != 0) { + ksft_print_msg("Not running as root\n"); + return true; + } + + return false; +} + +static size_t page_size_plus_8(void) +{ + return getpagesize() + 8; +} + +struct test { + const char *name; + uint64_t flags; + size_t size; + size_function size_function; + int expected; + enum test_mode test_mode; + filter_function filter; +}; + +static const struct test tests[] = { + { + .name = "simple clone3()", + .flags = 0, + .size = 0, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "clone3() in a new PID_NS", + .flags = CLONE_NEWPID, + .size = 0, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + .filter = not_root, + }, + { + .name = "CLONE_ARGS_SIZE_VER0", + .flags = 0, + .size = CLONE_ARGS_SIZE_VER0, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "CLONE_ARGS_SIZE_VER0 - 8", + .flags = 0, + .size = CLONE_ARGS_SIZE_VER0 - 8, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "sizeof(struct clone_args) + 8", + .flags = 0, + .size = sizeof(struct __clone_args) + 8, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "exit_signal with highest 32 bits non-zero", + .flags = 0, + .size = 0, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG, + }, + { + .name = "negative 32-bit exit_signal", + .flags = 0, + .size = 0, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG, + }, + { + .name = "exit_signal not fitting into CSIGNAL mask", + .flags = 0, + .size = 0, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG, + }, + { + .name = "NSIG < exit_signal < CSIG", + .flags = 0, + .size = 0, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG, + }, + { + .name = "Arguments sizeof(struct clone_args) + 8", + .flags = 0, + .size = sizeof(struct __clone_args) + 8, + .expected = 0, + .test_mode = CLONE3_ARGS_ALL_0, + }, + { + .name = "Arguments sizeof(struct clone_args) + 16", + .flags = 0, + .size = sizeof(struct __clone_args) + 16, + .expected = -E2BIG, + .test_mode = CLONE3_ARGS_ALL_0, + }, + { + .name = "Arguments sizeof(struct clone_arg) * 2", + .flags = 0, + .size = sizeof(struct __clone_args) + 16, + .expected = -E2BIG, + .test_mode = CLONE3_ARGS_ALL_0, + }, + { + .name = "Arguments > page size", + .flags = 0, + .size_function = page_size_plus_8, + .expected = -E2BIG, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "CLONE_ARGS_SIZE_VER0 in a new PID NS", + .flags = CLONE_NEWPID, + .size = CLONE_ARGS_SIZE_VER0, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + .filter = not_root, + }, + { + .name = "CLONE_ARGS_SIZE_VER0 - 8 in a new PID NS", + .flags = CLONE_NEWPID, + .size = CLONE_ARGS_SIZE_VER0 - 8, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "sizeof(struct clone_args) + 8 in a new PID NS", + .flags = CLONE_NEWPID, + .size = sizeof(struct __clone_args) + 8, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + .filter = not_root, + }, + { + .name = "Arguments > page size in a new PID NS", + .flags = CLONE_NEWPID, + .size_function = page_size_plus_8, + .expected = -E2BIG, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "New time NS", + .flags = CLONE_NEWTIME, + .size = 0, + .expected = 0, + .test_mode = CLONE3_ARGS_NO_TEST, + }, + { + .name = "exit signal (SIGCHLD) in flags", + .flags = SIGCHLD, + .size = 0, + .expected = -EINVAL, + .test_mode = CLONE3_ARGS_NO_TEST, + }, +}; + int main(int argc, char *argv[]) { - uid_t uid = getuid(); + size_t size; + int i; ksft_print_header(); - ksft_set_plan(19); + ksft_set_plan(ARRAY_SIZE(tests)); test_clone3_supported(); - /* Just a simple clone3() should return 0.*/ - test_clone3(0, 0, 0, CLONE3_ARGS_NO_TEST); + for (i = 0; i < ARRAY_SIZE(tests); i++) { + if (tests[i].filter && tests[i].filter()) { + ksft_test_result_skip("%s\n", tests[i].name); + continue; + } - /* Do a clone3() in a new PID NS.*/ - if (uid == 0) - test_clone3(CLONE_NEWPID, 0, 0, CLONE3_ARGS_NO_TEST); - else - ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); + if (tests[i].size_function) + size = tests[i].size_function(); + else + size = tests[i].size; - /* Do a clone3() with CLONE_ARGS_SIZE_VER0. */ - test_clone3(0, CLONE_ARGS_SIZE_VER0, 0, CLONE3_ARGS_NO_TEST); + ksft_print_msg("Running test '%s'\n", tests[i].name); - /* Do a clone3() with CLONE_ARGS_SIZE_VER0 - 8 */ - test_clone3(0, CLONE_ARGS_SIZE_VER0 - 8, -EINVAL, CLONE3_ARGS_NO_TEST); - - /* Do a clone3() with sizeof(struct clone_args) + 8 */ - test_clone3(0, sizeof(struct __clone_args) + 8, 0, CLONE3_ARGS_NO_TEST); - - /* Do a clone3() with exit_signal having highest 32 bits non-zero */ - test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG); - - /* Do a clone3() with negative 32-bit exit_signal */ - test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG); - - /* Do a clone3() with exit_signal not fitting into CSIGNAL mask */ - test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG); - - /* Do a clone3() with NSIG < exit_signal < CSIG */ - test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG); - - test_clone3(0, sizeof(struct __clone_args) + 8, 0, CLONE3_ARGS_ALL_0); - - test_clone3(0, sizeof(struct __clone_args) + 16, -E2BIG, - CLONE3_ARGS_ALL_0); - - test_clone3(0, sizeof(struct __clone_args) * 2, -E2BIG, - CLONE3_ARGS_ALL_0); - - /* Do a clone3() with > page size */ - test_clone3(0, getpagesize() + 8, -E2BIG, CLONE3_ARGS_NO_TEST); - - /* Do a clone3() with CLONE_ARGS_SIZE_VER0 in a new PID NS. */ - if (uid == 0) - test_clone3(CLONE_NEWPID, CLONE_ARGS_SIZE_VER0, 0, - CLONE3_ARGS_NO_TEST); - else - ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); - - /* Do a clone3() with CLONE_ARGS_SIZE_VER0 - 8 in a new PID NS */ - test_clone3(CLONE_NEWPID, CLONE_ARGS_SIZE_VER0 - 8, -EINVAL, - CLONE3_ARGS_NO_TEST); - - /* Do a clone3() with sizeof(struct clone_args) + 8 in a new PID NS */ - if (uid == 0) - test_clone3(CLONE_NEWPID, sizeof(struct __clone_args) + 8, 0, - CLONE3_ARGS_NO_TEST); - else - ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); - - /* Do a clone3() with > page size in a new PID NS */ - test_clone3(CLONE_NEWPID, getpagesize() + 8, -E2BIG, - CLONE3_ARGS_NO_TEST); - - /* Do a clone3() in a new time namespace */ - test_clone3(CLONE_NEWTIME, 0, 0, CLONE3_ARGS_NO_TEST); - - /* Do a clone3() with exit signal (SIGCHLD) in flags */ - test_clone3(SIGCHLD, 0, -EINVAL, CLONE3_ARGS_NO_TEST); + ksft_test_result(test_clone3(tests[i].flags, size, + tests[i].expected, + tests[i].test_mode), + "%s\n", tests[i].name); + } ksft_finished(); } From 5247e6dbed0041147a83137f89cd45043301de5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Wed, 18 Oct 2023 10:55:26 +0300 Subject: [PATCH 51/51] selftests/resctrl: Fix MBM test failure when MBA unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check") exposed a problem in feature detection logic in MBM selftest. If schemata does not support MB:x=x entries, the schemata write to initialize 100% memory bandwidth allocation in mbm_setup() will now fail with -EINVAL due to the error handling corrected by the commit 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check"). That commit just uncovers the failed write, it is not wrong itself. If MB:x=x is not supported by schemata, it is safe to assume 100% memory bandwidth is always set. Therefore, the previously ignored error does not make the MBM test itself wrong. Restore the previous behavior of MBM test by checking MB support before attempting to write it into schemata which results in behavior equivalent to ignoring the write error. Fixes: 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check") Signed-off-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Shuah Khan --- tools/testing/selftests/resctrl/mbm_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index d3c0d30c676a..741533f2b075 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -95,7 +95,7 @@ static int mbm_setup(struct resctrl_val_param *p) return END_OF_TESTS; /* Set up shemata with 100% allocation on the first run. */ - if (p->num_of_runs == 0) + if (p->num_of_runs == 0 && validate_resctrl_feature_request("MB", NULL)) ret = write_schemata(p->ctrlgrp, "100", p->cpu_no, p->resctrl_val);