* [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure
@ 2021-11-17 7:07 Li Wang
2021-11-17 7:07 ` [LTP] [PATCH v5 2/3] testcase: make use of .supported_archs Li Wang
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Li Wang @ 2021-11-17 7:07 UTC (permalink / raw)
To: ltp
Testcases for specific arch should be limited on that only being supported
platform to run, we now involve a .supported_archs to achieve this feature
in LTP library. All you need to run a test on the expected arch is to set
the '.supported_archs' array in the 'struct tst_test' to choose the required
arch list. e.g.
.supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
This helps move the TCONF info from code to tst_test metadata as well.
And, we also export a struct tst_arch to save the system architecture
for using in the whole test cases.
extern const struct tst_arch {
char name[16];
enum tst_arch_type type;
} tst_arch;
Signed-off-by: Li Wang <liwang@redhat.com>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
---
doc/c-test-api.txt | 36 +++++++++++++++++
include/tst_arch.h | 39 +++++++++++++++++++
include/tst_test.h | 7 ++++
lib/tst_arch.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++
lib/tst_test.c | 3 ++
5 files changed, 181 insertions(+)
create mode 100644 include/tst_arch.h
create mode 100644 lib/tst_arch.c
diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 3127018a4..64d0630ce 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -2261,6 +2261,42 @@ struct tst_test test = {
};
-------------------------------------------------------------------------------
+1.39 Testing on the specific architecture
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Testcases for specific arch should be limited on that only being supported
+platform to run, we now involve a .supported_archs to achieve this feature
+in LTP library. All you need to run a test on the expected arch is to set
+the '.supported_archs' array in the 'struct tst_test' to choose the required
+arch list. e.g.
+
+ .supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
+
+This helps move the TCONF info from code to tst_test metadata as well.
+
+And, we also export a struct tst_arch to save the system architecture for
+using in the whole test cases.
+
+ extern const struct tst_arch {
+ char name[16];
+ enum tst_arch_type type;
+ } tst_arch;
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static struct tst_test test = {
+ ...
+ .setup = setup,
+ .supported_archs = (const char *const []) {
+ "x86_64",
+ "ppc64",
+ "s390x",
+ NULL
+ },
+};
+-------------------------------------------------------------------------------
+
2. Common problems
------------------
diff --git a/include/tst_arch.h b/include/tst_arch.h
new file mode 100644
index 000000000..4a9473c6f
--- /dev/null
+++ b/include/tst_arch.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2021 Li Wang <liwang@redhat.com>
+ */
+
+#ifndef TST_ARCH_H__
+#define TST_ARCH_H__
+
+enum tst_arch_type {
+ TST_UNKNOWN,
+ TST_X86,
+ TST_X86_64,
+ TST_IA64,
+ TST_PPC,
+ TST_PPC64,
+ TST_S390,
+ TST_S390X,
+ TST_ARM,
+ TST_AARCH64,
+ TST_SPARC,
+};
+
+/*
+ * This tst_arch is to save the system architecture for
+ * using in the whole testcase.
+ */
+extern const struct tst_arch {
+ char name[16];
+ enum tst_arch_type type;
+} tst_arch;
+
+/*
+ * Check if test platform is in the given arch list. If yes return 1,
+ * otherwise return 0.
+ *
+ * @archlist A NULL terminated array of architectures to support.
+ */
+int tst_is_on_arch(const char *const *archlist);
+
+#endif /* TST_ARCH_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 602ce3090..c06a4729b 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -43,6 +43,7 @@
#include "tst_fips.h"
#include "tst_taint.h"
#include "tst_memutils.h"
+#include "tst_arch.h"
/*
* Reports testcase result.
@@ -139,6 +140,12 @@ struct tst_test {
const char *min_kver;
+ /*
+ * The supported_archs is a NULL terminated list of archs the test
+ * does support.
+ */
+ const char *const *supported_archs;
+
/* If set the test is compiled out */
const char *tconf_msg;
diff --git a/lib/tst_arch.c b/lib/tst_arch.c
new file mode 100644
index 000000000..f19802a03
--- /dev/null
+++ b/lib/tst_arch.c
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2021 Li Wang <liwang@redhat.com>
+ */
+
+#include <string.h>
+#include <stdlib.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_arch.h"
+#include "tst_test.h"
+
+const struct tst_arch tst_arch = {
+#if defined(__x86_64__)
+ .name = "x86_64",
+ .type = TST_X86_64,
+#elif defined(__i386__) || defined(__i586__) || defined(__i686__)
+ .name = "x86",
+ .type = TST_X86,
+#elif defined(__ia64__)
+ .name = "ia64",
+ .type = TST_IA64,
+#elif defined(__powerpc64__) || defined(__ppc64__)
+ .name = "ppc64",
+ .type = TST_PPC64,
+#elif defined(__powerpc__) || defined(__ppc__)
+ .name = "ppc",
+ .type = TST_PPC,
+#elif defined(__s390x__)
+ .name = "s390x",
+ .type = TST_S390X,
+#elif defined(__s390__)
+ .name = "s390",
+ .type = TST_S390,
+#elif defined(__aarch64__)
+ .name = "aarch64",
+ .type = TST_AARCH64,
+#elif defined(__arm__)
+ .name = "arm",
+ .type = TST_ARM,
+#elif defined(__sparc__)
+ .name = "sparc",
+ .type = TST_SPARC,
+#else
+ .name = "unknown",
+ .type = TST_UNKNOWN,
+#endif
+};
+
+static const char *const arch_type_list[] = {
+ "i386",
+ "i586",
+ "i686",
+ "x86_64",
+ "ia64",
+ "ppc",
+ "ppc64",
+ "s390",
+ "s390x",
+ "arm",
+ "aarch64",
+ "sparc",
+ NULL
+};
+
+static int is_valid_arch_name(const char *name)
+{
+ unsigned int i;
+
+ for (i = 0; arch_type_list[i]; i++) {
+ if (!strcmp(arch_type_list[i], name))
+ return 1;
+ }
+
+ return 0;
+}
+
+int tst_is_on_arch(const char *const *archlist)
+{
+ unsigned int i;
+
+ if (!archlist)
+ return 1;
+
+ for (i = 0; archlist[i]; i++) {
+ if (!is_valid_arch_name(archlist[i]))
+ tst_brk(TBROK, "%s is invalid arch, please reset!",
+ archlist[i]);
+ }
+
+ for (i = 0; archlist[i]; i++) {
+ if (!strcmp(tst_arch.name, archlist[i]))
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index d7f9e0f97..a79275722 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -976,6 +976,9 @@ static void do_setup(int argc, char *argv[])
if (tst_test->min_kver)
check_kver();
+ if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
+ tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
+
if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
tst_brk(TCONF, "Kernel is locked down, skipping test");
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 18+ messages in thread* [LTP] [PATCH v5 2/3] testcase: make use of .supported_archs 2021-11-17 7:07 [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Li Wang @ 2021-11-17 7:07 ` Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch Li Wang ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Li Wang @ 2021-11-17 7:07 UTC (permalink / raw) To: ltp In some places, we have to keep ifdefs to make the compiler work with unportable code. Signed-off-by: Li Wang <liwang@redhat.com> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> --- configure.ac | 1 + testcases/cve/meltdown.c | 16 ++++++----- testcases/kernel/syscalls/ptrace/ptrace07.c | 11 ++++---- testcases/kernel/syscalls/ptrace/ptrace08.c | 22 ++++++++------- testcases/kernel/syscalls/ptrace/ptrace09.c | 11 +++++--- testcases/kernel/syscalls/ptrace/ptrace10.c | 12 +++++---- .../syscalls/set_mempolicy/set_mempolicy05.c | 27 +++++++++---------- 7 files changed, 54 insertions(+), 46 deletions(-) diff --git a/configure.ac b/configure.ac index 859aa9857..9af32c859 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,7 @@ AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>]) AC_CHECK_HEADERS_ONCE([ \ asm/ldt.h \ + emmintrin.h \ ifaddrs.h \ keyutils.h \ linux/can.h \ diff --git a/testcases/cve/meltdown.c b/testcases/cve/meltdown.c index 5a984aba3..2577c1a80 100644 --- a/testcases/cve/meltdown.c +++ b/testcases/cve/meltdown.c @@ -6,8 +6,6 @@ #include "config.h" #include "tst_test.h" -#if defined(__x86_64__) || defined(__i386__) - #include <stdio.h> #include <string.h> #include <signal.h> @@ -17,6 +15,7 @@ #include <ctype.h> #include <sys/utsname.h> +#ifdef HAVE_EMMINTRIN_H #include <emmintrin.h> #include "tst_tsc.h" @@ -378,14 +377,17 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .min_kver = "2.6.32", + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"CVE", "2017-5754"}, {} } }; -#else /* #if defined(__x86_64__) || defined(__i386__) */ - -TST_TEST_TCONF("not x86_64 or i386"); - -#endif /* #else #if defined(__x86_64__) || defined(__i386__) */ +#else /* HAVE_EMMINTRIN_H */ + TST_TEST_TCONF("<emmintrin.h> is not supported"); +#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c b/testcases/kernel/syscalls/ptrace/ptrace07.c index 9e3f7511d..da62cadb0 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace07.c +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c @@ -48,13 +48,13 @@ # define NT_X86_XSTATE 0x202 #endif -#ifdef __x86_64__ static void check_regs_loop(uint32_t initval) { const unsigned long num_iters = 1000000000; uint32_t xmm0[4] = { initval, initval, initval, initval }; int status = 1; +#ifdef __x86_64__ asm volatile(" movdqu %0, %%xmm0\n" " mov %0, %%rbx\n" "1: dec %2\n" @@ -68,6 +68,7 @@ static void check_regs_loop(uint32_t initval) "3:\n" : "+m" (xmm0), "+r" (status) : "r" (num_iters) : "rax", "rbx", "xmm0"); +#endif if (status) { tst_res(TFAIL, @@ -178,6 +179,10 @@ static struct tst_test test = { .test_all = do_test, .forks_child = 1, .needs_checkpoints = 1, + .supported_archs = (const char *const []) { + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "814fb7bb7db5"}, {"CVE", "2017-15537"}, @@ -185,7 +190,3 @@ static struct tst_test test = { } }; - -#else /* !__x86_64__ */ - TST_TEST_TCONF("this test is only supported on x86_64"); -#endif /* __x86_64__ */ diff --git a/testcases/kernel/syscalls/ptrace/ptrace08.c b/testcases/kernel/syscalls/ptrace/ptrace08.c index 170cae64c..20df39a9b 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace08.c +++ b/testcases/kernel/syscalls/ptrace/ptrace08.c @@ -43,18 +43,16 @@ #include "tst_test.h" #include "tst_safe_stdio.h" -#if defined(__i386__) || defined(__x86_64__) - static pid_t child_pid; -#if defined(__x86_64__) -# define KERN_ADDR_MIN 0xffff800000000000 -# define KERN_ADDR_MAX 0xffffffffffffffff -# define KERN_ADDR_BITS 64 -#elif defined(__i386__) +#if defined(__i386__) # define KERN_ADDR_MIN 0xc0000000 # define KERN_ADDR_MAX 0xffffffff # define KERN_ADDR_BITS 32 +#else +# define KERN_ADDR_MIN 0xffff800000000000 +# define KERN_ADDR_MAX 0xffffffffffffffff +# define KERN_ADDR_BITS 64 #endif static int deffered_check; @@ -98,6 +96,7 @@ static void ptrace_try_kern_addr(unsigned long kern_addr) if (SAFE_WAITPID(child_pid, &status, WUNTRACED) != child_pid) tst_brk(TBROK, "Received event from unexpected PID"); +#if defined(__i386__) || defined(__x86_64__) SAFE_PTRACE(PTRACE_ATTACH, child_pid, NULL, NULL); SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[0]), (void *)1); @@ -127,6 +126,7 @@ static void ptrace_try_kern_addr(unsigned long kern_addr) addr = ptrace(PTRACE_PEEKUSER, child_pid, (void*)offsetof(struct user, u_debugreg[0]), NULL); +#endif if (!deffered_check && addr == kern_addr) tst_res(TFAIL, "Was able to set breakpoint on kernel addr"); @@ -161,6 +161,11 @@ static struct tst_test test = { * have to skip the test. */ .skip_in_compat = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "f67b15037a7a"}, {"CVE", "2018-1000199"}, @@ -168,6 +173,3 @@ static struct tst_test test = { {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace09.c b/testcases/kernel/syscalls/ptrace/ptrace09.c index 85875ce65..8ccdfcc4b 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace09.c +++ b/testcases/kernel/syscalls/ptrace/ptrace09.c @@ -22,7 +22,6 @@ #include <signal.h> #include "tst_test.h" -#if defined(__i386__) || defined(__x86_64__) static short watchpoint; static pid_t child_pid; @@ -46,6 +45,7 @@ static void run(void) { int status; +#if defined(__i386__) || defined(__x86_64__) child_pid = SAFE_FORK(); if (!child_pid) { @@ -60,6 +60,7 @@ static void run(void) SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[7]), (void *)0x30001); SAFE_PTRACE(PTRACE_CONT, child_pid, NULL, NULL); +#endif while (1) { if (SAFE_WAITPID(child_pid, &status, 0) != child_pid) @@ -92,12 +93,14 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .forks_child = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "d8ba61ba58c8"}, {"CVE", "2018-8897"}, {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/ptrace/ptrace10.c b/testcases/kernel/syscalls/ptrace/ptrace10.c index b5d6b9f8f..3bd8ca1a9 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace10.c +++ b/testcases/kernel/syscalls/ptrace/ptrace10.c @@ -22,8 +22,6 @@ #include <signal.h> #include "tst_test.h" -#if defined(__i386__) || defined(__x86_64__) - static pid_t child_pid; static void child_main(void) @@ -45,6 +43,7 @@ static void run(void) if (SAFE_WAITPID(child_pid, &status, WUNTRACED) != child_pid) tst_brk(TBROK, "Received event from unexpected PID"); +#if defined(__i386__) || defined(__x86_64__) SAFE_PTRACE(PTRACE_ATTACH, child_pid, NULL, NULL); SAFE_PTRACE(PTRACE_POKEUSER, child_pid, (void *)offsetof(struct user, u_debugreg[0]), (void *)1); @@ -53,6 +52,7 @@ static void run(void) addr = ptrace(PTRACE_PEEKUSER, child_pid, (void*)offsetof(struct user, u_debugreg[0]), NULL); +#endif if (addr == 2) tst_res(TPASS, "The rd0 was set on second PTRACE_POKEUSR"); @@ -76,11 +76,13 @@ static struct tst_test test = { .test_all = run, .cleanup = cleanup, .forks_child = 1, + .supported_archs = (const char *const []) { + "i386", + "x86_64", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "bd14406b78e6"}, {} } }; -#else -TST_TEST_TCONF("This test is only supported on x86 systems"); -#endif diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c index 86f6a95dc..c65cb1e18 100644 --- a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy05.c @@ -37,18 +37,10 @@ #include "config.h" #include "tst_test.h" -#if defined(__i386__) || defined(__powerpc__) - #include <string.h> static unsigned int i; static int sys_ret; -#ifdef __i386__ -static const int sys_num = 276; -static const int mode; -static const int node_mask_ptr = UINT_MAX; -static const int node_mask_sz = UINT_MAX; -#endif static volatile char *stack_ptr; static void run(void) @@ -58,6 +50,11 @@ static void run(void) register long mode __asm__("r3"); register long node_mask_ptr __asm__("r4"); register long node_mask_sz __asm__("r5"); +#else + const int sys_num = 276; + const int mode; + const int node_mask_ptr = UINT_MAX; + const int node_mask_sz = UINT_MAX; #endif char stack_pattern[0x400]; @@ -78,7 +75,8 @@ static void run(void) : "memory", "cr0", "r6", "r7", "r8", "r9", "r10", "r11", "r12"); sys_ret = mode; -#else /* __i386__ */ +#endif +#ifdef __i386__ asm volatile ( "add $0x400, %%esp\n\t" "int $0x80\n\t" @@ -114,15 +112,14 @@ static void run(void) static struct tst_test test = { .test_all = run, + .supported_archs = (const char *const []) { + "i386", + "ppc", + NULL + }, .tags = (const struct tst_tag[]) { {"linux-git", "cf01fb9985e8"}, {"CVE", "CVE-2017-7616"}, {} } }; - -#else /* #if defined(__x86_64__) || defined(__powerpc__) */ - -TST_TEST_TCONF("not i386 or powerpc"); - -#endif /* #else #if defined(__x86_64__) || defined(__powerpc__) */ -- 2.31.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-17 7:07 [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 2/3] testcase: make use of .supported_archs Li Wang @ 2021-11-17 7:07 ` Li Wang 2021-11-17 10:33 ` Richard Palethorpe 2021-11-17 9:58 ` [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Richard Palethorpe 2021-11-17 13:59 ` Joerg Vehlow 3 siblings, 1 reply; 18+ messages in thread From: Li Wang @ 2021-11-17 7:07 UTC (permalink / raw) To: ltp Signed-off-by: Li Wang <liwang@redhat.com> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/mem/tunable/max_map_count.c | 46 ++++++++++---------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c index 4f0ad0037..a4c3dbf8e 100644 --- a/testcases/kernel/mem/tunable/max_map_count.c +++ b/testcases/kernel/mem/tunable/max_map_count.c @@ -55,7 +55,6 @@ static long old_max_map_count = -1; static long old_overcommit = -1; -static struct utsname un; static void setup(void) { @@ -66,9 +65,6 @@ static void setup(void) old_max_map_count = get_sys_tune("max_map_count"); old_overcommit = get_sys_tune("overcommit_memory"); set_sys_tune("overcommit_memory", 0, 1); - - if (uname(&un) != 0) - tst_brk(TBROK | TERRNO, "uname error"); } static void cleanup(void) @@ -91,24 +87,30 @@ static bool filter_map(const char *line) if (ret != 1) return false; -#if defined(__x86_64__) || defined(__x86__) - /* On x86, there's an old compat vsyscall page */ - if (!strcmp(buf, "[vsyscall]")) - return true; -#elif defined(__ia64__) - /* On ia64, the vdso is not a proper mapping */ - if (!strcmp(buf, "[vdso]")) - return true; -#elif defined(__arm__) - /* Skip it when run it in aarch64 */ - if ((!strcmp(un.machine, "aarch64")) - || (!strcmp(un.machine, "aarch64_be"))) - return false; - - /* Older arm kernels didn't label their vdso maps */ - if (!strncmp(line, "ffff0000-ffff1000", 17)) - return true; -#endif + switch (tst_arch.type) { + case TST_X86: + case TST_X86_64: + /* On x86, there's an old compat vsyscall page */ + if (!strcmp(buf, "[vsyscall]")) + return true; + break; + case TST_IA64: + /* On ia64, the vdso is not a proper mapping */ + if (!strcmp(buf, "[vdso]")) + return true; + break; + case TST_ARM: + /* Skip it when run it in aarch64 */ + if (tst_kernel_bits() == 64) + return false; + + /* Older arm kernels didn't label their vdso maps */ + if (!strncmp(line, "ffff0000-ffff1000", 17)) + return true; + break; + default: + break; + }; return false; } -- 2.31.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-17 7:07 ` [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch Li Wang @ 2021-11-17 10:33 ` Richard Palethorpe 2021-11-17 13:28 ` Li Wang 0 siblings, 1 reply; 18+ messages in thread From: Richard Palethorpe @ 2021-11-17 10:33 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hello Li, Li Wang <liwang@redhat.com> writes: > Signed-off-by: Li Wang <liwang@redhat.com> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > --- > testcases/kernel/mem/tunable/max_map_count.c | 46 ++++++++++---------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c > index 4f0ad0037..a4c3dbf8e 100644 > --- a/testcases/kernel/mem/tunable/max_map_count.c > +++ b/testcases/kernel/mem/tunable/max_map_count.c > @@ -55,7 +55,6 @@ > > static long old_max_map_count = -1; > static long old_overcommit = -1; > -static struct utsname un; > > static void setup(void) > { > @@ -66,9 +65,6 @@ static void setup(void) > old_max_map_count = get_sys_tune("max_map_count"); > old_overcommit = get_sys_tune("overcommit_memory"); > set_sys_tune("overcommit_memory", 0, 1); > - > - if (uname(&un) != 0) > - tst_brk(TBROK | TERRNO, "uname error"); > } > > static void cleanup(void) > @@ -91,24 +87,30 @@ static bool filter_map(const char *line) > if (ret != 1) > return false; > > -#if defined(__x86_64__) || defined(__x86__) > - /* On x86, there's an old compat vsyscall page */ > - if (!strcmp(buf, "[vsyscall]")) > - return true; > -#elif defined(__ia64__) > - /* On ia64, the vdso is not a proper mapping */ > - if (!strcmp(buf, "[vdso]")) > - return true; > -#elif defined(__arm__) > - /* Skip it when run it in aarch64 */ > - if ((!strcmp(un.machine, "aarch64")) > - || (!strcmp(un.machine, "aarch64_be"))) > - return false; > - > - /* Older arm kernels didn't label their vdso maps */ > - if (!strncmp(line, "ffff0000-ffff1000", 17)) > - return true; > -#endif > + switch (tst_arch.type) { > + case TST_X86: > + case TST_X86_64: > + /* On x86, there's an old compat vsyscall page */ > + if (!strcmp(buf, "[vsyscall]")) > + return true; > + break; > + case TST_IA64: > + /* On ia64, the vdso is not a proper mapping */ > + if (!strcmp(buf, "[vdso]")) > + return true; > + break; > + case TST_ARM: > + /* Skip it when run it in aarch64 */ This should not be possible. If TST_ARM is set then how can we be on aarch64? We also have TST_AARCH64. > + if (tst_kernel_bits() == 64) > + return false; > + > + /* Older arm kernels didn't label their vdso maps */ > + if (!strncmp(line, "ffff0000-ffff1000", 17)) > + return true; > + break; > + default: > + break; > + }; > > return false; > } > -- > 2.31.1 -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-17 10:33 ` Richard Palethorpe @ 2021-11-17 13:28 ` Li Wang 2021-11-17 13:58 ` Richard Palethorpe 0 siblings, 1 reply; 18+ messages in thread From: Li Wang @ 2021-11-17 13:28 UTC (permalink / raw) To: Richard Palethorpe; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 632 bytes --] Hi Richard, > > + case TST_IA64: > > + /* On ia64, the vdso is not a proper mapping */ > > + if (!strcmp(buf, "[vdso]")) > > + return true; > > + break; > > + case TST_ARM: > > + /* Skip it when run it in aarch64 */ > > This should not be possible. If TST_ARM is set then how can we be on > aarch64? We also have TST_AARCH64. > Not exactly, I was thinking like this before, but as Cyril point that there is a possible 32bit ARM binary runs on 64bit aarch64 kernel. https://lists.linux.it/pipermail/ltp/2021-November/025925.html -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 1430 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch 2021-11-17 13:28 ` Li Wang @ 2021-11-17 13:58 ` Richard Palethorpe 0 siblings, 0 replies; 18+ messages in thread From: Richard Palethorpe @ 2021-11-17 13:58 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hello Li, Li Wang <liwang@redhat.com> writes: > Hi Richard, > > > > + case TST_IA64: > > + /* On ia64, the vdso is not a proper mapping */ > > + if (!strcmp(buf, "[vdso]")) > > + return true; > > + break; > > + case TST_ARM: > > + /* Skip it when run it in aarch64 */ > > This should not be possible. If TST_ARM is set then how can we be on > aarch64? We also have TST_AARCH64. > > Not exactly, I was thinking like this before, but as Cyril point that there is > a possible 32bit ARM binary runs on 64bit aarch64 kernel. > > https://lists.linux.it/pipermail/ltp/2021-November/025925.html Thanks Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com> -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 7:07 [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 2/3] testcase: make use of .supported_archs Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch Li Wang @ 2021-11-17 9:58 ` Richard Palethorpe 2021-11-17 13:51 ` Li Wang 2021-11-17 13:59 ` Joerg Vehlow 3 siblings, 1 reply; 18+ messages in thread From: Richard Palethorpe @ 2021-11-17 9:58 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hello Li, Li Wang <liwang@redhat.com> writes: > Testcases for specific arch should be limited on that only being supported > platform to run, we now involve a .supported_archs to achieve this feature > in LTP library. All you need to run a test on the expected arch is to set > the '.supported_archs' array in the 'struct tst_test' to choose the required > arch list. e.g. > > .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > > This helps move the TCONF info from code to tst_test metadata as well. > > And, we also export a struct tst_arch to save the system architecture > for using in the whole test cases. > > extern const struct tst_arch { > char name[16]; > enum tst_arch_type type; > } tst_arch; > > Signed-off-by: Li Wang <liwang@redhat.com> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > --- > doc/c-test-api.txt | 36 +++++++++++++++++ > include/tst_arch.h | 39 +++++++++++++++++++ > include/tst_test.h | 7 ++++ > lib/tst_arch.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++ > lib/tst_test.c | 3 ++ > 5 files changed, 181 insertions(+) > create mode 100644 include/tst_arch.h > create mode 100644 lib/tst_arch.c > > diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt > index 3127018a4..64d0630ce 100644 > --- a/doc/c-test-api.txt > +++ b/doc/c-test-api.txt > @@ -2261,6 +2261,42 @@ struct tst_test test = { > }; > ------------------------------------------------------------------------------- > > +1.39 Testing on the specific architecture > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > +Testcases for specific arch should be limited on that only being supported > +platform to run, we now involve a .supported_archs to achieve this feature > +in LTP library. All you need to run a test on the expected arch is to set > +the '.supported_archs' array in the 'struct tst_test' to choose the required > +arch list. e.g. > + > + .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > + > +This helps move the TCONF info from code to tst_test metadata as well. > + > +And, we also export a struct tst_arch to save the system architecture for > +using in the whole test cases. > + > + extern const struct tst_arch { > + char name[16]; > + enum tst_arch_type type; > + } tst_arch; > + > +[source,c] > +------------------------------------------------------------------------------- > +#include "tst_test.h" > + > +static struct tst_test test = { > + ... > + .setup = setup, > + .supported_archs = (const char *const []) { > + "x86_64", > + "ppc64", > + "s390x", > + NULL > + }, > +}; > +------------------------------------------------------------------------------- > + > 2. Common problems > ------------------ > > diff --git a/include/tst_arch.h b/include/tst_arch.h > new file mode 100644 > index 000000000..4a9473c6f > --- /dev/null > +++ b/include/tst_arch.h > @@ -0,0 +1,39 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (c) 2021 Li Wang <liwang@redhat.com> > + */ > + > +#ifndef TST_ARCH_H__ > +#define TST_ARCH_H__ > + > +enum tst_arch_type { > + TST_UNKNOWN, > + TST_X86, > + TST_X86_64, > + TST_IA64, > + TST_PPC, > + TST_PPC64, > + TST_S390, > + TST_S390X, > + TST_ARM, > + TST_AARCH64, > + TST_SPARC, > +}; > + > +/* > + * This tst_arch is to save the system architecture for > + * using in the whole testcase. > + */ > +extern const struct tst_arch { > + char name[16]; > + enum tst_arch_type type; > +} tst_arch; > + > +/* > + * Check if test platform is in the given arch list. If yes return 1, > + * otherwise return 0. > + * > + * @archlist A NULL terminated array of architectures to support. > + */ > +int tst_is_on_arch(const char *const *archlist); > + > +#endif /* TST_ARCH_H__ */ > diff --git a/include/tst_test.h b/include/tst_test.h > index 602ce3090..c06a4729b 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -43,6 +43,7 @@ > #include "tst_fips.h" > #include "tst_taint.h" > #include "tst_memutils.h" > +#include "tst_arch.h" > > /* > * Reports testcase result. > @@ -139,6 +140,12 @@ struct tst_test { > > const char *min_kver; > > + /* > + * The supported_archs is a NULL terminated list of archs the test > + * does support. > + */ > + const char *const *supported_archs; > + > /* If set the test is compiled out */ > const char *tconf_msg; > > diff --git a/lib/tst_arch.c b/lib/tst_arch.c > new file mode 100644 > index 000000000..f19802a03 > --- /dev/null > +++ b/lib/tst_arch.c > @@ -0,0 +1,96 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (c) 2021 Li Wang <liwang@redhat.com> > + */ > + > +#include <string.h> > +#include <stdlib.h> > + > +#define TST_NO_DEFAULT_MAIN > +#include "tst_arch.h" > +#include "tst_test.h" > + > +const struct tst_arch tst_arch = { > +#if defined(__x86_64__) > + .name = "x86_64", Writing out these string constants multiple times is error prone. Perhaps arch_type_list can be indexed with enum tst_arch_type and then name can be ".name = arch_type_list[TST_X86]"? > + .type = TST_X86_64, > +#elif defined(__i386__) || defined(__i586__) || defined(__i686__) > + .name = "x86", > + .type = TST_X86, > +#elif defined(__ia64__) > + .name = "ia64", > + .type = TST_IA64, > +#elif defined(__powerpc64__) || defined(__ppc64__) > + .name = "ppc64", > + .type = TST_PPC64, > +#elif defined(__powerpc__) || defined(__ppc__) > + .name = "ppc", > + .type = TST_PPC, > +#elif defined(__s390x__) > + .name = "s390x", > + .type = TST_S390X, > +#elif defined(__s390__) > + .name = "s390", > + .type = TST_S390, > +#elif defined(__aarch64__) > + .name = "aarch64", > + .type = TST_AARCH64, > +#elif defined(__arm__) > + .name = "arm", > + .type = TST_ARM, > +#elif defined(__sparc__) > + .name = "sparc", > + .type = TST_SPARC, > +#else > + .name = "unknown", > + .type = TST_UNKNOWN, > +#endif > +}; > + > +static const char *const arch_type_list[] = { > + "i386", > + "i586", > + "i686", These are not valid arch names AFAICT. There is no mapping from these to x86 in the tst_arch table above. Perhaps we could replace them all with x86? e.g [TST_X86] = "x86", [TST_X86_64] = "x86_64", etc... > + "x86_64", > + "ia64", > + "ppc", > + "ppc64", > + "s390", > + "s390x", > + "arm", > + "aarch64", > + "sparc", > + NULL > +}; > + -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 9:58 ` [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Richard Palethorpe @ 2021-11-17 13:51 ` Li Wang 2021-11-17 14:01 ` Richard Palethorpe 0 siblings, 1 reply; 18+ messages in thread From: Li Wang @ 2021-11-17 13:51 UTC (permalink / raw) To: Richard Palethorpe; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 1104 bytes --] Hi Richard, > > +const struct tst_arch tst_arch = { > > +#if defined(__x86_64__) > > + .name = "x86_64", > > Writing out these string constants multiple times is error > prone. Perhaps arch_type_list can be indexed with enum tst_arch_type and > then name can be ".name = arch_type_list[TST_X86]"? > Right, that will more flexible but you know, all arches we have are just those, and we write them only once in the LTP test library. I slightly wanted to keep string constants to make it more simple/readable. > > +static const char *const arch_type_list[] = { > > + "i386", > > + "i586", > > + "i686", > > These are not valid arch names AFAICT. There is no mapping from these to > x86 in the tst_arch table above. > > Perhaps we could replace them all with x86? > Yes, that is also the unsure part I was concerned about in patch v4. Because x86 is also an invalid arch (it is just a conventional name), if we use it in the arch_type_list we have to recognize it as a valid arch name in test case as well. I'm not sure that will be accepted by other people. -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 2336 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 13:51 ` Li Wang @ 2021-11-17 14:01 ` Richard Palethorpe 2021-11-18 11:42 ` Cyril Hrubis 0 siblings, 1 reply; 18+ messages in thread From: Richard Palethorpe @ 2021-11-17 14:01 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hello Li, Li Wang <liwang@redhat.com> writes: > Hi Richard, > > > +const struct tst_arch tst_arch = { > > +#if defined(__x86_64__) > > + .name = "x86_64", > > Writing out these string constants multiple times is error > prone. Perhaps arch_type_list can be indexed with enum tst_arch_type and > then name can be ".name = arch_type_list[TST_X86]"? > > Right, that will more flexible but you know, all arches we have are just > those, and we write them only once in the LTP test library. > > I slightly wanted to keep string constants to make it more > simple/readable. It's a minor thing, but IMO the extra complication is worth it to eliminate typos. Although the preprocessor could still hide errors on some arch until someone tries to compile it. > > > +static const char *const arch_type_list[] = { > > + "i386", > > + "i586", > > + "i686", > > These are not valid arch names AFAICT. There is no mapping from these to > x86 in the tst_arch table above. > > Perhaps we could replace them all with x86? > > Yes, that is also the unsure part I was concerned about in patch v4. > Because x86 is also an invalid arch (it is just a conventional name), > if we use it in the arch_type_list we have to recognize it as a valid arch > name in test case as well. I'm not sure that will be accepted by other > people. The folder in the kernel tree is arch/x86. -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 14:01 ` Richard Palethorpe @ 2021-11-18 11:42 ` Cyril Hrubis 0 siblings, 0 replies; 18+ messages in thread From: Cyril Hrubis @ 2021-11-18 11:42 UTC (permalink / raw) To: Richard Palethorpe; +Cc: LTP List Hi! > > These are not valid arch names AFAICT. There is no mapping from these to > > x86 in the tst_arch table above. > > > > Perhaps we could replace them all with x86? > > > > Yes, that is also the unsure part I was concerned about in patch v4. > > Because x86 is also an invalid arch (it is just a conventional name), > > if we use it in the arch_type_list we have to recognize it as a valid arch > > name in test case as well. I'm not sure that will be accepted by other > > people. > > The folder in the kernel tree is arch/x86. The x86 is basically a wildcard where the x could be substitued with any of {3,4,5,6,7}. Also I think that 786 was last 32bit intel arch ever released. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 7:07 [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Li Wang ` (2 preceding siblings ...) 2021-11-17 9:58 ` [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Richard Palethorpe @ 2021-11-17 13:59 ` Joerg Vehlow 2021-11-17 14:25 ` Richard Palethorpe 2021-11-18 12:07 ` Cyril Hrubis 3 siblings, 2 replies; 18+ messages in thread From: Joerg Vehlow @ 2021-11-17 13:59 UTC (permalink / raw) To: Li Wang, ltp Hi, On 11/17/2021 8:07 AM, Li Wang wrote: > Testcases for specific arch should be limited on that only being supported > platform to run, we now involve a .supported_archs to achieve this feature > in LTP library. All you need to run a test on the expected arch is to set > the '.supported_archs' array in the 'struct tst_test' to choose the required > arch list. e.g. > > .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > > This helps move the TCONF info from code to tst_test metadata as well. while I do like this, I wonder if it wouldn't be better to do this using kernel config. IIRC there are config switches for all architectures. Further more this would allow adding more complex conditions in the future. E.g: I am pretty sure, that there are some syscalls, that have existed "forever" in x86_64, but where only added in a specific version for aarch64. By making the arch a separate option, there is no way, to model this. If it was done in the kernel config check, it could be possible to add version and arch checks like (CONFIG_AARCH64 && CONFIG_VERSION > 5.3) || CONFIG_X86_64 While this probably does not produce a very good error message, it is more versatile. Sorry for this late questioning the whole approach. Joerg -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 13:59 ` Joerg Vehlow @ 2021-11-17 14:25 ` Richard Palethorpe 2021-11-18 5:50 ` Li Wang 2021-11-18 12:07 ` Cyril Hrubis 1 sibling, 1 reply; 18+ messages in thread From: Richard Palethorpe @ 2021-11-17 14:25 UTC (permalink / raw) To: Joerg Vehlow; +Cc: ltp Hello Joerg, Joerg Vehlow <lkml@jv-coder.de> writes: > Hi, > > On 11/17/2021 8:07 AM, Li Wang wrote: >> Testcases for specific arch should be limited on that only being supported >> platform to run, we now involve a .supported_archs to achieve this feature >> in LTP library. All you need to run a test on the expected arch is to set >> the '.supported_archs' array in the 'struct tst_test' to choose the required >> arch list. e.g. >> >> .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} >> >> This helps move the TCONF info from code to tst_test metadata as well. > > while I do like this, I wonder if it wouldn't be better to do this > using kernel config. IIRC there are config switches > for all architectures. Further more this would allow adding more > complex conditions in the future. > > E.g: I am pretty sure, that there are some syscalls, that have existed > "forever" in x86_64, but where only added > in a specific version for aarch64. By making the arch a separate > option, there is no way, to model this. > If it was done in the kernel config check, it could be possible to add > version and arch checks like > (CONFIG_AARCH64 && CONFIG_VERSION > 5.3) || CONFIG_X86_64 > > While this probably does not produce a very good error message, it is > more versatile. > > Sorry for this late questioning the whole approach. It should never be too late IMO. We should also consider whether there are existing tst_test flags which have been made redundant by kconfig. I suspect the main issue would be meta-data. As an arbitrarily complicated kconfig expression may confuse test harnesses. It might be better for us to tackle that issue and use kconfig though. -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 14:25 ` Richard Palethorpe @ 2021-11-18 5:50 ` Li Wang 0 siblings, 0 replies; 18+ messages in thread From: Li Wang @ 2021-11-18 5:50 UTC (permalink / raw) To: Richard Palethorpe; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 3072 bytes --] Hi Joerg, Richard, On Wed, Nov 17, 2021 at 10:37 PM Richard Palethorpe <rpalethorpe@suse.de> wrote: > Hello Joerg, > > Joerg Vehlow <lkml@jv-coder.de> writes: > > > Hi, > > > > On 11/17/2021 8:07 AM, Li Wang wrote: > >> Testcases for specific arch should be limited on that only being > supported > >> platform to run, we now involve a .supported_archs to achieve this > feature > >> in LTP library. All you need to run a test on the expected arch is to > set > >> the '.supported_archs' array in the 'struct tst_test' to choose the > required > >> arch list. e.g. > >> > >> .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > >> > >> This helps move the TCONF info from code to tst_test metadata as well. > > > > while I do like this, I wonder if it wouldn't be better to do this > > using kernel config. IIRC there are config switches > > for all architectures. Further more this would allow adding more > > complex conditions in the future. > > > > E.g: I am pretty sure, that there are some syscalls, that have existed > > "forever" in x86_64, but where only added > > in a specific version for aarch64. By making the arch a separate > > option, there is no way, to model this. > Umm, we shouldn't set .supported_archs to make it a separate option unless we have an explicit requirement on that. In other words, without that .supported_archs setting, it will support all arches by default and we can do anything by the exported tst_arch structure and enum tst_arch_type. > > If it was done in the kernel config check, it could be possible to add > > version and arch checks like > > (CONFIG_AARCH64 && CONFIG_VERSION > 5.3) || CONFIG_X86_64 > We definitely can achieve this in the current version as well. switch (tst_arch.type) case TST_AARCH64: if ((tst_kvercmp(5, 3, 0)) < 0) break; case TST_X86_64: ltp_syscall(__NR_special_syscall, ...) break; ... > > > > While this probably does not produce a very good error message, it is > > more versatile. > > > > Sorry for this late questioning the whole approach. > > It should never be too late IMO. We should also consider whether there > are existing tst_test flags which have been made redundant by kconfig. > After checking the config file again, yes, I agree that we can achieve the same thing just with existing kconfig functions to parse it. E.g. If required x86_64: .needs_kconfigs = (const char *[]) { "CONFIG_X86_64=y", NULL }, and s390x will be like: .needs_kconfigs = (const char *[]) { "CONFIG_64BIT=y", "CONFIG_S390=y" NULL }, ... > > I suspect the main issue would be meta-data. As an arbitrarily > complicated kconfig expression may confuse test harnesses. > Right, so I would like to keep the .supported_archs and tst_arch structure no change, even though goes with parsing config file in the library. > > It might be better for us to tackle that issue and use kconfig though. > > -- > Thank you, > Richard. > > -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 6394 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-17 13:59 ` Joerg Vehlow 2021-11-17 14:25 ` Richard Palethorpe @ 2021-11-18 12:07 ` Cyril Hrubis 2021-11-22 2:21 ` Li Wang 1 sibling, 1 reply; 18+ messages in thread From: Cyril Hrubis @ 2021-11-18 12:07 UTC (permalink / raw) To: Joerg Vehlow; +Cc: ltp Hi! > > Testcases for specific arch should be limited on that only being supported > > platform to run, we now involve a .supported_archs to achieve this feature > > in LTP library. All you need to run a test on the expected arch is to set > > the '.supported_archs' array in the 'struct tst_test' to choose the required > > arch list. e.g. > > > > .supported_archs = (const char *const []){"x86_64", "ppc64", NULL} > > > > This helps move the TCONF info from code to tst_test metadata as well. > > while I do like this, I wonder if it wouldn't be better to do this using > kernel config. IIRC there are config switches > for all architectures. Further more this would allow adding more complex > conditions in the future. > > E.g: I am pretty sure, that there are some syscalls, that have existed > "forever" in x86_64, but where only added > in a specific version for aarch64. By making the arch a separate option, > there is no way, to model this. > If it was done in the kernel config check, it could be possible to add > version and arch checks like > (CONFIG_AARCH64 && CONFIG_VERSION > 5.3) || CONFIG_X86_64 > > While this probably does not produce a very good error message, it is > more versatile. > > Sorry for this late questioning the whole approach. Not at all, this is a good point. The main problem is that the kernel architecture does not need to match the binary architecture which is what this patchset tries to cover. That means that 32bit binary on 64bit kernel would not match what we are supposed to match. Even more the config variables are confusing, on x86_64 with compat layer enabled we get: CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y That makes any reasoning quite messy. What would make much more sense would be injecting LTP specific variables to the parsed variables before evaluation. So for instance we would insert BINARY_ARCH variable which would cover this exact case and the check would look like: "(BINARY_ARCH == "aarch64" && CONFIG_VERSION > 5.3) || CONFIG_X86_64" However I would still like to have a simple list of supported architectures in the test structure as well, since that is much easier to read and reason about and it covers 99% of the cases. Nothing stops us for adding the more complex checks in the case that we see the need later on. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-18 12:07 ` Cyril Hrubis @ 2021-11-22 2:21 ` Li Wang 2021-11-22 13:27 ` Richard Palethorpe 0 siblings, 1 reply; 18+ messages in thread From: Li Wang @ 2021-11-22 2:21 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 181 bytes --] Hi All, By now, should I push this patch v5, or is any else change needed? (From the above discussion I feel v5 is satisfied almost all of our requirements.) -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 566 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-22 2:21 ` Li Wang @ 2021-11-22 13:27 ` Richard Palethorpe 2021-11-22 14:21 ` Cyril Hrubis 0 siblings, 1 reply; 18+ messages in thread From: Richard Palethorpe @ 2021-11-22 13:27 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hello Li, Li Wang <liwang@redhat.com> writes: > Hi All, > > By now, should I push this patch v5, or is any else change needed? I still don't understand how tst_arch.name is mapped to i386, i586 or i686? It appears that we will always get TCONF on any of these architectures because tst_arch.name will be set to x86 and the required arch will be i386, i586 or i686. > (From the above discussion I feel v5 is satisfied almost all of our > requirements.) Otherwise, yes, I agree. > > -- > Regards, > Li Wang -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-22 13:27 ` Richard Palethorpe @ 2021-11-22 14:21 ` Cyril Hrubis 2021-11-23 4:13 ` Li Wang 0 siblings, 1 reply; 18+ messages in thread From: Cyril Hrubis @ 2021-11-22 14:21 UTC (permalink / raw) To: Richard Palethorpe; +Cc: ltp Hi! > I still don't understand how tst_arch.name is mapped to i386, i586 or > i686? > > It appears that we will always get TCONF on any of these architectures > because tst_arch.name will be set to x86 and the required arch will be > i386, i586 or i686. Can we please have single name for 32bit intel i.e. "x86" please? > > (From the above discussion I feel v5 is satisfied almost all of our > > requirements.) > > Otherwise, yes, I agree. Same here, the rest looks good. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure 2021-11-22 14:21 ` Cyril Hrubis @ 2021-11-23 4:13 ` Li Wang 0 siblings, 0 replies; 18+ messages in thread From: Li Wang @ 2021-11-23 4:13 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 664 bytes --] Hi All, On Mon, Nov 22, 2021 at 10:20 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > I still don't understand how tst_arch.name is mapped to i386, i586 or > > i686? > > > > It appears that we will always get TCONF on any of these architectures > > because tst_arch.name will be set to x86 and the required arch will be > > i386, i586 or i686. > You are right, I neglected that point, anyway, let's switch to x86 in all. > > Can we please have single name for 32bit intel i.e. "x86" please? > Ok, sure. Same here, the rest looks good. > Thank you all for the reviews. I made changes by using the single name "x86" and pushed it. -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 1991 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-11-23 4:13 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-11-17 7:07 [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 2/3] testcase: make use of .supported_archs Li Wang 2021-11-17 7:07 ` [LTP] [PATCH v5 3/3] max_map_count: replace ifdefs by tst_arch Li Wang 2021-11-17 10:33 ` Richard Palethorpe 2021-11-17 13:28 ` Li Wang 2021-11-17 13:58 ` Richard Palethorpe 2021-11-17 9:58 ` [LTP] [PATCH v5 1/3] lib: adding .supported_archs field in tst_test structure Richard Palethorpe 2021-11-17 13:51 ` Li Wang 2021-11-17 14:01 ` Richard Palethorpe 2021-11-18 11:42 ` Cyril Hrubis 2021-11-17 13:59 ` Joerg Vehlow 2021-11-17 14:25 ` Richard Palethorpe 2021-11-18 5:50 ` Li Wang 2021-11-18 12:07 ` Cyril Hrubis 2021-11-22 2:21 ` Li Wang 2021-11-22 13:27 ` Richard Palethorpe 2021-11-22 14:21 ` Cyril Hrubis 2021-11-23 4:13 ` Li Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox